Fix axe accessibility checks for revealjs and dashboard formats#14125
Merged
Fix axe accessibility checks for revealjs and dashboard formats#14125
Conversation
Add !default fallback values for SCSS variables used by axe styling. For HTML+Bootstrap, these are overridden by the framework layer. For revealjs, these provide working defaults since sass-bundles compile separately from the revealjs theme. - src/format/html/format-html-axe.ts: Add -color, -color defaults - tests/docs/smoke-all/accessibility/: Add axe-html.qmd, axe-revealjs.qmd - tests/docs/playwright/: Add axe test documents - tests/integration/playwright/tests/: Add axe playwright specs
Test all axe output modes (console, json, document) across html, revealjs, and dashboard formats. HTML passes all modes. Revealjs tests are marked as expected-fail because axe-check.js is bundled in bootstrap-only quarto.js and never loads (#13781). Dashboard document mode is also expected-fail because the format has no <main> element for the document reporter. Add data-quarto-axe-complete attribute to axe-check.js as a deterministic completion signal for playwright tests. Consolidate 7 individual spec files into a single parameterized axe-accessibility.spec.ts following the html-math-katex.spec.ts pattern.
All current reporters are synchronous, but awaiting is free and prevents the completion signal from firing early if a reporter becomes async in the future.
axe accessibility checking only worked for bootstrap HTML because axe-check.js was imported by quarto.js, which only loads for bootstrap. Decouple axe-check.js from quarto.js so it loads independently via FormatDependency for all HTML formats (html, revealjs, dashboard). - Remove axe import and init() call from quarto.js - Add self-initialization to axe-check.js (runs when loaded as module) - Add FormatDependency in axeFormatDependencies to load axe-check.js as type="module" script when axe config is set - Fix document reporter to fall back to document.body when no <main> element exists (revealjs/dashboard layouts) - Update playwright tests: remove test.fail() markers, assert specific violations per format (color-contrast for html/dashboard, link-name for revealjs where CSS transforms prevent contrast detection)
Add negative tests verifying axe dependencies are absent when axe is not configured or explicitly disabled (axe: false). Fix CSS selector in all axe smoke-all tests to use script[src*="axe-check"] instead of script[src*="axe"] to avoid false matches on output filenames. Regenerate esbuild-analysis-cache.json to reflect removal of axe-check.js import from quarto.js (stale cache was causing unconditional inclusion).
Prevent double initialization if axe-check.js is imported after self-initializing. Remove unused export. Replace inline conditionals in playwright tests with a violation text lookup map for clarity.
The axe document reporter overlay was unreadable: no z-index (hidden behind RevealJS UI and dashboard cards), transparent background (text invisible on dark themes), and no height constraint (overflows viewport). Uses var(--r-css-prop, $sass-variable) pattern to bridge RevealJS (CSS custom properties from exposer.scss) and Bootstrap (Sass-compiled theme values) theming systems. Adds Playwright tests for CSS properties and a RevealJS dark theme test case to verify the cross-format theming bridge. Updates the dashboard test doc with a multi-card layout using value boxes and cards.
Rules file auto-loads when touching Sass-related files and flags the key constraint: RevealJS sass-bundles compile separately from the theme. llm-docs reference covers the full compilation pipeline, CSS custom property bridge, and debugging techniques.
Adds format-specific assertion for revealjs-dark test case: background must not be rgb(255, 255, 255), ensuring the CSS custom property bridge resolves to the theme color instead of the compile-time Sass fallback.
RevealJS document mode now expects the axe report inside a section.quarto-axe-report-slide with scrollable class and static positioning, rather than a fixed overlay with z-index: 9999. HTML/dashboard document mode assertions unchanged.
In RevealJS, the axe accessibility report now appears as a scrollable slide appended to the end of the deck instead of a fixed overlay. Hover-to-highlight uses Reveal.slide() to navigate to the offending slide rather than scrollIntoView(). Runtime detection (typeof Reveal !== 'undefined') selects the path; HTML and dashboard formats continue using the fixed overlay unchanged. After Reveal.sync(), re-navigate to the current slide to trigger visibility class assignment on the new slide (sync alone doesn't call updateSlides which sets past/present/future classes). Also fix hover highlight border to use red instead of $body-color, which was invisible on dark RevealJS themes.
RevealJS sets hidden/aria-hidden on non-visible slides, causing axe to only check the first slide. Temporarily remove these attributes before axe.run() and restore them after, so all slides get checked. Change violation target interaction from hover-to-navigate (jarring) to click-to-navigate for RevealJS. HTML/dashboard keep hover behavior. Add slide class to report section for scrollable CSS compatibility. Reduce font size on report slide for readability. Add playwright tests for cross-slide scanning (image-alt detected), presentation state restoration, and click-to-navigate behavior.
Move <img> from Slide 1 to Slide 2 in revealjs test docs so the
cross-slide scanning test validates that revealUnhideSlides() actually
works (previously the img was on the visible slide, passing without it).
Add scrollIntoView fallback in navigateToElement when closest("section")
returns null. Remove redundant appendChild call.
…fety Make report() return a Promise for the deferred Reveal path so data-quarto-axe-complete is only set after the report slide exists. Unhide all section elements (not just top-level) before axe.run() so vertical slides are also scanned. RevealJS manages hidden/aria-hidden on nested sections independently from their parents. Wrap axe.run() in try/finally to restore slide state if axe throws. Add tests for completion signal invariant and vertical slide scanning.
Replace textContent() + toContain() with toContainText() for auto-retrying assertions. Replace page.evaluate() DOM check with toBeAttached(). Replace page.waitForSelector with locator.waitFor(). Replace evaluate(getComputedStyle) with not.toHaveCSS(). Add descriptive message to console output assertion.
Dashboards now display the axe accessibility report in an offcanvas sidebar instead of a fixed overlay that covered dashboard cards. The offcanvas uses backdrop:false and scroll:true so dashboard content stays interactive and hover-to-highlight works. A floating toggle button allows closing and reopening the report panel.
Toggle button now has aria-label for screen readers, and .quarto-axe-toggle CSS is scoped under .quarto-dashboard for consistency with the offcanvas styles.
Deduplicate the interaction logic in createViolationElement — both the RevealJS click path and the HTML/Dashboard hover path shared querySelector + navigateToElement + classList add/remove. Now uses highlightTarget() and unhighlightTarget() helpers.
Match the pattern used for revealjsRules — only emit dashboard- specific CSS when rendering a dashboard format, not for all HTML formats. Uses format.identifier["base-format"] since dashboards use pandoc.to="html" (not "dashboard").
Document that callers must pass format.identifier["base-format"], not format.pandoc.to, since dashboards use the HTML pandoc writer.
When dashboard content visibility changes (page switch, card tabset, sidebar toggle, or browser back/forward), the axe report becomes stale. Listen for shown.bs.tab, bslib.sidebar, and popstate events to re-run axe.run() and replace the offcanvas body with updated results. A generation counter discards stale scan results when the user acts faster than axe completes.
Page 2 now contains a card tabset with a low-contrast violation in Tab A and clean content in Tab B. Tests verify that switching card tabs within a page triggers a rescan and correctly hides/shows violations for the active tab.
Replace CSS attribute selectors with getByRole() for page tabs, card tabset tabs, and sidebar toggle button. More readable and resilient to markup changes.
Wrap the scan in try/catch/finally so a failed axe.run() shows an error message instead of leaving "Scanning..." stuck forever. The completion attribute is always set in finally (guarded by generation counter) to unblock any waiters.
Three fixes from code review:
1. Wrap init() in try/catch/finally so data-quarto-axe-complete is always
set even if CDN import or initial scan fails, preventing Playwright
test hangs.
2. Move nodeElement creation inside the violation.nodes loop so each node
gets its own .quarto-axe-violation-selector div. Previously all targets
shared one div and event listeners accumulated across nodes.
3. Normalize `axe: true` to `{output: "console"}` in the QuartoAxeChecker
constructor, eliminating boolean-vs-object checks in init() and
setupDashboardRescan().
Also adds gitignore for smoke-all/dashboard render artifacts and
axe accessibility architecture doc in llm-docs.
Replaces manual temp file creation with FormatDependency head field for injecting axe configuration script tag. Eliminates unnecessary temp file handling and leverages existing dependency injection system. Changes: - Extract axeHtmlDependency() function returning FormatDependency with head field - Remove temp parameter from axeFormatDependencies() - Remove kIncludeInHeader usage in favor of head field - Update call site to remove temp argument The dependency system processes head content automatically, collecting all HTML into a single temp file during processHtmlDependencies(). Output is functionally identical.
Collaborator
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Fills test coverage gaps identified in PR #14125 review: - Dashboard console/json output modes (3 test cases) - Dashboard dark theme CSS custom property handling - HTML hover interaction and element highlighting - Negative tests for revealjs/dashboard (4 smoke-all tests) Updates hover interaction tests to use Playwright best practices: - Use .toHaveClass() instead of string interpolation - Use .first() to handle non-unique selectors (e.g., "span") All tests passing (75 Playwright, 9 smoke-all) across all browsers.
Collaborator
Author
|
Filled test coverage gaps from review feedback (beads
Also updated hover tests to use All tests passing: 75 Playwright (3 browsers), 9 smoke-all. |
Add explanatory comments to hover interaction tests: - Explain .first() usage (axe-core may produce non-unique selectors) - Clarify mouse.move(0,0) intent (clear hover state) Tests verify integration (hover triggers highlight) not selector uniqueness.
Add llm-docs/playwright-best-practices.md with patterns for writing reliable Playwright tests, derived from comprehensive axe accessibility test development (PR #14125). Covers web-first assertions, role-based selectors, handling non-unique selectors, completion signals, and parameterized testing. Refactor .claude/rules/testing/playwright-tests.md to brief reference format with link to detailed documentation, reducing auto-loaded context while preserving knowledge for explicit access.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adding
axe: trueto revealjs format fails with SCSS compilation error (Undefined variable: $body-color). Additionally, axe checks never run for revealjs or dashboard formats even when the SCSS issue is worked around.Root Cause
Two issues in the original axe implementation (#12730):
axe-check.jswas imported byquarto.js, which only loads for bootstrap-based HTML. RevealJS and dashboard formats never loadquarto.js, so axe never initializes.Axe SCSS uses Bootstrap variables (
$body-color,$body-bg) that don't exist in the revealjs Sass compilation context, which compiles separately from the theme.Fix
Phase 1: Test coverage (TDD)
Wrote comprehensive tests first to confirm the broken state. All axe output modes (console, json, document) tested across html, revealjs, and dashboard. HTML passed; revealjs and dashboard failed as expected.
Phase 2: Core loading fix
Decoupled axe from the bootstrap pipeline:
axe-check.jsimport fromquarto.jsaxe-check.js(runs when loaded as module)FormatDependencyinformat-html-axe.tsto loadaxe-check.jsastype="module"when axe config is present!defaultSCSS fallback values with CSS custom property bridge (var(--r-main-color, $body-color)) for cross-format themingThis fixed axe loading for all formats, but revealed that the document reporter assumes a
<main>element exists — which neither revealjs nor dashboard have.Phase 3: Format-specific report placement
Rather than a simple DOM fallback, each format got appropriate UX:
Reveal.slide()to navigate to the offending slide. All slides (including hidden ones) are temporarily unhidden for scanning since RevealJS setshidden/aria-hiddenon non-visible slides.backdrop: false,scroll: true) so content stays interactive. Floating toggle button to open/close. Automatically rescans when switching pages, card tabsets, or toggling the sidebar — dashboard hides inactive content withdisplay: none, making axe results stale on navigation.Phase 4: Hardening and cleanup
From code review: wrapped
init()in try/catch/finally sodata-quarto-axe-completeis always set (prevents Playwright hangs on CDN failure), fixed nodeElement creation inside the violation nodes loop (was sharing one div across all nodes), normalizedaxe: trueto{output: "console"}at construction time.Refactored axe options injection to use
FormatDependencyhead field instead of temp file +kIncludeInHeader, leveraging the existing dependency system.Test Coverage
axe-accessibility.spec.ts): all output modes across html, revealjs (light/dark/vertical slides), and dashboard (multi-page, card tabsets)Fixes #13781