Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Path to uv.html can now be set with options.embedTemplate #1254

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from
Open

Path to uv.html can now be set with options.embedTemplate #1254

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8c2e399
feat(embed): move /uv.html to options.embedTemplate for easier embedd…
crhallberg Dec 12, 2024
ea32f49
Commit from GitHub Actions (Lint code)
github-actions[bot] Dec 12, 2024
40ac1f1
revert: test code
crhallberg Dec 16, 2024
0e40060
refactor: add uv.html back to getAppUri.
crhallberg Jan 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions src/content-handlers/iiif/extensions/uv-aleph-extension/Extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,16 +121,21 @@ export default class Extension
}

getEmbedScript(template: string, width: number, height: number): string {
const hashParams = new URLSearchParams({
manifest: this.helper.manifestUri,
});

const appUri: string = this.getAppUri();
const title: string = this.helper.getLabel() || "";
const iframeSrc: string = `${appUri}#?manifest=${this.helper.manifestUri}`;
const title: string = this.helper.getLabel() ?? "";
const script: string = Strings.format(
template,
iframeSrc,
appUri,
hashParams.toString(),
width.toString(),
height.toString(),
title
);

return script;
}
}
2 changes: 1 addition & 1 deletion src/content-handlers/iiif/extensions/uv-aleph-extension/config/config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@
"options": {
"embedEnabled": true,
"shareEnabled": true,
"embedTemplate": "<iframe src=\"{0}\" width=\"{1}\" height=\"{2}\" allowfullscreen frameborder=\"0\" title=\"{3}\"></iframe>",
"embedTemplate": "<iframe src=\"{0}/uv.html#?{1}\" width=\"{2}\" height=\"{3}\" allowfullscreen frameborder=\"0\" title=\"{4}\"></iframe>",
"instructionsEnabled": false,
"shareFrameEnabled": true,
"shareManifestsEnabled": true,
Expand Down
15 changes: 12 additions & 3 deletions src/content-handlers/iiif/extensions/uv-av-extension/Extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,16 +149,25 @@ export default class Extension
}

getEmbedScript(template: string, width: number, height: number): string {
const hashParams = new URLSearchParams({
manifest: this.helper.manifestUri,
c: this.helper.collectionIndex.toString(),
m: this.helper.manifestIndex.toString(),
cv: this.helper.canvasIndex.toString(),
rid: this.helper.rangeId?.toString() ?? "",
});

const appUri: string = this.getAppUri();
const title: string = this.helper.getLabel() || "";
const iframeSrc: string = `${appUri}#?manifest=${this.helper.manifestUri}&c=${this.helper.collectionIndex}&m=${this.helper.manifestIndex}&cv=${this.helper.canvasIndex}&rid=${this.helper.rangeId}`;
const title: string = this.helper.getLabel() ?? "";
const script: string = Strings.format(
template,
iframeSrc,
appUri,
hashParams.toString(),
width.toString(),
height.toString(),
title
);

return script;
}

Expand Down
2 changes: 1 addition & 1 deletion src/content-handlers/iiif/extensions/uv-av-extension/config/config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@
"options": {
"embedEnabled": true,
"shareEnabled": true,
"embedTemplate": "<iframe src=\"{0}\" width=\"{1}\" height=\"{2}\" allowfullscreen frameborder=\"0\" title=\"{3}\"></iframe>",
"embedTemplate": "<iframe src=\"{0}/uv.html#?{1}\" width=\"{2}\" height=\"{3}\" allowfullscreen frameborder=\"0\" title=\"{4}\"></iframe>",
"instructionsEnabled": false,
"shareFrameEnabled": true,
"shareManifestsEnabled": true,
Expand Down
14 changes: 11 additions & 3 deletions src/content-handlers/iiif/extensions/uv-default-extension/Extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,16 +116,24 @@ export default class Extension
}

