Skip to content

use explicit manifest #1755

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

Open
LanieOkorodudu wants to merge 2 commits into
base: dev
Choose a base branch
from
+46 −8
Open

use explicit manifest #1755

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
28c2488
use explicit manifest
LanieOkorodudu May 11, 2026
8e4d9af
Merge branch 'UniversalViewer:dev' into add-test
LanieOkorodudu May 12, 2026
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
52 changes: 45 additions & 7 deletions __tests__/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,20 @@ test.skip("Configuration options", () => {});
const puppeteer = require("puppeteer");
const { BASE_URL } = require("../scripts/testBaseUrl");

// Cookbook manifest for viewer control tests
const COOKBOOK_MANIFEST =
Copy link
Copy Markdown
Contributor

@demiankatz demiankatz May 12, 2026

Choose a reason for hiding this comment

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

Maybe we should give this a more specific name, since we'll likely have more cookbook manifests in the future -- perhaps COOKBOOK_BOUND_MULTIVOLUME_MANIFEST?

"https://iiif.io/api/cookbook/recipe/0031-bound-multivolume/manifest.json";

// PDF manifest for PDF-specific behaviour
const PDF_MANIFEST =
Copy link
Copy Markdown
Contributor

@demiankatz demiankatz May 12, 2026

Choose a reason for hiding this comment

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

Maybe this should be PDF_MULTI_FILE_MANIFEST -- since the edge case this manifest was provided to test is a set of multiple PDF files in a single manifest. One of the things we should confirm when loading the manifest is that the left sidebar appears with multiple files listed. (If we load a single-PDF manifest, the left sidebar should NOT appear).

"https://digital.library.villanova.edu/Item/vudl:294631/Manifest";

const viewerUrl = (manifestUrl) => {
const separator = BASE_URL.includes("?") ? "&" : "?";

return `${BASE_URL}${separator}manifest=${encodeURIComponent(manifestUrl)}`;
Copy link
Copy Markdown
Contributor

@demiankatz demiankatz May 12, 2026

Choose a reason for hiding this comment

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

Does this really work? I thought the manifest argument had to be part of the hash parameters, not part of the base query parameters... in other words, https://www.universalviewer.dev/?manifest=https://digital.library.villanova.edu/Item/vudl:294631 does not work, but https://www.universalviewer.dev/#?manifest=https://digital.library.villanova.edu/Item/vudl:294631 does work. Note the #? instead of just ?.

Copy link
Copy Markdown
Collaborator Author

@LanieOkorodudu LanieOkorodudu May 12, 2026

Choose a reason for hiding this comment

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

It did appear to work when I tested yesterday, but you are right that the manifest parameter should probably be part of the hash parameters rather than the base query parameters. I’ll update viewerUrl to use #?manifest= instead. Thanks

};

describe("Universal Viewer", () => {
let browser;
let page;
Expand Down Expand Up @@ -85,6 +99,7 @@ describe("Universal Viewer", () => {
await browser.close();
});

// Default manifest test
it("has the correct page title", async () => {
const title = await page.title();
expect(title).toBe("Universal Viewer Examples");
Expand Down Expand Up @@ -194,9 +209,10 @@ describe("Universal Viewer", () => {
expect(isSettingsButtonVisible).toBe(true);
});

// COOKBOOK MANIFEST TEST
describe("viewer controls", () => {
afterEach(async () => {
await page.goto(BASE_URL);
beforeEach(async () => {
await page.goto(viewerUrl(COOKBOOK_MANIFEST));
});

// can navigate back and forth
Expand Down Expand Up @@ -335,8 +351,8 @@ describe("Universal Viewer", () => {
const contentThumbnailsTab = ".thumbs.tab";
const contentThumbnailsActiveTab = ".thumbs.tab.on";

afterEach(async () => {
await page.goto(BASE_URL);
beforeEach(async () => {
await page.goto(viewerUrl(COOKBOOK_MANIFEST));
});

// switch content tabs and collapse content panel
Expand All @@ -351,7 +367,9 @@ describe("Universal Viewer", () => {
).toBe(true);

expect(
await page.$eval(contentThumbnailsTab, (el) => el.classList.contains("on"))
await page.$eval(contentThumbnailsTab, (el) =>
el.classList.contains("on")
)
).toBe(false);
});

Expand All @@ -371,8 +389,8 @@ describe("Universal Viewer", () => {
const moreInfoCollapseBtn = ".rightPanel button.collapseButton";
const moreInfoHeader = ".rightPanel div.header";

afterEach(async () => {
await page.goto(BASE_URL);
beforeEach(async () => {
await page.goto(viewerUrl(COOKBOOK_MANIFEST));
});

it("can expand and collapse moreInformation panel", async () => {
Expand All @@ -394,4 +412,24 @@ describe("Universal Viewer", () => {
await page.waitForSelector(moreInfoHeader, { hidden: true });
});
});

// PDF MANIFEST TEST
describe("PDF manifest", () => {
beforeEach(async () => {
await page.goto(viewerUrl(PDF_MANIFEST), {
waitUntil: "domcontentloaded",
});
});

it("loads PDF manifest successfully", async () => {
expect(page.url()).toContain(encodeURIComponent(PDF_MANIFEST));

await page.waitForSelector(".uv", { visible: true });

const pageText = await page.evaluate(() => document.body.innerText);

expect(pageText).not.toContain("Unable to load");
expect(pageText).not.toContain("Error loading");
});
});
});
2 changes: 1 addition & 1 deletion jest.setup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
jest.setTimeout(10000);
jest.setTimeout(60000);
Loading