Skip to content

Commit a48118d

Browse files
committed
Remove useless page-id attribute in thumbnails
And make sure that the sidebar is fully open before starting the tests in order to have an intermittent exception when finishing the test.
1 parent bc8efa1 commit a48118d

4 files changed

Lines changed: 69 additions & 35 deletions

File tree

test/integration/reorganize_pages_spec.mjs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,19 @@ import {
3333
} from "./test_utils.mjs";
3434

3535
async function waitForThumbnailVisible(page, pageNums) {
36+
const hasAnimations = await page.evaluate(
37+
() => !window.matchMedia("(prefers-reduced-motion: reduce)").matches
38+
);
3639
await page.click("#viewsManagerToggleButton");
40+
if (hasAnimations) {
41+
await page.waitForSelector("#outerContainer.viewsManagerMoving", {
42+
visible: true,
43+
});
44+
}
45+
await page.waitForSelector(
46+
"#outerContainer:not(.viewsManagerMoving).viewsManagerOpen",
47+
{ visible: true }
48+
);
3749

3850
const thumbSelector = "#thumbnailsView .thumbnailImageContainer > img";
3951
await page.waitForSelector(thumbSelector, { visible: true });
@@ -283,12 +295,12 @@ describe("Reorganize Pages View", () => {
283295
await waitForThumbnailVisible(page, 1);
284296
const rect1 = await getRect(page, getThumbnailSelector(1));
285297
const rect2 = await getRect(page, getThumbnailSelector(2));
286-
await (await page.$(".thumbnail[page-id='14'")).scrollIntoView();
298+
await (await page.$(".thumbnail[page-number='14'")).scrollIntoView();
287299
await page.waitForSelector(getThumbnailSelector(14), {
288300
visible: true,
289301
});
290302
await page.click(`.thumbnail:has(${getThumbnailSelector(14)}) input`);
291-
await (await page.$(".thumbnail[page-id='1'")).scrollIntoView();
303+
await (await page.$(".thumbnail[page-number='1'")).scrollIntoView();
292304
await page.waitForSelector(getThumbnailSelector(1), {
293305
visible: true,
294306
});

test/integration/thumbnail_view_spec.mjs

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,23 @@ async function waitForMenu(page, buttonSelector, visible = true) {
2929
);
3030
}
3131

32+
async function showViewsManager(page) {
33+
const hasAnimations = await page.evaluate(
34+
() => !window.matchMedia("(prefers-reduced-motion: reduce)").matches
35+
);
36+
await page.click("#viewsManagerToggleButton");
37+
if (hasAnimations) {
38+
await page.waitForSelector("#outerContainer.viewsManagerMoving", {
39+
visible: true,
40+
});
41+
}
42+
await page.waitForSelector("#viewsManager", { visible: true });
43+
await page.waitForSelector(
44+
"#outerContainer:not(.viewsManagerMoving).viewsManagerOpen",
45+
{ visible: true }
46+
);
47+
}
48+
3249
describe("PDF Thumbnail View", () => {
3350
describe("Works without errors", () => {
3451
let pages;
@@ -44,7 +61,7 @@ describe("PDF Thumbnail View", () => {
4461
it("should render thumbnails without errors", async () => {
4562
await Promise.all(
4663
pages.map(async ([browserName, page]) => {
47-
await page.click("#viewsManagerToggleButton");
64+
await showViewsManager(page);
4865

4966
const thumbSelector =
5067
"#thumbnailsView .thumbnailImageContainer > img";
@@ -62,7 +79,7 @@ describe("PDF Thumbnail View", () => {
6279
it("should have accessible label on resizer", async () => {
6380
await Promise.all(
6481
pages.map(async ([browserName, page]) => {
65-
await page.click("#viewsManagerToggleButton");
82+
await showViewsManager(page);
6683

6784
const ariaLabel = await page.$eval("#viewsManagerResizer", el =>
6885
el.getAttribute("aria-label")
@@ -104,8 +121,7 @@ describe("PDF Thumbnail View", () => {
104121
it("should scroll the view", async () => {
105122
await Promise.all(
106123
pages.map(async ([browserName, page]) => {
107-
await page.click("#viewsManagerToggleButton");
108-
124+
await showViewsManager(page);
109125
await waitForThumbnailVisible(page, 1);
110126

111127
for (const pageNum of [14, 1, 13, 2]) {
@@ -141,8 +157,7 @@ describe("PDF Thumbnail View", () => {
141157
it("should navigate with the keyboard", async () => {
142158
await Promise.all(
143159
pages.map(async ([browserName, page]) => {
144-
await page.click("#viewsManagerToggleButton");
145-
160+
await showViewsManager(page);
146161
await waitForThumbnailVisible(page, 1);
147162
await waitForThumbnailVisible(page, 2);
148163
await waitForThumbnailVisible(page, 3);
@@ -235,7 +250,7 @@ describe("PDF Thumbnail View", () => {
235250
it("should open with Enter key and remain open", async () => {
236251
await Promise.all(
237252
pages.map(async ([browserName, page]) => {
238-
await page.click("#viewsManagerToggleButton");
253+
await showViewsManager(page);
239254
await waitForThumbnailVisible(page, 1);
240255

241256
await enableMenuItems(page);
@@ -267,7 +282,7 @@ describe("PDF Thumbnail View", () => {
267282
it("should open with Space key and remain open", async () => {
268283
await Promise.all(
269284
pages.map(async ([browserName, page]) => {
270-
await page.click("#viewsManagerToggleButton");
285+
await showViewsManager(page);
271286
await waitForThumbnailVisible(page, 1);
272287

273288
await enableMenuItems(page);
@@ -326,8 +341,7 @@ describe("PDF Thumbnail View", () => {
326341
it("should have accessible label on checkbox", async () => {
327342
await Promise.all(
328343
pages.map(async ([browserName, page]) => {
329-
await page.click("#viewsManagerToggleButton");
330-
344+
await showViewsManager(page);
331345
await waitForThumbnailVisible(page, 1);
332346

333347
const ariaLabel = await page.$eval(
@@ -362,7 +376,7 @@ describe("PDF Thumbnail View", () => {
362376
it("must navigate menus with ArrowDown and Tab keys", async () => {
363377
await Promise.all(
364378
pages.map(async ([browserName, page]) => {
365-
await page.click("#viewsManagerToggleButton");
379+
await showViewsManager(page);
366380
await waitForThumbnailVisible(page, 1);
367381

368382
// Focus the views manager selector button
@@ -431,7 +445,7 @@ describe("PDF Thumbnail View", () => {
431445
it("should show the manage button in thumbnail view and hide it in outline view", async () => {
432446
await Promise.all(
433447
pages.map(async ([browserName, page]) => {
434-
await page.click("#viewsManagerToggleButton");
448+
await showViewsManager(page);
435449
await waitForThumbnailVisible(page, 1);
436450

437451
// The status bar (Select pages + Manage button) must be visible in
@@ -471,8 +485,7 @@ describe("PDF Thumbnail View", () => {
471485
it("should focus checkboxes with Tab key", async () => {
472486
await Promise.all(
473487
pages.map(async ([browserName, page]) => {
474-
await page.click("#viewsManagerToggleButton");
475-
488+
await showViewsManager(page);
476489
await waitForThumbnailVisible(page, 1);
477490

478491
// Focus the first thumbnail button
@@ -499,8 +512,7 @@ describe("PDF Thumbnail View", () => {
499512
it("should navigate checkboxes with arrow keys", async () => {
500513
await Promise.all(
501514
pages.map(async ([browserName, page]) => {
502-
await page.click("#viewsManagerToggleButton");
503-
515+
await showViewsManager(page);
504516
await waitForThumbnailVisible(page, 1);
505517
await waitForThumbnailVisible(page, 2);
506518

web/pdf_thumbnail_view.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,6 @@ class PDFThumbnailView extends RenderableView {
119119
const thumbnailContainer = (this.div = document.createElement("div"));
120120
thumbnailContainer.className = "thumbnail";
121121
thumbnailContainer.setAttribute("page-number", id);
122-
thumbnailContainer.setAttribute("page-id", id);
123122

124123
const imageContainer = (this.imageContainer =
125124
document.createElement("div"));

web/views_manager.js

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,9 @@ const UI_NOTIFICATION_CLASS = "pdfSidebarNotification";
6969
class ViewsManager extends Sidebar {
7070
static #l10nDescription = null;
7171

72+
#hasAnimations = !window.matchMedia("(prefers-reduced-motion: reduce)")
73+
.matches;
74+
7275
/**
7376
* @param {PDFSidebarOptions} options
7477
*/
@@ -303,15 +306,20 @@ class ViewsManager extends Sidebar {
303306
toggleExpandedBtn(this.toggleButton, true);
304307
this.switchView(this.active);
305308

306-
// Changing `hidden` above may cause a reflow which would prevent the
307-
// CSS transition from being applied correctly, so we need to delay
308-
// adding the relevant CSS classes.
309-
queueMicrotask(() => {
310-
this.outerContainer.classList.add(
311-
"viewsManagerMoving",
312-
"viewsManagerOpen"
313-
);
314-
});
309+
if (this.#hasAnimations) {
310+
// Changing `hidden` above may cause a reflow which would prevent the
311+
// CSS transition from being applied correctly, so we need to delay
312+
// adding the relevant CSS classes.
313+
queueMicrotask(() => {
314+
this.outerContainer.classList.add(
315+
"viewsManagerMoving",
316+
"viewsManagerOpen"
317+
);
318+
});
319+
} else {
320+
this.outerContainer.classList.add("viewsManagerOpen");
321+
this.eventBus.dispatch("resize", { source: this });
322+
}
315323
if (this.active === SidebarView.THUMBS) {
316324
this.onUpdateThumbnails();
317325
}
@@ -392,13 +400,16 @@ class ViewsManager extends Sidebar {
392400
#addEventListeners() {
393401
const { eventBus, outerContainer } = this;
394402

395-
this.sidebarContainer.addEventListener("transitionend", evt => {
396-
if (evt.target === this.sidebarContainer) {
397-
outerContainer.classList.remove("viewsManagerMoving");
398-
// Ensure that rendering is triggered after opening/closing the sidebar.
399-
eventBus.dispatch("resize", { source: this });
400-
}
401-
});
403+
if (this.#hasAnimations) {
404+
this.sidebarContainer.addEventListener("transitionend", evt => {
405+
if (evt.target === this.sidebarContainer) {
406+
outerContainer.classList.remove("viewsManagerMoving");
407+
// Ensure that rendering is triggered after opening/closing the
408+
// sidebar.
409+
eventBus.dispatch("resize", { source: this });
410+
}
411+
});
412+
}
402413

403414
// Buttons for switching views.
404415
this.thumbnailButton.addEventListener("click", () => {

0 commit comments

Comments
 (0)