Fix popover focus-management gaps and add active-descendant support - #40
Open
gohabereg wants to merge 11 commits into
Open
Fix popover focus-management gaps and add active-descendant support#40gohabereg wants to merge 11 commits into
gohabereg wants to merge 11 commits into
Conversation
Adds @playwright/test with chromium and webkit projects running against the existing vite dev server on port 3300, plus dedicated fixture pages for the desktop menu, inline toolbar and mobile popovers. The preview playground is left untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Popover items now carry a role derived from their toggle param (menuitem, menuitemcheckbox, menuitemradio, or button for the inline popover), an accessible name taken from the title with a fallback to the hint title, and aria-disabled. Icons and chevrons are marked decorative. toggleActive() keeps aria-checked/aria-pressed in sync with the active class, and neighbouring items sharing a toggle key are wrapped into a role="group" element that generates no box, so layout is unchanged. The items container is exposed as a menu (toolbar for the inline popover) and can be named via the new optional messages.label. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Flipper gets an opt-in 'focusItems' mode that moves DOM focus to the current item and maintains a roving tabindex over the navigated elements. It is off by default, so other Flipper users are unaffected. The desktop popover enables it: items are focusable but not tabbable, the opened popover makes its first item tabbable as the keyboard entry point, and a closed popover makes all of them untabbable again so Tab pressed outside does not land on a hidden item. The focused modifier keeps providing the visual highlight, and the UA focus ring is suppressed to keep the appearance unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The popover header back button renders an icon only and had no accessible name, so a screen reader user could not leave a nested popover on mobile. It now takes its name from the new 'back' message and points the popover at the header text via aria-labelledby, so entering a submenu announces which one. Nested desktop popovers are named after the item they were opened from. The search input gets an aria-label and type="search" (its native decorations are suppressed to keep the appearance unchanged). Instead of being permanently untabbable, it becomes tabbable while the popover is opened and untabbable again once it closes, which is what the tabIndex workaround was really after. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds a polite live region owned by the popover. Filtering the items now reports the number of results or that nothing was found, and entering confirmation mode announces what is being confirmed — previously the item silently turned into a different control with a different action. Confirmation mode also re-applies the item's role, name and state, since it replaces the item content in place. Items with children advertise aria-haspopup and keep aria-expanded in sync with the nested popover, whether it was opened by hover or by click. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The mobile popover is a modal dialog: it gets role="dialog" and aria-modal, traps Tab inside itself while opened, closes on Escape and returns focus to whatever it was opened from. Separators are exposed as separators, the html item wrapper takes role="none" so its content participates in the menu directly, and filtered out items get the hidden attribute alongside the class. Hints hold shortcut descriptions and were hover-only, hence unreachable from the keyboard. They now have role="tooltip", are referenced from their item via aria-describedby and are shown on focus as well as on hover. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds optional ariaLabel and role params to the default popover item for the cases the derived semantics do not fit. Both fall back to the derived values, so existing call sites are unaffected. Excludes the Playwright specs from the vitest run, they have their own runner. @editorjs/ui-kit 1.1.5 -> 1.2.0 and @editorjs/dom 1.1.0 -> 1.2.0: additive API plus the changed popover focus behaviour. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two regressions found while comparing the rendered geometry against main. The radio group wrapper generates no box, but it does scope :last-of-type to itself, so the item closing a group lost its 1px bottom margin and everything below it moved up by a pixel. The margin is restored unless the group closes the list. More importantly, WebKit drops the text selection once focus moves to a button, so arrow navigation in the inline popover destroyed the selection the inline tools operate on — the very problem the button wrapper tag works around. The inline popover now keeps focus in the text and only moves the highlight, while menus keep the roving focus model. Covered by an e2e test asserting the selection survives keyboard navigation. Focusing the first element of the mobile dialog no longer scrolls the page. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Manual keyboard/AT-tree testing against the previous commits turned up several dead ends the automated e2e suite didn't cover: - Nested desktop submenus had no way out via keyboard: Escape now backs out of the deepest open submenu (or closes the popover when there is none), and ArrowLeft/ArrowRight mirror that plus open a submenu, per the WAI-ARIA menu pattern. A single listener on the root popover resolves the deepest open level itself, so it stays correct regardless of nesting depth. - `flippable: false` menus were a total keyboard dead end: only the first item ever got a tabindex, Enter/Space did nothing, and the items container itself (a scrollable region) was sequentially focusable, blocking forward Tab past it. Every item is now its own Tab stop when there's no roving-tabindex Flipper to move focus for it, Enter/Space clicks the focused item directly, and the items container opts out of scroll-driven focusability. - Desktop popovers now restore focus to whatever triggered them on close, matching what the mobile dialog already did, but only when focus is still inside the popover (a click on another trigger already moved focus there first and shouldn't be clobbered). - The mobile dialog's separator was reachable by Tab; separators are excluded from its focus trap now. - A nested popover's `onOpen` callback now fires after the nested popover is attached and shown rather than before, since a consumer focusing one of its elements from that callback was a no-op on a detached node. - Search-result announcements are debounced so a burst of keystrokes collapses into one announcement, and a single matching item is announced as "1 result" instead of misusing the plural template. None of this changes the public TypeScript surface, but real focus movement, the radio-group DOM wrapper, and the `hidden` attribute on filtered items are behavior changes a consumer could observe. @editorjs/ui-kit 1.2.0 -> 2.0.0 to flag that. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ent navigation Search filtering left every item's roving tabindex stripped with nothing to restore it, so Tab from the search field landed nowhere in the results. The mobile dialog's focus trap toggled tabindex on an html item's non-interactive role="none" wrapper instead of its real controls, leaving those permanently unreachable by Tab. Both were silent gaps in the a11y work from prior commits, caught while reviewing the branch and confirmed via new regression tests. Also adds PopoverEvent.ActiveDescendantChanged, since inline popovers keep real DOM focus on the text selection and had no way to tell a screen reader which toolbar button is currently highlighted - a consumer can now mirror it onto aria-activedescendant of whichever element holds focus. Removes the planning brief now that the implementation is done. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Summary
Makes the popover (
@editorjs/ui-kit) usable with a screen reader and keyboard-only: proper ARIA semantics, real keyboard focus management with roving tabindex, a modal focus trap on mobile, and live-region announcements for dynamic changes. Also fixes two focus-management gaps found while reviewing the branch, and adds a hook for popovers that can't move real focus.A11y features added
menuitem/menuitemcheckbox/menuitemradio/option/button), derived fromtoggleand overridable viarole/ariaLabelaria-checked/aria-pressed/aria-selected,aria-disabled,aria-haspopup/aria-expandedkept in sync with item statetogglekey) wrapped inrole="group"flippable: false): every item is its own Tab stop, Enter/Space activaterole="dialog"+aria-modal, with a real Tab focus trap, Escape to close, and focus returned to the trigger on closearia-live="polite") announces search result counts (debounced, singular/plural), "nothing found", and confirmation-mode promptstype="search"box, only tabbable while openaria-describedbyand now shown on keyboard focus, not just hoverbackButtonLabel, item title, ormessages.label)role="separator", excluded from tab orderhiddenattribute mirrors visual hiding so assistive tech doesn't see filtered/hidden itemsPopoverEvent.ActiveDescendantChanged— inline popovers keep real focus on the text selection, so there was no way for a screen reader to know which toolbar button is highlighted; a consumer can now mirror this ontoaria-activedescendantof whichever element holds focusBreaking UX/UI changes
<div role="group">(display: contents — no layout impact, but changes DOM structure/:last-of-typescoping for custom CSS)hiddenattribute in addition to the existing CSS classtype="search"with native search decorations suppressedonOpencallback now fires after it's attached and shown, not before (fixes focusing a custom element from that callback, which was a no-op on a detached node)(
@editorjs/ui-kitwas already bumped 1.2.0 → 2.0.0 for the above in a prior commit on this branch.)API changes
PopoverItemDefaultBaseParams.ariaLabel?: string— explicit accessible name overridePopoverItemDefaultBaseParams.role?: PopoverItemRole— explicit ARIA role overridePopoverMessages.back,.result,.results,.label— new i18n message keysPopoverEvent.ActiveDescendantChanged— new event, payloadstring | null(highlighted item's element id, ornull)Test plan
yarn build(type-check + bundle) passesyarn test(vitest, other packages) passesActiveDescendantChangedcovers highlight tracking and clearing on close🤖 Generated with Claude Code