getEmbedScript(template: string, width: number, height: number): string {
const hashParams = new URLSearchParams({
manifest: this.helper.manifestUri,
c: this.helper.collectionIndex.toString(),
m: this.helper.manifestIndex.toString(),
cv: this.helper.canvasIndex.toString(),
});

const appUri: string = this.getAppUri();
const title: string = this.helper.getLabel() || "";
const iframeSrc: string = `${appUri}#?manifest=${this.helper.manifestUri}&c=${this.helper.collectionIndex}&m=${this.helper.manifestIndex}&cv=${this.helper.canvasIndex}`;
const title: string = this.helper.getLabel() ?? "";
const script: string = Strings.format(
template,
iframeSrc,
appUri,
hashParams.toString(),
width.toString(),
height.toString(),
title
);

return script;
}
}
2 changes: 1 addition & 1 deletion src/content-handlers/iiif/extensions/uv-default-extension/config/config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
"options": {
"embedEnabled": true,
"shareEnabled": true,
"embedTemplate": "<iframe src=\"{0}\" width=\"{1}\" height=\"{2}\" allowfullscreen frameborder=\"0\" title=\"{3}\"></iframe>",
"embedTemplate": "<iframe src=\"{0}/uv.html#?{1}\" width=\"{2}\" height=\"{3}\" allowfullscreen frameborder=\"0\" title=\"{4}\"></iframe>",
"instructionsEnabled": false,
"shareFrameEnabled": true,
"shareManifestsEnabled": true,
Expand Down
12 changes: 9 additions & 3 deletions src/content-handlers/iiif/extensions/uv-ebook-extension/Extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,16 +142,22 @@ export default class Extension
}

getEmbedScript(template: string, width: number, height: number): string {
const hashParams = new URLSearchParams({
manifest: this.helper.manifestUri,
cfi: this.cfiFragement,
});

const appUri: string = this.getAppUri();
const title: string = this.helper.getLabel() || "";
const iframeSrc: string = `${appUri}#?manifest=${this.helper.manifestUri}&cfi=${this.cfiFragement}`;
const title: string = this.helper.getLabel() ?? "";
const script: string = Strings.format(
template,
iframeSrc,
appUri,
hashParams.toString(),
width.toString(),
height.toString(),
title
);

return script;
}

Expand Down
2 changes: 1 addition & 1 deletion src/content-handlers/iiif/extensions/uv-ebook-extension/config/config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@
"options": {
"embedEnabled": true,
"shareEnabled": true,
"embedTemplate": "<iframe src=\"{0}\" width=\"{1}\" height=\"{2}\" allowfullscreen frameborder=\"0\" title=\"{3}\"></iframe>",
"embedTemplate": "<iframe src=\"{0}/uv.html#?{1}\" width=\"{2}\" height=\"{3}\" allowfullscreen frameborder=\"0\" title=\"{4}\"></iframe>",
"instructionsEnabled": false,
"shareFrameEnabled": true,
"shareManifestsEnabled": true,
Expand Down
14 changes: 11 additions & 3 deletions src/content-handlers/iiif/extensions/uv-mediaelement-extension/Extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,16 +236,24 @@ export default class Extension
}

getEmbedScript(template: string, width: number, height: number): string {
const hashParams = new URLSearchParams({
manifest: this.helper.manifestUri,
c: this.helper.collectionIndex.toString(),
m: this.helper.manifestIndex.toString(),
cv: this.helper.canvasIndex.toString(),
});

const appUri: string = this.getAppUri();
const title: string = this.helper.getLabel() || "";
const iframeSrc: string = `${appUri}#?manifest=${this.helper.manifestUri}&c=${this.helper.collectionIndex}&m=${this.helper.manifestIndex}&cv=${this.helper.canvasIndex}`;
const title: string = this.helper.getLabel() ?? "";
const script: string = Strings.format(
template,
iframeSrc,
appUri,
hashParams.toString(),
width.toString(),
height.toString(),
title
);

return script;
}

