Repository navigation
a11y: WCAG 2.2 AA pass on post editor + site editor - #535
Conversation
Applies 16 of 18 findings from a programmatic audit of the editor and site-editor chrome: - C1: form errors now use aria-invalid + aria-describedby across navigation-inspector, link-picker, pattern dialogs/inspector, menu dialog, and create-entity dialog - C2: post editor and site editor each gain a <main> landmark and a visually-hidden <h1> describing the current document/tab - C3: drop misused role="toolbar" from the editor top <header> - M1: drop incomplete tree role on NavigationCanvas (kept plain button list; dnd-kit KeyboardSensor still handles reorder) - M2: drop incomplete listbox/option roles on LinkPicker results - M4: add prefers-reduced-motion handling and a shared .screen-reader-text class via a new a11y.css imported from both entry points - M5: bump drag-handle and other icon-only buttons to >=24x24 - M6: replace border-color-only focus styles with real outlines on navigator links and the top-bar back link - M7: role="status" + aria-busy on sidebar-frame loading/empty states - M8: Field/ChipList helpers use useId + aria-labelledby to associate label and value - m1, m3, m5: redundant ARIA cleanup, decorative symbols wrapped in aria-hidden spans, autoFocus race removed from CreatePatternDialog Deferred: - M3 (modal scrim aria-hidden) needs a structural change since the scrim is the parent of the dialog element; tracking as follow-up. - Keyboard-only and VoiceOver walkthroughs are human tasks and were verified separately against the acceptance criteria. Closes #450
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds shared a11y CSS and imports, wraps editor shells in labeled landmarks with hidden headings, wires form error IDs and label associations with useId, improves focus outlines and sizing, and removes/adjusts redundant ARIA roles and decorative glyph markup across editor and site-editor.ChangesWCAG 2.2 AA Accessibility Audit
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
resources/js/visual-editor/editor/block-library-sidebar.css (1)
218-227: ⚡ Quick winSimplify conflicting height constraints for clarity.
The button has both
height: 16px(line 221) andmin-height: 24px(line 222). Sincemin-heighttakes precedence, theheight: 16pxdeclaration is effectively overridden. This creates confusion between the original layout goal mentioned in the comment above ("shrink the buttons to 16px tall") and the new WCAG 2.5.5 Level AA requirement for 24×24 minimum target sizes.Consider either removing the
height: 16pxdeclaration or updating the comment to clarify that the 24px minimum is an accessibility override.♻️ Proposed simplification
.ap-visual-editor-block-library__panel--layouts .block-editor-list-view-leaf .block-editor-block-mover-button { - height: 16px; min-height: 24px; width: 28px; min-width: 24px; margin: 0; padding: 0; }Alternatively, update the comment to document the accessibility override:
/** * Tighten the Layouts list-view up/down movers. Gutenberg's default * rules target a `.mover-cell-alignment-wrapper` class that's no * longer rendered (the wrapper moved inside `TreeGridItem` divs in a * recent refactor), so the two buttons stack with the table cell's * natural vertical-align spacing, leaving a big gap between ^ and v. - * These overrides shrink the buttons to 16px tall and zero their - * surrounding margins so the arrows sit snugly one atop the other. + * These overrides zero the surrounding margins so the arrows sit snugly + * one atop the other, while maintaining the 24×24px minimum required + * by WCAG 2.5.5 Level AA (Target Size). */🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resources/js/visual-editor/editor/block-library-sidebar.css` around lines 218 - 227, The CSS for .ap-visual-editor-block-library__panel--layouts .block-editor-list-view-leaf .block-editor-block-mover-button contains conflicting height rules (height: 16px and min-height: 24px); since min-height wins this is confusing — either remove the explicit height: 16px declaration or keep it but update the adjacent comment to state that min-height: 24px is an intentional WCAG accessibility override for 24×24 targets; modify the rule for .ap-visual-editor-block-library__panel--layouts .block-editor-list-view-leaf .block-editor-block-mover-button accordingly and update the comment above it to reflect which behavior is intended (shrunk 16px vs accessibility-enforced 24px).resources/js/visual-editor/site-editor/patterns/create-pattern-dialog.tsx (1)
308-369: ⚡ Quick winConsider adding
aria-describedbyto the sync radio group for consistency.The sync radio group displays validation errors (lines 360-368) but doesn't wire them via
aria-describedby, unlike every other validated field in this PR. For consistency with the pattern applied to text inputs and selects, consider generating asyncErrorIdand addingaria-describedby={validationErrors?.sync?.[0] !== undefined ? syncErrorId : undefined}to the<fieldset>element.The current implementation works (the error has
role="alert"and is structurally within the fieldset), but explicit association would align with WCAG 2.2 best practices and match the rest of this PR's approach.♻️ Proposed enhancement
Add at line 105:
const slugErrorId = useId(); +const syncErrorId = useId();Update the fieldset at line 308:
-<fieldset className="ap-pattern-dialog__field"> +<fieldset + className="ap-pattern-dialog__field" + aria-describedby={ + validationErrors?.sync?.[0] !== undefined + ? syncErrorId + : undefined + } +>Update the error paragraph at line 361:
<p + id={syncErrorId} className="ap-pattern-dialog__error" role="alert"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resources/js/visual-editor/site-editor/patterns/create-pattern-dialog.tsx` around lines 308 - 369, The sync radio fieldset is not associating its validation message via aria-describedby like other fields; create a unique id (e.g., syncErrorId) and use it when an error exists: generate syncErrorId near the other ID constants, add aria-describedby={validationErrors?.sync?.[0] !== undefined ? syncErrorId : undefined} to the <fieldset> that renders the sync radios, and set the error <p> (which already uses role="alert" and data-testid="ap-pattern-dialog-create-sync-error") to have id={syncErrorId}; update references around the sync state/validation (sync, setSync, validationErrors) to ensure the id is present only when an error exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@resources/js/visual-editor/editor/block-library-sidebar.css`:
- Around line 218-227: The CSS for
.ap-visual-editor-block-library__panel--layouts .block-editor-list-view-leaf
.block-editor-block-mover-button contains conflicting height rules (height: 16px
and min-height: 24px); since min-height wins this is confusing — either remove
the explicit height: 16px declaration or keep it but update the adjacent comment
to state that min-height: 24px is an intentional WCAG accessibility override for
24×24 targets; modify the rule for
.ap-visual-editor-block-library__panel--layouts .block-editor-list-view-leaf
.block-editor-block-mover-button accordingly and update the comment above it to
reflect which behavior is intended (shrunk 16px vs accessibility-enforced 24px).
In `@resources/js/visual-editor/site-editor/patterns/create-pattern-dialog.tsx`:
- Around line 308-369: The sync radio fieldset is not associating its validation
message via aria-describedby like other fields; create a unique id (e.g.,
syncErrorId) and use it when an error exists: generate syncErrorId near the
other ID constants, add aria-describedby={validationErrors?.sync?.[0] !==
undefined ? syncErrorId : undefined} to the <fieldset> that renders the sync
radios, and set the error <p> (which already uses role="alert" and
data-testid="ap-pattern-dialog-create-sync-error") to have id={syncErrorId};
update references around the sync state/validation (sync, setSync,
validationErrors) to ensure the id is present only when an error exists.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cf85c5cb-0f8d-44b2-9393-85ea38199669
📒 Files selected for processing (25)
resources/js/visual-editor/a11y.cssresources/js/visual-editor/editor/__tests__/top-bar.test.tsxresources/js/visual-editor/editor/block-library-sidebar.cssresources/js/visual-editor/editor/editor-app.tsxresources/js/visual-editor/editor/main.tsxresources/js/visual-editor/editor/top-bar.tsxresources/js/visual-editor/site-editor/create-entity-dialog.tsxresources/js/visual-editor/site-editor/main.tsxresources/js/visual-editor/site-editor/navigation/create-menu-dialog.tsxresources/js/visual-editor/site-editor/navigation/link-picker.tsxresources/js/visual-editor/site-editor/navigation/navigation-canvas.cssresources/js/visual-editor/site-editor/navigation/navigation-canvas.tsxresources/js/visual-editor/site-editor/navigation/navigation-inspector.tsxresources/js/visual-editor/site-editor/navigator-sidebar.cssresources/js/visual-editor/site-editor/patterns/create-pattern-dialog.tsxresources/js/visual-editor/site-editor/patterns/pattern-dialog.tsxresources/js/visual-editor/site-editor/patterns/pattern-inspector.tsxresources/js/visual-editor/site-editor/sidebars/global-styles-sidebar.tsxresources/js/visual-editor/site-editor/sidebars/menu-sidebar.tsxresources/js/visual-editor/site-editor/sidebars/pattern-sidebar.tsxresources/js/visual-editor/site-editor/sidebars/sidebar-frame.tsxresources/js/visual-editor/site-editor/sidebars/template-sidebar.tsxresources/js/visual-editor/site-editor/site-editor-app.cssresources/js/visual-editor/site-editor/site-editor-app.tsxresources/js/visual-editor/site-editor/styles/styles-navigator.tsx
💤 Files with no reviewable changes (2)
- resources/js/visual-editor/editor/top-bar.tsx
- resources/js/visual-editor/site-editor/styles/styles-navigator.tsx
- block-library-sidebar.css: drop overridden height: 16px and reword the comment to document the 24x24 min-size as a WCAG 2.5.5 AA override. - create-pattern-dialog.tsx: wire the sync radio fieldset to its error via aria-describedby + syncErrorId, matching the rest of the dialog.
Description
Applies 16 of 18 findings from a programmatic WCAG 2.2 AA audit of the post editor and site editor chrome ahead of v1.0. The audit covered
resources/js/visual-editor/editor/andresources/js/visual-editor/site-editor/plus shared chrome.Closes: #450
Type of Change
Related Issue
Issue: #450
Motivation and Context
v1.0 is the public launch surface and the editor is the most interaction-heavy area of the product. #347 (Slice 2.5) moved the canvas into a
BlockCanvasiframe, which prompted a broader audit rather than relying on "WordPress probably handles it." This pass addresses the issues a code-level audit can surface; the human keyboard + screen-reader walkthroughs were performed separately against the acceptance criteria.Changes Made
Critical
aria-invalid+aria-describedbyacross navigation-inspector, link-picker, pattern dialogs/inspector, create-menu dialog, and create-entity dialog.<main>landmark and a visually-hidden<h1>describing the current document/tab (Editing: {title}/Site Editor — {section}).role="toolbar"from the editor top<header>(the toolbar pattern requires roving tabindex we don't implement; the<header>banner role is appropriate).Major
KeyboardSensorstill handles reorder.prefers-reduced-motionhandling and a shared.screen-reader-textclass via a newa11y.cssimported from both entry points.outline: 2px solid var(--color-primary)on navigator links and the top-bar back link.role="status"+aria-busyon sidebar-frame loading/empty states.Field/ChipListhelpers in template, menu, pattern, and global-styles sidebars useuseId+aria-labelledbyto associate label and value.Minor
role="list"/role="listitem"on native list elements in styles-navigator.×and⋮⋮symbols in<span aria-hidden="true">.autoFocusrace on CreatePatternDialog.Deferred / out of scope
aria-hidden) — The backdrop is the parent of the dialog element, so addingaria-hiddenwould hide the dialog itself from AT. Needs a small structural change (move scrim to a sibling or::before). Recommend filing as a follow-up.__experimentalLibraryinternals — shipped by@wordpress/*; Gutenberg's own a11y posture applies.How Has This Been Tested?
Testing Environment:
release/1.0Tests Performed:
./vendor/bin/pest --compact— 878 passed (2,450 assertions).npm test -- --run— 1,836 passed across 229 files.release/1.0— 0 findings.Accessibility Tests Run
Details:
<main>landmark and the new visually-hidden<h1>on both editors.Tests Added
Test details: One existing top-bar test was updated from
getByRole('toolbar')to assert the banner<header>after droppingrole="toolbar". No new tests were added — the changes are markup/CSS adjustments covered by the existing render and interaction tests.Documentation
Pre-Submission Checklist
Summary by CodeRabbit
New Features
Style
Tests