Merge pull request #20731 from calixteman/remove_page_id

Remove useless page-id attribute in thumbnails

test/integration/reorganize_pages_spec.mjs

@@ -33,7 +33,19 @@ import {
 } from "./test_utils.mjs";
 
 async function waitForThumbnailVisible(page, pageNums) {
+  const hasAnimations = await page.evaluate(
+    () => !window.matchMedia("(prefers-reduced-motion: reduce)").matches
+  );
   await page.click("#viewsManagerToggleButton");
+  if (hasAnimations) {
+    await page.waitForSelector("#outerContainer.viewsManagerMoving", {
+      visible: true,
+    });
+  }
+  await page.waitForSelector(
+    "#outerContainer:not(.viewsManagerMoving).viewsManagerOpen",
+    { visible: true }
+  );
 
   const thumbSelector = "#thumbnailsView .thumbnailImageContainer > img";
   await page.waitForSelector(thumbSelector, { visible: true });
@@ -283,12 +295,12 @@ describe("Reorganize Pages View", () => {
           await waitForThumbnailVisible(page, 1);
           const rect1 = await getRect(page, getThumbnailSelector(1));
           const rect2 = await getRect(page, getThumbnailSelector(2));
-          await (await page.$(".thumbnail[page-id='14'")).scrollIntoView();
+          await (await page.$(".thumbnail[page-number='14'")).scrollIntoView();
           await page.waitForSelector(getThumbnailSelector(14), {
             visible: true,
           });
           await page.click(`.thumbnail:has(${getThumbnailSelector(14)}) input`);
-          await (await page.$(".thumbnail[page-id='1'")).scrollIntoView();
+          await (await page.$(".thumbnail[page-number='1'")).scrollIntoView();
           await page.waitForSelector(getThumbnailSelector(1), {
             visible: true,
           });

test/integration/thumbnail_view_spec.mjs

@@ -29,6 +29,23 @@ async function waitForMenu(page, buttonSelector, visible = true) {
   );
 }
 
+async function showViewsManager(page) {
+  const hasAnimations = await page.evaluate(
+    () => !window.matchMedia("(prefers-reduced-motion: reduce)").matches
+  );
+  await page.click("#viewsManagerToggleButton");
+  if (hasAnimations) {
+    await page.waitForSelector("#outerContainer.viewsManagerMoving", {
+      visible: true,
+    });
+  }
+  await page.waitForSelector("#viewsManager", { visible: true });
+  await page.waitForSelector(
+    "#outerContainer:not(.viewsManagerMoving).viewsManagerOpen",
+    { visible: true }
+  );
+}
+
 describe("PDF Thumbnail View", () => {
   describe("Works without errors", () => {
     let pages;
@@ -44,7 +61,7 @@ describe("PDF Thumbnail View", () => {
     it("should render thumbnails without errors", async () => {
       await Promise.all(
         pages.map(async ([browserName, page]) => {
-          await page.click("#viewsManagerToggleButton");
+          await showViewsManager(page);
 
           const thumbSelector =
             "#thumbnailsView .thumbnailImageContainer > img";
@@ -62,7 +79,7 @@ describe("PDF Thumbnail View", () => {
     it("should have accessible label on resizer", async () => {
       await Promise.all(
         pages.map(async ([browserName, page]) => {
-          await page.click("#viewsManagerToggleButton");
+          await showViewsManager(page);
 
           const ariaLabel = await page.$eval("#viewsManagerResizer", el =>
             el.getAttribute("aria-label")
@@ -104,8 +121,7 @@ describe("PDF Thumbnail View", () => {
     it("should scroll the view", async () => {
       await Promise.all(
         pages.map(async ([browserName, page]) => {
-          await page.click("#viewsManagerToggleButton");
+          await showViewsManager(page);
-
           await waitForThumbnailVisible(page, 1);
 
           for (const pageNum of [14, 1, 13, 2]) {
@@ -141,8 +157,7 @@ describe("PDF Thumbnail View", () => {
     it("should navigate with the keyboard", async () => {
       await Promise.all(
         pages.map(async ([browserName, page]) => {
-          await page.click("#viewsManagerToggleButton");
+          await showViewsManager(page);
-
           await waitForThumbnailVisible(page, 1);
           await waitForThumbnailVisible(page, 2);
           await waitForThumbnailVisible(page, 3);
@@ -235,7 +250,7 @@ describe("PDF Thumbnail View", () => {
     it("should open with Enter key and remain open", async () => {
       await Promise.all(
         pages.map(async ([browserName, page]) => {
-          await page.click("#viewsManagerToggleButton");
+          await showViewsManager(page);
           await waitForThumbnailVisible(page, 1);
 
           await enableMenuItems(page);
@@ -267,7 +282,7 @@ describe("PDF Thumbnail View", () => {
     it("should open with Space key and remain open", async () => {
       await Promise.all(
         pages.map(async ([browserName, page]) => {
-          await page.click("#viewsManagerToggleButton");
+          await showViewsManager(page);
           await waitForThumbnailVisible(page, 1);
 
           await enableMenuItems(page);
@@ -326,8 +341,7 @@ describe("PDF Thumbnail View", () => {
     it("should have accessible label on checkbox", async () => {
       await Promise.all(
         pages.map(async ([browserName, page]) => {
-          await page.click("#viewsManagerToggleButton");
+          await showViewsManager(page);
-
           await waitForThumbnailVisible(page, 1);
 
           const ariaLabel = await page.$eval(
@@ -362,7 +376,7 @@ describe("PDF Thumbnail View", () => {
     it("must navigate menus with ArrowDown and Tab keys", async () => {
       await Promise.all(
         pages.map(async ([browserName, page]) => {
-          await page.click("#viewsManagerToggleButton");
+          await showViewsManager(page);
           await waitForThumbnailVisible(page, 1);
 
           // Focus the views manager selector button
@@ -431,7 +445,7 @@ describe("PDF Thumbnail View", () => {
     it("should show the manage button in thumbnail view and hide it in outline view", async () => {
       await Promise.all(
         pages.map(async ([browserName, page]) => {
-          await page.click("#viewsManagerToggleButton");
+          await showViewsManager(page);
           await waitForThumbnailVisible(page, 1);
 
           // The status bar (Select pages + Manage button) must be visible in
@@ -471,8 +485,7 @@ describe("PDF Thumbnail View", () => {
     it("should focus checkboxes with Tab key", async () => {
       await Promise.all(
         pages.map(async ([browserName, page]) => {
-          await page.click("#viewsManagerToggleButton");
+          await showViewsManager(page);
-
           await waitForThumbnailVisible(page, 1);
 
           // Focus the first thumbnail button
@@ -499,8 +512,7 @@ describe("PDF Thumbnail View", () => {
     it("should navigate checkboxes with arrow keys", async () => {
       await Promise.all(
         pages.map(async ([browserName, page]) => {
-          await page.click("#viewsManagerToggleButton");
+          await showViewsManager(page);
-
           await waitForThumbnailVisible(page, 1);
           await waitForThumbnailVisible(page, 2);
 

web/pdf_thumbnail_view.js

@@ -119,7 +119,6 @@ class PDFThumbnailView extends RenderableView {
     const thumbnailContainer = (this.div = document.createElement("div"));
     thumbnailContainer.className = "thumbnail";
     thumbnailContainer.setAttribute("page-number", id);
-    thumbnailContainer.setAttribute("page-id", id);
 
     const imageContainer = (this.imageContainer =
       document.createElement("div"));

web/views_manager.js

@@ -69,6 +69,9 @@ const UI_NOTIFICATION_CLASS = "pdfSidebarNotification";
 class ViewsManager extends Sidebar {
   static #l10nDescription = null;
 
+  #hasAnimations = !window.matchMedia("(prefers-reduced-motion: reduce)")
+    .matches;
+
   /**
    * @param {PDFSidebarOptions} options
    */
@@ -303,15 +306,20 @@ class ViewsManager extends Sidebar {
     toggleExpandedBtn(this.toggleButton, true);
     this.switchView(this.active);
 
-    // Changing `hidden` above may cause a reflow which would prevent the
+    if (this.#hasAnimations) {
-    // CSS transition from being applied correctly, so we need to delay
+      // Changing `hidden` above may cause a reflow which would prevent the
-    // adding the relevant CSS classes.
+      // CSS transition from being applied correctly, so we need to delay
-    queueMicrotask(() => {
+      // adding the relevant CSS classes.
-      this.outerContainer.classList.add(
+      queueMicrotask(() => {
-        "viewsManagerMoving",
+        this.outerContainer.classList.add(
-        "viewsManagerOpen"
+          "viewsManagerMoving",
-      );
+          "viewsManagerOpen"
-    });
+        );
+      });
+    } else {
+      this.outerContainer.classList.add("viewsManagerOpen");
+      this.eventBus.dispatch("resize", { source: this });
+    }
     if (this.active === SidebarView.THUMBS) {
       this.onUpdateThumbnails();
     }
@@ -392,13 +400,16 @@ class ViewsManager extends Sidebar {
   #addEventListeners() {
     const { eventBus, outerContainer } = this;
 
-    this.sidebarContainer.addEventListener("transitionend", evt => {
+    if (this.#hasAnimations) {
-      if (evt.target === this.sidebarContainer) {
+      this.sidebarContainer.addEventListener("transitionend", evt => {
-        outerContainer.classList.remove("viewsManagerMoving");
+        if (evt.target === this.sidebarContainer) {
-        // Ensure that rendering is triggered after opening/closing the sidebar.
+          outerContainer.classList.remove("viewsManagerMoving");
-        eventBus.dispatch("resize", { source: this });
+          // Ensure that rendering is triggered after opening/closing the
-      }
+          // sidebar.
-    });
+          eventBus.dispatch("resize", { source: this });
+        }
+      });
+    }
 
     // Buttons for switching views.
     this.thumbnailButton.addEventListener("click", () => {