Fix website listings stacking update handlers on hash change

news/changelog-1.11.md

@@ -140,6 +140,7 @@ All changes included in 1.11:
 - ([#15091](https://github.com/quarto-dev/quarto-cli/pull/15091)): Fix page-level `bread-crumbs`, `back-to-top-navigation`, `page-navigation`, `margin-header` and `margin-footer` options not being validated or offered for completion in editors. A page value of the wrong type now fails to render with an error instead of being accepted silently.
 - ([#15091](https://github.com/quarto-dev/quarto-cli/pull/15091)): Fix `open-graph: false` or `twitter-card: false` in page front matter or `_metadata.yml` not disabling Open Graph or Twitter Card metadata for that page when the website enables it.
 - ([#15114](https://github.com/quarto-dev/quarto-cli/pull/15114)): Fix a website category containing backticks showing broken text in the page title block, and a category containing a backslash, such as a Windows path, failing the render or losing its backslashes.
+- ([#15206](https://github.com/quarto-dev/quarto-cli/issues/15206)): Fix website listings attaching another update handler on every URL hash change, and an extra one per additional listing on the same page.
 
 ## Lua API
 

src/project/types/website/listing/website-listing-template.ts

@@ -575,12 +575,6 @@ export function templateJsScript(
       window['quarto-listing-loaded']();
     }
   });
-
-  window.addEventListener('hashchange',() => {
-    if (window['quarto-listing-loaded']) {
-      window['quarto-listing-loaded']();
-    }
-  })
   `;
   return jsScript;
 }

src/resources/projects/website/listing/quarto-listing.js

@@ -10,29 +10,41 @@ window.quartoListingCategory = (category) => {
   }
 };
 
-window["quarto-listing-loaded"] = () => {
-  // Process any existing hash
+const wiredListings = new Set();
+
+// Apply the category and per-listing page stored in the URL hash
+function applyListingHash() {
   const hash = getHash();
+  if (!hash || !window["quarto-listings"]) {
+    return;
+  }
 
-  if (hash) {
-    // If there is a category, switch to that
-    if (hash.category) {
-      // category hash are URI encoded so we need to decode it before processing
-      // so that we can match it with the category element processed in JS
-      activateCategory(decodeURIComponent(hash.category));
-    }
-    // Paginate a specific listing
-    const listingIds = Object.keys(window["quarto-listings"]);
-    for (const listingId of listingIds) {
-      const page = hash[getListingPageKey(listingId)];
-      if (page) {
-        showPage(listingId, page);
-      }
+  // If there is a category, switch to that
+  if (hash.category) {
+    // category hash are URI encoded so we need to decode it before processing
+    // so that we can match it with the category element processed in JS
+    activateCategory(decodeURIComponent(hash.category));
+  }
+  // Paginate a specific listing
+  for (const listingId of Object.keys(window["quarto-listings"])) {
+    const page = hash[getListingPageKey(listingId)];
+    if (page) {
+      showPage(listingId, page);
     }
   }
+}
+
+window["quarto-listing-loaded"] = () => {
+  // Process any existing hash
+  applyListingHash();
+
+  // Each listing calls this once its List is created, so wire each one once
+  for (const listingId of Object.keys(window["quarto-listings"])) {
+    if (wiredListings.has(listingId)) {
+      continue;
+    }
+    wiredListings.add(listingId);
 
-  const listingIds = Object.keys(window["quarto-listings"]);
-  for (const listingId of listingIds) {
     // The actual list
     const list = window["quarto-listings"][listingId];
 
@@ -56,6 +68,8 @@ window["quarto-listing-loaded"] = () => {
   }
 };
 
+window.addEventListener("hashchange", applyListingHash);
+
 window.document.addEventListener("DOMContentLoaded", function (_event) {
   // Attach click handlers to categories
   const categoryEls = window.document.querySelectorAll(

tests/docs/playwright/listings/interactions/.gitignore

@@ -0,0 +1,3 @@
+/.quarto/
+**/*.quarto_ipynb
+/_site/

tests/docs/playwright/listings/interactions/_quarto.yml

@@ -0,0 +1,7 @@
+project:
+  type: website
+
+website:
+  title: "Listing update handlers"
+
+format: html

tests/docs/playwright/listings/interactions/index.qmd

@@ -0,0 +1,22 @@
+---
+title: "Two paginated listings"
+listing:
+  - id: first-listing
+    contents: posts
+    type: default
+    sort: "title asc"
+    page-size: 1
+    categories: true
+    filter-ui: true
+  - id: second-listing
+    contents: posts
+    type: default
+    sort: "title asc"
+    page-size: 1
+---
+
+::: {#first-listing}
+:::
+
+::: {#second-listing}
+:::

tests/docs/playwright/listings/interactions/posts/post-1.qmd

@@ -0,0 +1,7 @@
+---
+title: "Post 1"
+categories: [cat1]
+image: thumb.png
+---
+
+Post 1 content.

tests/docs/playwright/listings/interactions/posts/post-2.qmd

@@ -0,0 +1,7 @@
+---
+title: "Post 2"
+categories: [cat2]
+image: thumb.png
+---
+
+Post 2 content.

tests/docs/playwright/listings/interactions/posts/post-3.qmd

@@ -0,0 +1,7 @@
+---
+title: "Post 3"
+categories: [cat3]
+image: thumb.png
+---
+
+Post 3 content.

tests/integration/playwright/tests/listing-interactions.spec.ts

@@ -0,0 +1,130 @@
+import { expect, Page, test } from "@playwright/test";
+
+const fixture = "./listings/interactions/_site/index.html";
+const listingIds = ["first-listing", "second-listing"];
+
+const titles = (page: Page, listingId: string) =>
+  page.locator(`#listing-${listingId} .list .listing-title`);
+
+const pageLink = (page: Page, listingId: string, n: string) =>
+  page.locator(`#${listingId}-pagination .page.page-link`, { hasText: n });
+
+const categoryEl = (page: Page, name: string) =>
+  page.locator(".quarto-listing-category .category", { hasText: name });
+
+async function changeHash(page: Page, hash: string) {
+  await page.evaluate((hash) =>
+    new Promise<void>((resolve) => {
+      window.addEventListener("hashchange", () => resolve(), { once: true });
+      window.location.hash = hash;
+    }), hash);
+}
+
+test.describe("Listing categories", () => {
+  test("category title click resets the filter", async ({ page }) => {
+    await page.goto(fixture);
+    await categoryEl(page, "cat3").click();
+    await expect(titles(page, "first-listing")).toHaveText(["Post 3"]);
+
+    await page.locator(".quarto-listing-category-title").click();
+
+    await expect(titles(page, "first-listing")).toHaveText(["Post 1"]);
+    await expect(page.locator("#first-listing-pagination .page.page-link")).toHaveCount(3);
+    await expect(page.locator(".quarto-listing-category .category.active"))
+      .toHaveAttribute("data-category", "");
+  });
+
+  test("hash change to a category filters the listings", async ({ page }) => {
+    await page.goto(fixture);
+
+    await changeHash(page, "category=cat3");
+
+    for (const listingId of listingIds) {
+      await expect(titles(page, listingId)).toHaveText(["Post 3"]);
+    }
+  });
+
+  test("back and forward restore the category", async ({ page }) => {
+    await page.goto(fixture);
+    await categoryEl(page, "cat3").click();
+    await expect(titles(page, "first-listing")).toHaveText(["Post 3"]);
+    await categoryEl(page, "cat1").click();
+    await expect(titles(page, "first-listing")).toHaveText(["Post 1"]);
+
+    await page.goBack();
+    await expect(titles(page, "first-listing")).toHaveText(["Post 3"]);
+    await expect(categoryEl(page, "cat3")).toHaveClass(/active/);
+
+    await page.goForward();
+    await expect(titles(page, "first-listing")).toHaveText(["Post 1"]);
+    await expect(categoryEl(page, "cat1")).toHaveClass(/active/);
+  });
+});
+
+test.describe("Listing pagination", () => {
+  test("page hash on initial load shows that page of that listing only", async ({ page }) => {
+    await page.goto(`${fixture}#listing-second-listing-page=2`);
+
+    await expect(titles(page, "second-listing")).toHaveText(["Post 2"]);
+    await expect(titles(page, "first-listing")).toHaveText(["Post 1"]);
+  });
+
+  test("page links rebuilt after an update still write the page hash", async ({ page }) => {
+    await page.goto(fixture);
+    await pageLink(page, "second-listing", "2").click();
+    await expect(titles(page, "second-listing")).toHaveText(["Post 2"]);
+
+    await pageLink(page, "second-listing", "3").click();
+
+    await expect(titles(page, "second-listing")).toHaveText(["Post 3"]);
+    await expect(page).toHaveURL(/#listing-second-listing-page=3$/);
+  });
+
+  test("back restores the previous page", async ({ page }) => {
+    await page.goto(fixture);
+    await pageLink(page, "second-listing", "2").click();
+    await expect(titles(page, "second-listing")).toHaveText(["Post 2"]);
+    await pageLink(page, "second-listing", "3").click();
+    await expect(titles(page, "second-listing")).toHaveText(["Post 3"]);
+
+    await page.goBack();
+
+    await expect(titles(page, "second-listing")).toHaveText(["Post 2"]);
+  });
+});
+
+test.describe("Listing update side effects", () => {
+  test("progressive image gets its src once its page is shown", async ({ page }) => {
+    await page.goto(fixture);
+    // list.js keeps hidden items out of the DOM; read Post 2 through its item.
+    const hiddenImageAttrs = await page.evaluate(() => {
+      const list = (window as any)["quarto-listings"]["listing-second-listing"];
+      const img = list.items[1].elm.querySelector("img.thumbnail-image");
+      return { src: img.getAttribute("src"), dataSrc: img.getAttribute("data-src") };
+    });
+    expect(hiddenImageAttrs.src).toBeNull();
+    expect(hiddenImageAttrs.dataSrc).toMatch(/thumb\.png$/);
+
+    const image = page.locator("#listing-second-listing .list img.thumbnail-image");
+    await pageLink(page, "second-listing", "2").click();
+
+    await expect(titles(page, "second-listing")).toHaveText(["Post 2"]);
+    await expect(image).toHaveAttribute("src", /thumb\.png$/);
+    await expect(image).not.toHaveAttribute("data-src");
+  });
+
+  test("no matching message follows the filter", async ({ page }) => {
+    await page.goto(fixture);
+    const noMatching = page.locator("#listing-first-listing .listing-no-matching");
+    const filter = page.locator("#listing-first-listing").getByPlaceholder("Filter");
+    await expect(noMatching).toHaveClass(/d-none/);
+
+    // list.js searches on keyup, which fill() alone does not send.
+    await filter.fill("nothing matches this");
+    await filter.press("Enter");
+    await expect(noMatching).not.toHaveClass(/d-none/);
+
+    await filter.fill("");
+    await expect(noMatching).toHaveClass(/d-none/);
+  });
+});

tests/integration/playwright/tests/listing-update-handlers.spec.ts

@@ -0,0 +1,60 @@
+import { expect, Page, test } from "@playwright/test";
+
+const fixture = "./listings/interactions/_site/index.html";
+const listingIds = ["first-listing", "second-listing"];
+
+async function changeHash(page: Page, hash: string) {
+  await page.evaluate((hash) =>
+    new Promise<void>((resolve) => {
+      window.addEventListener("hashchange", () => resolve(), { once: true });
+      window.location.hash = hash;
+    }), hash);
+}
+
+// The update handler calls window.quartoDecorateExternalLinks with the
+// listing's list element; counting those calls per listing counts the runs.
+async function countUpdateHandlerRuns(page: Page) {
+  await page.evaluate(() => {
+    const w = window as any;
+    const decorate = w.quartoDecorateExternalLinks;
+    w.updateRuns = {};
+    w.quartoDecorateExternalLinks = (root: Element) => {
+      const id = root?.closest?.(".quarto-listing")?.id;
+      if (id) w.updateRuns[id] = (w.updateRuns[id] ?? 0) + 1;
+      return decorate(root);
+    };
+  });
+}
+
+// A pagination click updates the list twice (list.js and Quarto both handle
+// it), so trigger exactly one update through the List.js API instead.
+async function updateRunsForOneUpdate(page: Page, listingId: string) {
+  return page.evaluate((id) => {
+    const w = window as any;
+    w.updateRuns = {};
+    w["quarto-listings"][id].update();
+    return w.updateRuns[id];
+  }, `listing-${listingId}`);
+}
+
+test.describe("Listing update handlers", () => {
+  for (const listingId of listingIds) {
+    test(`${listingId} runs its update handler once per update`, async ({ page }) => {
+      await page.goto(fixture);
+      await countUpdateHandlerRuns(page);
+
+      expect(await updateRunsForOneUpdate(page, listingId)).toBe(1);
+    });
+
+    test(`${listingId} keeps one update handler across hash changes`, async ({ page }) => {
+      await page.goto(fixture);
+      await countUpdateHandlerRuns(page);
+
+      for (const hash of ["one", "two", "three"]) {
+        await changeHash(page, hash);
+      }
+
+      expect(await updateRunsForOneUpdate(page, listingId)).toBe(1);
+    });
+  }
+});