Expand Down
2 changes: 1 addition & 1 deletion src/content-handlers/iiif/extensions/uv-mediaelement-extension/config/config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
"options": {
"embedEnabled": true,
"shareEnabled": true,
"embedTemplate": "<iframe src=\"{0}\" width=\"{1}\" height=\"{2}\" allowfullscreen frameborder=\"0\" title=\"{3}\"></iframe>",
"embedTemplate": "<iframe src=\"{0}/uv.html#?{1}\" width=\"{2}\" height=\"{3}\" allowfullscreen frameborder=\"0\" title=\"{4}\"></iframe>",
"instructionsEnabled": false,
"shareFrameEnabled": true,
"shareManifestsEnabled": true,
Expand Down
14 changes: 11 additions & 3 deletions src/content-handlers/iiif/extensions/uv-model-viewer-extension/Extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,16 +255,24 @@ export default class ModelViewerExtension extends BaseExtension<Config> {
}

getEmbedScript(template: string, width: number, height: number): string {
const hashParams = new URLSearchParams({
manifest: this.helper.manifestUri,
c: this.helper.collectionIndex.toString(),
m: this.helper.manifestIndex.toString(),
cv: this.helper.canvasIndex.toString(),
});

const appUri: string = this.getAppUri();
const title: string = this.helper.getLabel() || "";
const iframeSrc: string = `${appUri}#?manifest=${this.helper.manifestUri}&c=${this.helper.collectionIndex}&m=${this.helper.manifestIndex}&cv=${this.helper.canvasIndex}`;
const title: string = this.helper.getLabel() ?? "";
const script: string = Strings.format(
template,
iframeSrc,
appUri,
hashParams.toString(),
width.toString(),
height.toString(),
title
);

return script;
}
}
2 changes: 1 addition & 1 deletion src/content-handlers/iiif/extensions/uv-model-viewer-extension/config/config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
"options": {
"embedEnabled": true,
"shareEnabled": true,
"embedTemplate": "<iframe src=\"{0}\" width=\"{1}\" height=\"{2}\" allowfullscreen frameborder=\"0\" title=\"{3}\"></iframe>",
"embedTemplate": "<iframe src=\"{0}/uv.html#?{1}\" width=\"{2}\" height=\"{3}\" allowfullscreen frameborder=\"0\" title=\"{4}\"></iframe>",
"instructionsEnabled": false,
"shareFrameEnabled": true,
"shareManifestsEnabled": true,
Expand Down
20 changes: 16 additions & 4 deletions src/content-handlers/iiif/extensions/uv-openseadragon-extension/Extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1369,18 +1369,30 @@ export default class OpenSeadragonExtension extends BaseExtension<Config> {
zoom: string,
rotation: number
): string {
const config: string = this.data.config!.uri || "";
const locales: string | null = this.getSerializedLocales();
const config: string = this.data.config?.uri ?? "";
const locales: string = this.getSerializedLocales() ?? "";
const hashParams = new URLSearchParams({
manifest: this.helper.manifestUri,
c: this.helper.collectionIndex.toString(),
m: this.helper.manifestIndex.toString(),
cv: this.helper.canvasIndex.toString(),
config: config,
locales: locales,
xywh: zoom,
r: rotation.toString(),
});

const appUri: string = this.getAppUri();
const title: string = this.helper.getLabel() || "";
const iframeSrc: string = `${appUri}#?manifest=${this.helper.manifestUri}&c=${this.helper.collectionIndex}&m=${this.helper.manifestIndex}&cv=${this.helper.canvasIndex}&config=${config}&locales=${locales}&xywh=${zoom}&r=${rotation}`;
const script: string = Strings.format(
template,
iframeSrc,
appUri,
hashParams.toString(),
width.toString(),
height.toString(),
title
);

return script;
}

Expand Down
2 changes: 1 addition & 1 deletion src/content-handlers/iiif/extensions/uv-openseadragon-extension/config/config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@
"shareDialogue": {
"options": {
"embedEnabled": true,
"embedTemplate": "<iframe src=\"{0}\" width=\"{1}\" height=\"{2}\" allowfullscreen frameborder=\"0\" title=\"{3}\"></iframe>",
"embedTemplate": "<iframe src=\"{0}/uv.html#?{1}\" width=\"{2}\" height=\"{3}\" allowfullscreen frameborder=\"0\" title=\"{4}\"></iframe>",
"instructionsEnabled": false,
"shareEnabled": true,
"shareFrameEnabled": true,
Expand Down
14 changes: 11 additions & 3 deletions src/content-handlers/iiif/extensions/uv-pdf-extension/Extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,16 +160,24 @@ export default class Extension
}

getEmbedScript(template: string, width: number, height: number): string {
const hashParams = new URLSearchParams({
manifest: this.helper.manifestUri,
c: this.helper.collectionIndex.toString(),
m: this.helper.manifestIndex.toString(),
cv: this.helper.canvasIndex.toString(),
});

const appUri: string = this.getAppUri();
const title: string = this.helper.getLabel() || "";
const iframeSrc: string = `${appUri}#?manifest=${this.helper.manifestUri}&c=${this.helper.collectionIndex}&m=${this.helper.manifestIndex}&cv=${this.helper.canvasIndex}`;
const title: string = this.helper.getLabel() ?? "";
const script: string = Strings.format(
template,
iframeSrc,
appUri,
hashParams.toString(),
width.toString(),
height.toString(),
title
);

return script;
}
}
2 changes: 1 addition & 1 deletion src/content-handlers/iiif/extensions/uv-pdf-extension/config/config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@
"options": {
"embedEnabled": true,
"shareEnabled": true,
"embedTemplate": "<iframe src=\"{0}\" width=\"{1}\" height=\"{2}\" allowfullscreen frameborder=\"0\" title=\"{3}\"></iframe>",
"embedTemplate": "<iframe src=\"{0}/uv.html#?{1}\" width=\"{2}\" height=\"{3}\" allowfullscreen frameborder=\"0\" title=\"{4}\"></iframe>",
"instructionsEnabled": false,
"shareFrameEnabled": true,
"shareManifestsEnabled": true,
Expand Down
9 changes: 2 additions & 7 deletions src/content-handlers/iiif/modules/uv-shared-module/BaseExtension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -765,13 +765,8 @@ export class BaseExtension<T extends BaseConfig> implements IExtension {
}

getAppUri(): string {
const appUri: string =
window.location.protocol +
"//" +
window.location.hostname +
(window.location.port ? ":" + window.location.port : "");

return appUri + "/uv.html";
const port = window.location.port ? `:${window.location.port}` : "";
return `${window.location.protocol}//${window.location.hostname}${port}`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than moving /uv.html from here into the embed template, would it be better to create a new config setting like uvAppPath that defaults to /uv.html? Then we wouldn't have to change the embed templates as much, and other code looking for the full path to uv.html would also work correctly.

I wonder if it would make sense to split this PR in two: one to do the hash parameter simplification, since I think that is a useful improvement to readability, and one to add the new config setting. Another advantage to this approach: we wouldn't have to change the number of placeholders in the embed template, which would better maintain backward compatibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea. I think that getAppUri should not include /uv.html but if I'm misunderstanding the usage...

Copy link
Contributor

@demiankatz demiankatz Dec 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's okay for getAppUri to include /uv.html -- that is part of the URL of the UV application itself. I could imagine it being useful to get the URL of the path to the application without the filename, but I think that would be a separate method that doesn't exist yet (presumably because it hasn't been needed yet). I'd be inclined to keep method names and behaviors as they are, and refactor in future if a need arises.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright. I switched it back. I would like to hear other opinions about this if anyone feels strongly bc I think it makes the template string harder to read and understand.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I've marked this for steering group review so we can discuss it on a future meeting.

My view here is that getAppUri should return a full working URL to the running UV app -- and that includes the uv.html part. If we only want the path part without the actual filename on the end, I would think that would require a different method like getAppPath.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the distinction for sure. Having both could be the solution. Although I’d be inclined to use getAppPath for the readability in the template - which would leave getAppUri unused.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if somebody customizes their UV to use a filename other than uv.html, we would need a way to override that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what the customization of the template solves.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't seem appropriate to bring it up during steering committee. Summoning @edsilv for his thoughts!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you're saying. We have a to-do checkbox up above to make the actual application path a configurable option. I think it would be nice to have more granular configuration so we could configure just the piece that needs to be tweaked instead of having to copy, paste and modify the whole template (which could get out of sync over time if other things change)... but I see how either approach could be potentially workable, and the two might complement each other.

}

getSettings(): ISettings {
Expand Down
5 changes: 5 additions & 0 deletions src/uv-iiif-config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@
"options": {
"downloadEnabled": true
}
},
"shareDialogue": {
"options": {
"embedTemplate": "<iframe src=\"{0}/assets/uv.html#?{1}\" width=\"{2}\" height=\"{3}\" allowfullscreen frameborder=\"0\" title=\"{4}\"></iframe>"
crhallberg marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
}
Loading