Improve attachment accessibility - #1053
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves accessibility and keyboard ergonomics for attachments (including galleries) by moving destructive controls out of the contenteditable, improving screen-reader announcements for NodeSelections, and adding keyboard-based attachment reordering while aiming to keep serialized HTML stable.
Changes:
- Introduces a contextual
<lexxy-attachment-toolbar>(Alt+F10 / Esc) and removes the inline delete button element. - Adds a live region +
announce()API and uses it to announce keyboard reordering actions (Alt+Shift+Arrow…). - Improves caret/selection behavior around decorator nodes (atomic caret stepping + “fake” DOM selection anchoring) and enhances caption accessibility/keyboard exit.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/javascript/unit/helpers/direction.test.js | Adds unit coverage for the new Direction helper. |
| test/browser/tests/prompts/mention_navigation.test.js | Adds browser coverage for keyboard navigation/select behavior around mentions. |
| test/browser/tests/formatting/horizontal_divider.test.js | Updates divider deletion to use the new contextual toolbar. |
| test/browser/tests/attachments/gallery.test.js | Refactors duplicated gallery helpers into shared helpers. |
| test/browser/tests/attachments/gallery_navigation.test.js | Adds keyboard navigation tests for moving selection within galleries. |
| test/browser/tests/attachments/drop_in_gallery.test.js | Uses shared gallery helpers instead of local duplicates. |
| test/browser/tests/attachments/attachments.test.js | Updates attachment deletion to use the new contextual toolbar. |
| test/browser/tests/attachments/attachment_toolbar.test.js | Adds coverage for toolbar visibility, shortcut focusing, and delete action. |
| test/browser/tests/attachments/attachment_keyboard_move.test.js | Adds coverage for Alt+Shift+Arrow attachment move/reorder + announcements. |
| test/browser/tests/attachments/attachment_fake_selection.test.js | Adds coverage that the fake selection span is attached and labeled. |
| test/browser/tests/attachments/attachment_caption.test.js | Adds coverage for Tab-to-caption and Esc-to-exit caption editing. |
| test/browser/helpers/gallery_test_helpers.js | Centralizes gallery helper utilities used by multiple tests. |
| test/browser/helpers/attachment_helpers.js | Adds shared helpers for selecting attachments and building attachment HTML. |
| src/nodes/horizontal_divider_node.js | Removes inline delete UI and adds a label for accessibility tooling. |
| src/nodes/custom_action_text_attachment_node.js | Removes inline delete UI, adds decorative alt="" for inner images, adds label. |
| src/nodes/action_text_attachment_node.js | Adds label, caption focus helper, caption label mirroring, Esc-to-exit caption behavior. |
| src/helpers/lexical_helper.js | Adds selection helpers for labelled decorator nodes + editor announce helper. |
| src/helpers/direction.js | Introduces a forward/backward direction abstraction used by caret + reorder logic. |
| src/extensions/attachments_extension.js | Registers new attachment accessibility behaviors (tab-to-caption, fake selection, caret, keyboard move). |
| src/elements/node_delete_button.js | Removes the old inline delete button custom element. |
| src/elements/live_region.js | Adds a debounced polite live region element for announcements. |
| src/elements/index.js | Registers new custom elements (live region + attachment toolbar) and removes old one. |
| src/elements/editor.js | Installs live region and attachment toolbar; exposes announce(). |
| src/elements/attachment_toolbar.js | Adds contextual toolbar UI for removing the currently selected labelled decorator node. |
| src/editor/attachments/keyboard_move.js | Implements Alt+Shift+Arrow attachment moves + gallery creation/extraction + announcements. |
| src/editor/attachments/fake_selection.js | Implements the hidden span + DOM-range parking for SR focus-mode announcements. |
| src/editor/attachments/decorator_node_caret.js | Adds atomic caret behavior around decorator nodes and NodeSelection drop-back logic. |
| app/assets/stylesheets/lexxy-editor.css | Updates floating-controls styling for the new attachment toolbar and visually-hidden helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
e992e22 to
0f64cdd
Compare
0f64cdd to
69193a2
Compare
69193a2 to
d2a0007
Compare
There was a problem hiding this comment.
@brunoprietog I've made a few comments to start threads on the various workarounds proposed. I do want to note the high quality of the execution of the workarounds; my questions are on overall approach.
There was a problem hiding this comment.
Is the Alt+F10 shortcut tied to a floating delete button? Could the same shortcut be mapped to jumping on/off an individual DecoratorNode's deletion button?
I ask as floating buttons are more difficult to style onto the node and can end up mis-aligned with the node. The best positional code is the one we don't have to write. I think it's better to have more complexity jumping onto/off each node's button rather than moving a singleton button around.
There was a problem hiding this comment.
Alt+F10 is used to focus the toolbar, which could have any button actually. The problem is that we can't have those buttons inside the contenteditable element, which is why I left them as siblings. I improved the code there by using a ResizeObserver.
There was a problem hiding this comment.
This is the change which gives me the most pause and seems like it will need the most continuing maintenance to keep working.
What's the viability of manually announcing what's in the NodeSelection accessibility-wise?
There was a problem hiding this comment.
This was the trickiest part for sure. The reason for this fake selection is that we need to make the screen reader believe that something has changed in the cursor itself. If we use a live region or anything else, even if we can announce the correct information, the cursor will also be read, with incorrect information about the cursor, such as the first letter of the previous paragraph. In other words, the live region is just an additional announcement, but it doesn’t fix the incorrect announcements about the cursor.
d2a0007 to
ac3d093
Compare
ac3d093 to
2899401
Compare
2899401 to
fb8b659
Compare
fb8b659 to
91a131f
Compare
91a131f to
16fa22d
Compare
16fa22d to
1314191
Compare
1314191 to
3a4ca9a
Compare
53d947d to
2aaa1d4
Compare
2aaa1d4 to
fd3efc6
Compare
fd3efc6 to
74d8453
Compare
74d8453 to
2b42937
Compare
2b42937 to
bb99b1f
Compare
bb99b1f to
72e7497
Compare
72e7497 to
ef7d5de
Compare
ef7d5de to
fd74d0c
Compare
c7ddda0 to
52ccabe
Compare
Introduces <lexxy-live-region> as a visually hidden announcer appended to every editor, plus an announceFromEditor helper that resolves the editor element from a Lexical editor instance. Use document.ariaNotify at high priority when the browser supports it. The fallback separates durable additions from transient caret announcements so repeated messages remain observable without leaving stale text under the cursor in browse mode.
Lexical clears the DOM range whenever a NodeSelection commits. With no range anchored anywhere meaningful, a screen reader in focus mode reads whatever the browser leaves selected, often stray characters next to the attachment instead of the attachment itself. AttachmentFakeSelection parks the DOM range on a visually hidden span carrying the decorator's label so the announcement matches what was selected. Backed by a generic registerLabelledDecoratorSelection helper that fires on any single-node selection whose node exposes a label.
When focus mode reads a line that contains a decorator, the screen reader stops at the first <img alt> or treats the figure as a single character. Two cases need different handling: For inline image-backed figures (mentions) we leave the avatar's alt empty so line-by-line reading flows past, and set it to the label only while the caret is one keystroke away or on the figure. As soon as the caret moves, alt goes back to empty. For gallery images we push the caption through the live region as the caret approaches and temporarily aria-hide the figcaption to avoid a duplicate reading where screen readers honor it. DecoratorAnnouncement drives both via a small opt-in contract any DecoratorNode can implement: isAnnounceable, setupAnnouncement, teardownAnnouncement, and optionally shouldAnnounceLabel + label. Repeated editor updates for the same caret position do not announce the caption again.
The per-figure <lexxy-node-delete-button> sat inside the attachment DOM, which Lexical treats as part of the decorator's content. Screen readers picked it up while reading the figure even when it wasn't needed, the button drifted off-screen on narrow containers, and the markup made keyboard navigation harder than it should have been. <lexxy-attachment-toolbar> is mounted once per editor as a floating toolbar that tracks the currently selected attachment. It exposes the Remove action with a proper toolbar role, supports Shift+Tab back to the figure, and Tab forward to the caption when the attachment has one. The horizontal divider figure loses its custom delete button and relies on the same toolbar.
Attachment captions used a bare textarea sitting inside the figure with no label, no obvious role, and no way to exit without losing the figure selection. Screen readers read the caption inconsistently between focus and browse modes, and Escape from the textarea dropped the user back at the start of the document. The textarea now carries an aria-label scoped to the attachment type (Image vs Video) and stays aria-hidden until it is focused, with a mirror <span class="attachment__caption-text"> that browse mode reads in its place. Escape returns focus to the figure and re-selects the attachment as a NodeSelection so Tab cycles back into the toolbar. The mirror is only rewritten when the label changes, preserving its text node through unrelated decorator updates and avoiding redundant screen-reader announcements.
A selected attachment, divider, or gallery couldn't be reordered without a mouse. Drag and drop is the only path today, and there's nothing for screen reader users to anchor on. AttachmentKeyboardMove handles Alt+ArrowUp and Alt+ArrowDown when a single decorator is selected. It moves figures past adjacent paragraphs and into or out of galleries, keeps the selection on the moved node so the next keystroke continues from there, and announces each move through the live region so the user knows the position changed.
52ccabe to
53dc420
Compare
|
Status check on this one, plus one change that should take the pressure off it. The customer-facing half is now split out as #1224. A customer reported in early July that Tab lands in an image caption instead of the submit button, so Tab+Return submits a comment only when there is no image in it (card: [Lexxy] Tab > Return behavior inconsistent when images are added). That is fixed by It needed nothing from the two files under discussion here. No toolbar, no fake selection; every symbol it uses was already on This PR stays open for the toolbar and announcement work. Splitting was only about not making a one-line customer fix wait on a design conversation. @samuelpecher — two threads are still open and I think they're with you:
Bruno answered both on 23 May and neither has a reply since, so they've been sitting for ~2.5 months waiting on your read rather than on his. If Bruno's answers land, resolving them would unblock; if the maintenance concern on the fake selection still stands, that's worth saying out loud, because it's the load-bearing design question in the PR and everything else here is settled. @jorgemanrubia — you've had a review requested since 23 May that hasn't been submitted, and you're the assignee on the customer card. The remaining call is the fake-selection tradeoff Sam raised, which is a judgement call rather than a code detail. Worth noting the other 25 threads are resolved, so this is genuinely down to those two. |
Make attachments and galleries usable by keyboard and screen-reader users while keeping the existing markup and serialized HTML unchanged.
Screen reader announcements
DecoratorAnnouncementwatches the editor and flips DOM hints on an attachment's<figure>only while the caret is on it or one keystroke from crossing it, reverting as soon as the caret moves away. A permanent<img alt>would otherwise stop line-by-line focus-mode reading at the figure; a permanentaria-hiddenfigcaption would make browse mode skip the caption text. Decorators opt in by exposingisAnnounceable,setupAnnouncement(figure)andteardownAnnouncement(figure). Gallery images also opt in to a live-region push viashouldAnnounceLabel, since their announcement form has to silence the figcaption.AttachmentFakeSelectionparks the DOM range on a visually hidden span inside the figure when an attachment receives a NodeSelection, so the screen reader announces the widget label in focus mode. (Same pattern CKEditor 5 uses for widgets.)Selection toolbar
<lexxy-attachment-toolbar>sibling, reachable withAlt+F10and dismissed withEsc. The inline<lexxy-node-delete-button>is gone.Caption
<textarea>witharia-labeland letEscexit caption editing by re-selecting the attachment.Live region
aria-liveregion on<lexxy-editor>with a debouncedannounce()helper.Keyboard reordering (#877)
Alt+Shift+Arrowmoves the selected attachment up/down and creates or extracts galleries when adjacent images line up. When the attachment is already at the start or end of its container, the screen reader is told instead of the move silently failing.Basecamp card: https://app.basecamp.com/2914079/buckets/47229151/card_tables/cards/9894911453