Have vault Fallback on Embedded Canvas Data in the Manifest#434
Have vault Fallback on Embedded Canvas Data in the Manifest#434thehabes wants to merge 11 commits into417-strengthen-attempts-to-load-canvasfrom
vault Fallback on Embedded Canvas Data in the Manifest#434Conversation
Consider IndexedDB for caching large IIIF resourcesCurrently, In TPEN Interfaces, users frequently move between pages within a project, and the same manifest is needed repeatedly — that's unnecessary network overhead each time. Suggestion: Use IndexedDB for persisting these large types across page loads. It's a native browser API (no npm packages needed), has much larger storage quotas (typically 50MB+), and handles structured data well. The tradeoff is that IndexedDB is async-only with a more verbose API than This isn't blocking for the current PR, but worth considering as a follow-up improvement. |
| // Pre-load manifest into vault so embedded canvases are available for fallback | ||
| const manifestUri = ev.detail?.manifest?.[0] | ||
| if (manifestUri) { | ||
| import('../js/vault.js').then(m => m.default.get(manifestUri, 'manifest').catch(() => {})) |
There was a problem hiding this comment.
This couples vault.js into TPEN.js which I don't think is wanted or needed.
If this behavior is wanted, it can be added to vault.js so it looks and listens when the module is loaded. There is no reason to load vault and start populating it unless the component is going to use it.
| let page | ||
| try { | ||
| page = vault.get({id:annotationPageID,type:"AnnotationPage"}) ?? await vault.load(annotationPageID) | ||
| page = await vault.get(annotationPageID, 'annotationpage', true, 'tpen-transcription') |
There was a problem hiding this comment.
This is the actual "Vault" syntax. We might look at supporting both, but I think we're good here.
| return userMessage(err.message ?? err.statusText ?? err.text ?? 'Unknown error') | ||
| } | ||
| } | ||
| if (!page?.items) return userMessage('No annotations found on this page') |
There was a problem hiding this comment.
Page is an AnnotationPage here... we might need to look at what happens to an AnnotationList, since it has annotations in a different place and wouldn't have items.
| })).then(results => results.flat()) | ||
| this.#transcriptionContainer.append(...lines) | ||
| this.activeLine = lines[0].line | ||
| this.activeLine = lines[0]?.line |
There was a problem hiding this comment.
This ? should never be used because !lines is an escape clause above.
| } | ||
| const context = resolvedPage["@context"] | ||
| if(!(context.includes("iiif.io/api/presentation/3/context.json") || context.includes("w3.org/ns/anno.jsonld"))){ | ||
| if(context && !(context.includes("iiif.io/api/presentation/3/context.json") || context.includes("w3.org/ns/anno.jsonld"))){ |
There was a problem hiding this comment.
I don't like the context && syntax. If we think there will be something missing, we can context?.includes?.() which will not only trip if context is undefined, but also not true-fail on values like 5 which is truthy but has no "includes()".
In real life this line feels like a utility to be extracted, but if it's just used here, I'm over it.
| */ | ||
| async _resolveCanvasFromManifest(canvasId, component) { | ||
| // 1. Check if already available from a cached manifest | ||
| let embedded = this._getEmbeddedCanvas(canvasId) |
| * @param {string} canvasId - The canvas ID | ||
| * @param {string} manifestUri - The manifest URI containing this canvas | ||
| */ | ||
| registerManifestHint(canvasId, manifestUri) { |
There was a problem hiding this comment.
Gross. I literally prefer to never care ever what Manifest it is part of. In fact, if it gets shunted into several and the same ID is used, I hope the most recent one loaded replaces the older and causes conflicts.
| ) | ||
| this._dispatchCanvasError(error) | ||
| } else { | ||
| return null |
There was a problem hiding this comment.
why not return when response is not ok?
| if (isCanvas) { | ||
| const validationError = this._validateCanvas(data, uri) | ||
| if (validationError) { | ||
| validationError.component = component |
There was a problem hiding this comment.
Is this the only place component matters? Do we need this?
| this.store.delete(type) | ||
| // Clean up canvasToManifest entries when clearing manifests | ||
| if (type === 'manifest') { | ||
| this.canvasToManifest.clear() |
2 participants
Closes #429