diff --git a/news/changelog-1.11.md b/news/changelog-1.11.md index 57eb1f4d8e7..9034dc3a090 100644 --- a/news/changelog-1.11.md +++ b/news/changelog-1.11.md @@ -8,6 +8,13 @@ All changes included in 1.11: - ([#14615](https://github.com/quarto-dev/quarto-cli/issues/14615)): Fix invalid `role="menu"` on the website navbar's collapse toggle button, flagged by axe-core (`aria-allowed-role`) and WAVE (`aria_menu_broken`) when the navbar collapses to the hamburger at narrow viewports. +## Formats + +### `dashboard` + +- ([#14818](https://github.com/quarto-dev/quarto-cli/issues/14818)): Fix a dashboard with more than one page going blank when the URL hash does not name a page, such as a footnote link or a cross-reference. Such a hash is now left alone, so the current page stays visible. +- ([#14819](https://github.com/quarto-dev/quarto-cli/issues/14819)): Fix keyboard focus starting inside the page content when a dashboard opens at a page hash, such as `dashboard.html#sales`. Tabbing forward could not reach the navbar. Focus now starts at the top of the document. + ## Engines ### `knitr` diff --git a/src/resources/formats/dashboard/quarto-dashboard.js b/src/resources/formats/dashboard/quarto-dashboard.js index 6b771b9bb76..cce09bb9f76 100644 --- a/src/resources/formats/dashboard/quarto-dashboard.js +++ b/src/resources/formats/dashboard/quarto-dashboard.js @@ -106,10 +106,17 @@ window.document.addEventListener("DOMContentLoaded", function (_event) { } } - // Try to process the hash and activate a tab + // Try to process the hash and activate a tab. A hash that does not name a + // page (an in-page anchor, a footnote link, a cross-reference) is left + // alone, so the browser can scroll to it and the current page stays visible. const hash = window.decodeURIComponent(window.location.hash); - if (hash.length > 0) { + if (QuartoDashboardUtils.isPage(hash)) { QuartoDashboardUtils.showPage(hash, () => { + // a page hash selects a page, it is not a position within one, so undo + // the browser's fragment behaviour of starting sequential focus inside + // the target. Without this, tabbing from a shared "...#page" URL starts + // inside the page content and never reaches the navbar. + QuartoDashboardUtils.resetFocusToDocumentStart(); window.document.documentElement.classList.remove("hidden"); }); } else { @@ -119,7 +126,11 @@ window.document.addEventListener("DOMContentLoaded", function (_event) { // navigate to a tab when the history changes window.addEventListener("popstate", function (e) { const hash = window.decodeURIComponent(window.location.hash); - QuartoDashboardUtils.showPage(hash); + // only switch pages for a hash that names one; any other hash change + // must leave the pages alone, otherwise showPage hides all of them + if (hash.length === 0 || QuartoDashboardUtils.isPage(hash)) { + QuartoDashboardUtils.showPage(hash); + } }); // Hook tabs and use that to update history / active tabs @@ -226,9 +237,40 @@ window.QuartoDashboardUtils = { window.scrollTo(0, 0); }, 10); }, + resetFocusToDocumentStart: function () { + // focusing the body moves the sequential focus navigation starting point + // back to the top of the document; the tabindex only needs to exist for + // the focus() call, so it is removed again immediately. + // this has to run after the browser has applied its own fragment + // behaviour, which happens after DOMContentLoaded, hence the deferral + const reset = function () { + const bodyEl = document.body; + bodyEl.setAttribute("tabindex", "-1"); + bodyEl.focus(); + bodyEl.removeAttribute("tabindex"); + }; + if (document.readyState === "complete") { + window.setTimeout(reset, 0); + } else { + window.addEventListener("load", function () { + window.setTimeout(reset, 0); + }); + } + }, isPage: function (hash) { - const tabPaneEl = document.querySelector(`.dashboard-page.tab-pane${hash}`); - return tabPaneEl !== null; + // use getElementById rather than building a selector: the hash comes from + // the URL and may not be a valid CSS selector (`#`, `#1foo` and `#a:b` all + // throw a SyntaxError in querySelector) + const id = hash.startsWith("#") ? hash.substring(1) : hash; + if (id === "") { + return false; + } + const tabPaneEl = document.getElementById(id); + return ( + tabPaneEl !== null && + tabPaneEl.classList.contains("dashboard-page") && + tabPaneEl.classList.contains("tab-pane") + ); }, showPage: function (hash, fnCallback) { // If the hash is empty, just select the first tab and activate that diff --git a/tests/docs/playwright/dashboard/hash-navigation.qmd b/tests/docs/playwright/dashboard/hash-navigation.qmd new file mode 100644 index 00000000000..f438da91a28 --- /dev/null +++ b/tests/docs/playwright/dashboard/hash-navigation.qmd @@ -0,0 +1,31 @@ +--- +title: "Hash navigation" +format: dashboard +--- + +# Heat + +## Row + +::: {.card title="Heat content"} +Content for the Heat page, with a footnote[^1] whose link is an in-page +anchor that names no page. + +[^1]: The footnote target is `#fn1`, not a dashboard page. + +```{=html} + +``` +::: + +# Cool + +## Row + +::: {.card title="Cool content"} +Content for the Cool page. + +```{=html} + +``` +::: diff --git a/tests/integration/playwright/tests/dashboard-hash-navigation.spec.ts b/tests/integration/playwright/tests/dashboard-hash-navigation.spec.ts new file mode 100644 index 00000000000..4401ab21fc6 --- /dev/null +++ b/tests/integration/playwright/tests/dashboard-hash-navigation.spec.ts @@ -0,0 +1,82 @@ +import { test, expect } from '@playwright/test'; + +const dashboard = '/dashboard/hash-navigation.html'; + +const heatPane = (page) => page.locator('#heat.dashboard-page'); +const coolPane = (page) => page.locator('#cool.dashboard-page'); + +// https://github.com/quarto-dev/quarto-cli/issues/14818 +test.describe('a hash that names no page leaves the pages alone', () => { + test('on load', async ({ page }) => { + await page.goto(`${dashboard}#no-such-thing`); + await expect(heatPane(page)).toHaveClass(/active/); + await expect(coolPane(page)).not.toHaveClass(/active/); + await expect(page.locator('html')).not.toHaveClass(/hidden/); + }); + + test('on a hash change', async ({ page }) => { + await page.goto(dashboard); + await expect(heatPane(page)).toHaveClass(/active/); + // an in-page anchor that exists but is not a page, such as a footnote + await page.evaluate(() => { window.location.hash = '#fn1'; }); + await expect(heatPane(page)).toHaveClass(/active/); + }); + + // isPage used to build a selector from the hash, which throws for a hash + // that is not a valid CSS selector and left the whole dashboard hidden + test('when the hash is not a valid CSS selector', async ({ page }) => { + await page.goto(`${dashboard}#1foo`); + await expect(page.locator('html')).not.toHaveClass(/hidden/); + await expect(heatPane(page)).toHaveClass(/active/); + }); +}); + +test('page navigation and history still work', async ({ page }) => { + await page.goto(dashboard); + await expect(heatPane(page)).toHaveClass(/active/); + + await page.locator('.navbar .nav-link[data-bs-target="#cool"]').click(); + await expect(coolPane(page)).toHaveClass(/active/); + await expect(heatPane(page)).not.toHaveClass(/active/); + + await page.goBack(); + await expect(heatPane(page)).toHaveClass(/active/); + + await page.goForward(); + await expect(coolPane(page)).toHaveClass(/active/); +}); + +// https://github.com/quarto-dev/quarto-cli/issues/14819 +test('landing on a page hash starts tabbing at the top of the document', async ({ page, browserName }) => { + // WebKit only tabs to links when the Alt modifier is held, which matches + // Safari's default "Press Tab to highlight each item on a webpage" setting + const tabKey = browserName === 'webkit' ? 'Alt+Tab' : 'Tab'; + + await page.goto(`${dashboard}#cool`); + await expect(coolPane(page)).toHaveClass(/active/); + + // the browser would otherwise start sequential focus inside the target + // pane, which puts the navbar out of reach of a forward Tab + await page.keyboard.press(tabKey); + const firstStop = await page.evaluate(() => ({ + isBody: document.activeElement === document.body, + insideAPage: document.activeElement?.closest('.dashboard-page') !== null, + })); + expect(firstStop.isBody).toBe(false); + expect(firstStop.insideAPage).toBe(false); + + // the navbar tab for the current page is reachable by tabbing forward. + // the exact number of stops before it depends on what else the format + // puts at the top of the body, so tab forward a bounded number of times + // rather than assuming a fixed position. + let reachedNavbarTab = false; + for (let i = 0; i < 5 && !reachedNavbarTab; i++) { + reachedNavbarTab = await page.evaluate( + () => document.activeElement?.id === 'tab-cool' + ); + if (!reachedNavbarTab) { + await page.keyboard.press(tabKey); + } + } + expect(reachedNavbarTab).toBe(true); +});