Repository navigation
Editor: Describe inline revision diffs with aria-describedby - #80207
thisismyurl wants to merge 1 commit into
Conversation
The inline diff formats in the Visual History canvas communicated their meaning to assistive technology only through the title attribute, which screen readers may ignore in low-verbosity modes. Each format now also references a visually hidden description element injected into the canvas document, which is announced reliably. The title attribute stays for the mouse-hover tooltip and the detailed format-change counts. Part of WordPress#77530. See https://core.trac.wordpress.org/ticket/65122. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning: Type of PR label mismatch To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.
Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task. |
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @thisismyurl! In case you missed it, we'd love to have you join us in our Slack community. If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information. |
There was a problem hiding this comment.
Pull request overview
This PR improves the accessibility of inline revision diffs in the Visual History canvas by adding stable aria-describedby descriptions for del, ins, and format-change spans (so AT doesn’t rely on title announcements).
Changes:
- Register
aria-describedbyas a rich-text format attribute and add auseDiffDescriptionsRef()hook to inject description elements into the canvas document. - Apply the new
ariaDescribedByattribute at inline diff formatting call sites and merge the new ref with the existing diff-markers ref. - Add/update unit tests to validate the rendered attributes and injected description elements.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/editor/src/components/post-revisions-preview/diff-format-types.js | Registers aria-describedby and injects description elements into the canvas document via a ref hook. |
| packages/editor/src/components/post-revisions-preview/block-diff.js | Applies ariaDescribedBy IDs when marking added/removed/format-changed inline diffs. |
| packages/editor/src/components/post-revisions-preview/revisions-canvas.js | Merges the diff-markers ref with the new descriptions ref on the canvas content element. |
| packages/editor/src/components/post-revisions-preview/test/block-diff.js | Updates expected inline diff HTML strings to include aria-describedby. |
| packages/editor/src/components/post-revisions-preview/test/diff-format-types.js | Adds tests for mounting/unmounting the injected description elements. |
| const container = ownerDocument.createElement( 'div' ); | ||
| // Hidden elements are still used for the accessible description | ||
| // computation when referenced by `aria-describedby`. | ||
| container.hidden = true; | ||
| for ( const { descriptionId, title } of DIFF_FORMAT_TYPES ) { |
| for ( const [ id, text ] of Object.entries( EXPECTED_DESCRIPTIONS ) ) { | ||
| const description = screen.getByText( text ); | ||
| expect( description ).toHaveAttribute( 'id', id ); | ||
| expect( description ).not.toBeVisible(); | ||
| } |
|
Sorry that I didn't see this one; I reviewed #80440, which is very similar. |
|
Two failing checks — both are non-code issues:
No code changes needed for either. |
|
Confirmed — pulling in trunk turns up a straight duplicate of #80440 (same IDs wired into |
What?
Part of #77530.
Core tracking ticket: https://core.trac.wordpress.org/ticket/65122
Gives the inline diff formats in Visual History (
del,ins, and the format-change spans) a reliable accessible description: each one now carriesaria-describedbypointing at a visually hidden description element rendered into the canvas document.Why?
From @joedolson's audit in #77530:
The
titleattribute maps to an ARIA description in the accessibility tree, but that mapping is exactly what low-verbosity modes drop. An explicitaria-describedbyreference wins the description computation and is announced consistently.How?
diff-format-types.jsregistersaria-describedbyas a format attribute alongsidetitle, and exports a newuseDiffDescriptionsRef()hook. The hook appends a hidden container of description elements (Removed,Added,Format added,Format removed,Format changed) to the document that hosts the ref element. That matters because the canvas is an iframe, andaria-describedbyonly resolves within the same document.block-diff.jsadds the matchingariaDescribedByattribute at the threeapplyFormat()call sites.revisions-canvas.jsmerges the new ref with the existing diff-markers ref on the canvas content element.One trade-off worth flagging: the format-change spans previously carried detailed counts in
title("2 formats added, 1 format removed"). Those stay intitlefor the mouse tooltip, but thearia-describedbydescription is the generic per-type text ("Format added"), since the referenced elements are static. If you'd rather screen reader users get the detailed counts, the descriptions container would need to be built per-instance from the diff output. I kept this version minimal, but happy to build that instead if it's the right call.I considered injecting the descriptions through the existing
usePrivateStyleOverrideassets channel (how the removed-blocks SVG filter gets into the iframe), but that channel renders inside an<svg>element, which is no place for HTML description elements. So the injection is ref-based instead, next to the existing diff-markers pattern.Testing Instructions
del/ins/format-change span in the canvas iframe: each should havearia-describedbypointing at an element with the corresponding ID (revision-diff-description-removed,-added,-format-*), and that hidden container should exist in the iframe'sbody.npm run test:unit packages/editor/src/components/post-revisions-previewTo be upfront about the verification boundary: the unit tests prove the DOM structure (attributes present, correct IDs, hidden container mounted and cleaned up), but I haven't been able to run this against a live screen reader on this machine, so I'd appreciate an AT confirmation during review.
Testing Instructions for Keyboard
title-based descriptions are not announced.Props @thisismyurl.
Use of AI Tools
(full disclosure: AI helped me identify the issue and verify my work)