feat: add tile-level focus trap and keyboard navigation for toolbars [CLUE-391]#2783
Open
dougmartin wants to merge 9 commits intomasterfrom
Open
feat: add tile-level focus trap and keyboard navigation for toolbars [CLUE-391]#2783dougmartin wants to merge 9 commits intomasterfrom
dougmartin wants to merge 9 commits intomasterfrom
Conversation
…LUE-391] Implement keyboard accessibility for tile toolbars and focus trap cycling so keyboard-only and screen reader users can navigate into tiles, operate toolbar buttons, and move between tiles without a mouse. - Add focus trap in tile-component.tsx: Tab/Shift+Tab cycles through title, toolbar (single stop), and content; Escape exits to tile container; ArrowUp exits from non-editable elements; Enter enters trap - Add roving tabindex to tile toolbar (Left/Right arrow keys navigate buttons as a flat sequence regardless of visual row layout) - Add ARIA attributes: role="toolbar" on toolbar, aria-label on toolbar and buttons, aria-pressed on toggle buttons, role="group" and aria-label on tile containers - Switch disabled toolbar buttons from HTML disabled to aria-disabled so they remain focusable; announce "Select something to enable this action" on activation - Add ITileApi.getFocusableElements() for tile-type-agnostic focus navigation; implement for text tiles (Slate editor + title) - Add RegisterToolbarContext for cross-FloatingPortal tile↔toolbar communication without document.querySelector - Add inter-tile navigation: Escape→Tab moves to sibling tiles with one-shot pass-through (escapedFocusTrap + justArrivedViaNav flags prevent infinite navigation loops) - Add screen reader announcements via aria-live region on trap enter/exit, including the toolbar Escape path (FloatingPortal is outside tile DOM so needs its own announcement) - Use native capture-phase DOM listeners for Tab/Shift+Tab (React 17 delegated events fire too late to preventDefault) - Fix useRovingTabindex stale state: split useLayoutEffect so tabindex attributes update on every render for dynamic button changes - Focus trap entry and cycling verify focus actually moved via focusFirstAvailable(), skipping non-focusable elements (e.g., plain div title) to prevent dead-end traps - Add 64 new unit tests across tile-component, tile-toolbar, and use-roving-tabindex
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #2783 +/- ##
==========================================
- Coverage 85.46% 83.14% -2.32%
==========================================
Files 847 847
Lines 46358 46613 +255
Branches 12048 12151 +103
==========================================
- Hits 39618 38756 -862
- Misses 6311 7355 +1044
- Partials 429 502 +73
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
collaborative-learning
|
||||||||||||||||||||||||||||
| Project |
collaborative-learning
|
| Branch Review |
CLUE-391-tab-order-in-tiles-toolbars
|
| Run status |
|
| Run duration | 03m 17s |
| Commit |
|
| Committer | Doug Martin |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
4
|
| View all changes introduced in this branch ↗︎ | |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds tile-level keyboard accessibility by introducing a focus trap that cycles through a tile’s title/toolbar/content, plus roving tabindex keyboard navigation for tile toolbar buttons (including ARIA improvements and SR announcements). This builds on the existing tile + FloatingPortal toolbar architecture by adding explicit cross-portal coordination for focus management.
Changes:
- Implement tile focus-trap entry/exit/cycling (Tab/Shift+Tab, Enter to enter, Escape/ArrowUp to exit) and inter-tile navigation behavior.
- Add roving tabindex support + native capture-phase Tab/Escape handling for FloatingPortal toolbars, plus ARIA attributes for toolbars/buttons/tiles.
- Extend tile API with
getFocusableElements()and implement it for text tiles; add substantial unit test coverage for focus behavior.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/hooks/use-roving-tabindex.ts | Updates roving tabindex behavior to keep tabindex attributes in sync with dynamic button DOM changes. |
| src/hooks/use-roving-tabindex.test.tsx | Adds tests for dynamic add/remove button scenarios and roving target clamping. |
| src/components/toolbar/tile-toolbar.tsx | Adds roving tabindex wiring, capture-phase Tab/Escape handling, toolbar ARIA, and registers toolbar element back to tiles. |
| src/components/toolbar/tile-toolbar.test.tsx | Adds ARIA and keyboard navigation tests for toolbars (including Tab/Escape/ArrowUp routing). |
| src/components/toolbar/tile-toolbar-button.tsx | Switches to aria-disabled, adds ARIA labeling/pressed state, and announces activation of disabled actions. |
| src/components/tiles/tile-component.tsx | Implements tile-level focus trap + SR announcements, plus inter-tile navigation and toolbar registration plumbing. |
| src/components/tiles/tile-component.test.tsx | Adds extensive focus-trap/inter-tile navigation/live-region tests via a dedicated test tile type. |
| src/components/tiles/tile-api.tsx | Introduces ITileApi.getFocusableElements() and RegisterToolbarContext for focus trap integration. |
| src/components/tiles/text/text-tile.tsx | Implements getFocusableElements() for text tiles (Slate editor + title). |
| specs/CLUE-391-tab-order-in-tiles-toolbars-text-tiles.md | Adds/records the detailed accessibility spec and decisions for the new behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Toolbar buttons using aria-disabled remain clickable, so without an explicit type="button" they default to type="submit" inside forms, which could cause unintended form submission.
When tabbing from the workspace toolbar into a text tile, the Slate
editor received DOM focus but silently ignored keyboard input because
ReactEditor.focus() does not create a selection if the editor has never
been focused before.
Changes:
- Add tabIndex={0} to tile containers so they are reachable via Tab
- Add handleFocus handler to auto-enter the focus trap on Tab arrival
- Add focusContent callback to ITileFocusableElements for custom editors
- Implement focusContent in text-tile using ReactEditor.focus() with
fallback selection initialization at end of document
- Update tile-toolbar Tab handler to use focusContent when cycling
- Update CLUE-391 spec with new architecture notes
… trap auto-entry handleFocus was auto-entering the focus trap on every .focus() call to the tile container, including programmatic focus from ArrowUp exit and inter-tile navigation. Added relatedTarget check so auto-entry only triggers when focus arrives from outside the tile (e.g., Tab from the workspace toolbar). Updated tests to reflect that pass-through navigation now auto-enters the destination tile's focus trap.
…destination tile The justArrivedViaNav/tile-navigation-focus mechanism caused a confusing two-hop skip: Escape+Tab from tile 1 would land on tile 2 without trapping, then Tab again would skip to tile 3 where focus finally trapped. This was inconsistent with the first tile's behavior where Tab immediately entered the focus trap. Removed the entire pass-through mechanism (justArrivedViaNav field, tile-navigation-focus event, propagateNavMode parameter) so that every tile now auto-enters its focus trap when Tab'd into via handleFocus. This ensures consistent behavior: Tab always traps, Escape+Tab always navigates to the next tile and traps there.
emcelroy
approved these changes
Mar 2, 2026
Contributor
emcelroy
left a comment
emcelroy
left a comment
There was a problem hiding this comment.
Looking good 👍 I left some comments about things you may want to clean up or tweak before merging.
- Use generic wording in spec (tile's editable content, not text-specific) - Change aria-live from "assertive" to "polite" for non-urgent status messages - Remove unused titleElement from text-tile getFocusableElements - Improve contentElement typing to avoid unnecessary cast - Add comment explaining why DOM-based tile navigation is used - Remove empty afterEach in tile-toolbar test
…nges Update Cypress test assertions to check aria-disabled instead of HTML disabled attribute, since toolbar buttons now use aria-disabled to remain keyboard-focusable. Add tileHandlesOwnSelection guard in handleFocus to prevent placeholder tiles from being auto-selected on focus. Add stopPropagation to editable-axis-label Escape handler to prevent tile-component from stealing focus and triggering an unwanted blur-save.
…s trap Change keyboard navigation so Tab shows a focus ring without selecting or entering tiles. Enter explicitly selects and enters the focus trap, Escape always deselects. This removes the escapedFocusTrap flag entirely — tile selection state is the sole routing signal for Tab behavior. Key changes: - handleFocus no longer selects or enters focus trap, only announces - handleTabKeyDown routes on selection: selected → enter trap, else → sibling - Enter selects unconditionally then enters focus trap best-effort - Escape deselects from anywhere inside tile or on selected container - navigateToSiblingTile no longer selects destination tile - handleToolbarEscape deselects tile on toolbar Escape - :focus-visible ring hidden when tile is selected (.selected overrides) - aria-disabled styling added to toolbar buttons
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
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.
Implement keyboard accessibility for tile toolbars and focus trap cycling so keyboard-only and screen reader users can navigate into tiles, operate toolbar buttons, and move between tiles without a mouse.