Fix popover focus-management gaps and add active-descendant support by gohabereg · Pull Request #40 · editor-js/utils

gohabereg · 2026-07-26T21:52:51Z

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

ARIA roles for items (menuitem/menuitemcheckbox/menuitemradio/option/button), derived from toggle and overridable via role/ariaLabel
aria-checked/aria-pressed/aria-selected, aria-disabled, aria-haspopup/aria-expanded kept in sync with item state
Radio-group items (shared string toggle key) wrapped in role="group"
Real DOM focus + roving tabindex during keyboard navigation (desktop menus), so screen readers announce the item as it's reached
Keyboard support even without a Flipper (flippable: false): every item is its own Tab stop, Enter/Space activate
Mobile popover exposed as role="dialog" + aria-modal, with a real Tab focus trap, Escape to close, and focus returned to the trigger on close
Nested submenu keyboard nav matching the WAI-ARIA menu pattern: Escape backs out one level (or closes the popover), ArrowLeft/ArrowRight mirror that
Live region (aria-live="polite") announces search result counts (debounced, singular/plural), "nothing found", and confirmation-mode prompts
Search input is a labeled type="search" box, only tabbable while open
Hints are referenced via aria-describedby and now shown on keyboard focus, not just hover
Back button and nested-popover/dialog get accessible names (backButtonLabel, item title, or messages.label)
Separators exposed as role="separator", excluded from tab order
hidden attribute mirrors visual hiding so assistive tech doesn't see filtered/hidden items
Fixed in this PR: typing in the search box no longer strips every item's tabindex without restoring it — Tab from the search field now reaches the filtered results
Fixed in this PR: a custom HTML item's real control (button/input), not its non-interactive wrapper, is now what the mobile dialog's focus trap makes tabbable
New in this PR: PopoverEvent.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 onto aria-activedescendant of whichever element holds focus

Breaking UX/UI changes

Keyboard navigation moves real DOM focus (roving tabindex) instead of only a visual highlight class, on desktop menus
Radio-group items are now wrapped in an extra <div role="group"> (display: contents — no layout impact, but changes DOM structure/:last-of-type scoping for custom CSS)
Filtered/hidden items now carry a hidden attribute in addition to the existing CSS class
Popover closing now restores focus to whatever opened it (previously mobile-only behavior)
Search input is now type="search" with native search decorations suppressed
A nested popover's onOpen callback 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-kit was 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 override
PopoverItemDefaultBaseParams.role?: PopoverItemRole — explicit ARIA role override
PopoverMessages.back, .result, .results, .label — new i18n message keys
PopoverEvent.ActiveDescendantChanged — new event, payload string | null (highlighted item's element id, or null)

Test plan

Full Playwright e2e suite (132 tests, chromium + webkit) passes
yarn build (type-check + bundle) passes
yarn test (vitest, other packages) passes
New regression tests confirmed to fail without the fixes, pass with them (search→Tab reachability, mobile html-item tabbability)
New test for ActiveDescendantChanged covers highlight tracking and clearing on close

🤖 Generated with Claude Code

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>

A closed popover is collapsed with opacity/max-height so the open transition has something to animate from, which leaves it in the accessibility tree: a screen reader's virtual cursor could still land on items that are invisible and unclickable. The container is now visibility: hidden, with inert toggled alongside it to cover focus and the tab order. Toggling inert alone was not enough to pull a dynamic change like this out of Safari/VoiceOver's tree, hence both. Reviewing the branch then turned up four keyboard navigation gaps, each reproduced with a regression test before being fixed: - Closing a submenu left the parent menu unusable. The Flipper drops its cursor and every item's tabindex when it hands the keyboard over, and activating it back does not restore either without a cursor position. The trigger item held the focus with no --focused class marking it - the default ring is suppressed, so the focus was invisible - the menu had no tab stop left to be re-entered by, and the next arrow press resumed from the top of the list instead of continuing after the trigger. - ArrowRight opened a submenu but left the focus behind on the trigger, so nothing was announced and ArrowLeft could not close it until the user pressed ArrowDown first. Activating an item now moves the focus into the submenu it opens, unless a consumer's onOpen handler focused something else. - Inline popovers make every item an individual tab stop while the Flipper tracks a separate highlight, so Enter, which the Flipper handles against its own cursor, could activate a different item than the focused one a screen reader announces. The cursor now follows the real focus. - Hovering an item opened a nested popover without recording which item it came from, so that popover could not hand the focus back afterwards, and onChildrenClose fired on whichever item had last opened one by click. Fixes two Flipper bugs sitting behind these: deactivate() removed its keydown listener without the capturing flag it was added with, so the handler stayed on the document for the lifetime of the page, and DomIterator.setCursor admitted -1 as the 'nothing active' position and then dereferenced items[-1]. Drops the hidden attribute set alongside the --hidden class. An author display rule beats the UA [hidden] rule, so it affected neither rendering nor the accessibility tree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

ui-kit was the only package whose build did not typecheck it: the others run tsc, while this one runs vite build, which transpiles through esbuild and emits declarations without failing on type errors. A deliberate type error in popover-desktop.ts built cleanly. The cause was the tsconfig rather than the script - moduleResolution 'bundler' with no module setting makes tsc reject the project outright with TS5095 before it checks anything, which is presumably why nothing ever ran it. Setting module/target/lib yields zero errors on the existing source. The e2e fixtures import ui-kit from source but resolved sibling workspace packages to their built dist, so a change in packages/dom stayed invisible to the suite until that package was rebuilt - and a stale build is not an error, it just quietly tests yesterday's code. The dev server now resolves them to their sources instead, while the library build keeps resolving them the way a consumer would. Verified by appending a throw to dom's dist: the suite passes with the alias in place and fails without it. Also gives the orphaned e2e tsconfig a caller, since nothing referenced it and the specs went unchecked as a result, and stops Playwright from reusing whatever process happens to hold the port when running on CI. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull request overview

Copilot reviewed 43 out of 46 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

packages/ui-kit/src/popover/popover-desktop.ts:164

consumesArrowKeys() is used to avoid hijacking ArrowLeft/ArrowRight when focus is inside an input-like control, but it currently ignores . Select elements use arrow keys for value navigation, so the nested-navigation handler can incorrectly intercept ArrowLeft/ArrowRight while a select is focused inside the popover. packages/ui-kit/src/popover/popover-mobile.ts:16 FOCUSABLE_SELECTOR matches disabled /<textarea> because they aren't filtered with :not([disabled]). Disabled form controls can't receive focus, so they can end up as the computed first/last element and break the Tab loop (focus() becomes a no-op and the trap keeps trying to move to it). Consider also including contenteditable elements so custom editors can participate in the trap.

Reversean

Maybe better next time decompose PR in stack?

E.g. we could split it into smaller PR's

ARIA semantics
focus mechanics
live announcments

- Clear a pending search-results announcement when the query is cleared, so a stale "N results" never outlives the query it counted - Replace the three duplicated instancesCount counters with generateId() - Reuse Flipper for the mobile item list so its role="menu" is honoured: arrows navigate, the menu is one Tab stop, and the panel's trap loops Tab between the back button and the list. The keyboard layer moves into PopoverAbstract so getControls() is the single definition of "navigable" - Only restore focus on mobile close when it is still inside the popover - Show an html item's hint on keyboard focus by listening for focusin and describing its controls rather than its role="none" wrapper - Lay the item list out again on addItem/removeItemByName, so a dynamically added item is grouped and reachable without a close/reopen - Give PopoverDesktop ownership of Tab, so the ring it walks takes in the search field and Shift+Tab leafs the items backwards instead of leaving Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F3JEisQDVD82VYAp55Lhcp

Copilot

🟡 Changes recommended

Several focus, ARIA, lifecycle, and CI integration defects remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (7)

Previously missed (7) — in code that hasn't changed since the last review.

packages/ui-kit/src/popover/components/popover-item/popover-item-default/popover-item-default.ts:95

Every toolbar command is currently exposed as a toggle button, even when it has no toggle behavior (for example the non-toggle Link item in the preview). aria-pressed="false" changes what assistive technology announces; only actual toggle buttons should receive this state.

      case 'button':
        return 'aria-pressed';

packages/ui-kit/e2e/fixtures/inline-selection.html:30

This example points aria-activedescendant at toolbar buttons that are neither DOM descendants of the editable nor logically related through aria-owns/the applicable aria-controls relationship. That is not a valid active-descendant relationship, so assistive technologies may ignore the headline feature even though the attribute test passes. Establish a valid ownership/control relationship and test the accessibility behavior rather than only the string attribute.

  popover.on(PopoverEvent.ActiveDescendantChanged, (id) => {
    if (id === null) {
      editable.removeAttribute('aria-activedescendant');
    } else {
      editable.setAttribute('aria-activedescendant', id);

packages/ui-kit/src/popover/popover-desktop.ts:751

The PR states that filtered items mirror visual hiding with the hidden attribute, but this path only calls toggleHidden(), whose default, HTML, and separator implementations only toggle CSS classes. The promised DOM/assistive-technology state is therefore absent. Update all item implementations to set element.hidden = isHidden as well.
packages/ui-kit/src/popover/types/popover-item.ts:120
option and button are exposed as public item-role overrides, but the public popover container remains role="menu"; those roles are not valid owned children of a menu. Conversely, overriding an inline-toolbar item to a menu role also breaks its hierarchy. Couple item-role choices to a matching container role, or restrict the public override to roles valid for that popover.
packages/ui-kit/src/popover/popover-desktop.ts:184
Calling show() on an already-open popover overwrites the original opener with the currently focused search/item element. On close, restoration then targets an element that has just become inert, losing focus instead of returning to the trigger. Capture this value only on the closed-to-open transition.
packages/ui-kit/src/popover/popover-desktop.ts:436
Appending the nested element to this.nodes.popover does not guarantee it is connected to the document. PopoverInline opens children.isOpen during its constructor, before the caller can insert the root popover, so this callback still runs on a detached subtree and focus() remains a no-op in the exact auto-open use case described by the PR. Defer onChildrenOpen until the root is connected/shown, or move auto-opening out of the constructor.
packages/ui-kit/e2e/tests/announcements.spec.ts:80
This test never reopens the popover or checks the restored name; it only verifies that the same DOM node remains attached, which would also pass if aria-label stayed “Are you sure?”. Reopen and assert the original accessible name so the new ARIA reset path is actually covered.

    await hidePopover(page);

    /**
     * The closed popover is hidden from assistive tech (visibility: hidden), so getByRole()
     * can no longer find it - this checks the DOM directly to confirm the item itself is still
     * there (not destroyed), just no longer exposed while the popover is closed
     */
    await expect(page.locator('[data-item-name="delete"]')).toBeAttached();

Files reviewed: 43/46 changed files
Comments generated: 12
Review effort level: Balanced

+  &:focus-visible {
+    outline: none;
+  }


+    back: 'Back',
+    result: '{count} result',
+    results: '{count} results',


+     * A click on another trigger, for example, already moved focus there on mousedown, before
+     * this popover even learns it should close, and that focus should not be clobbered
+     */
+    const shouldRestoreFocus = document.activeElement !== null && this.nodes.popoverContainer.contains(document.activeElement);


+    if (this.flippableElements.length > 0) {
+      this.flipper?.focusFirst();
+
+      return;
+    }


+      return;
+    }
+
+    this.flipper?.focusFirst();


+    const focusedItem = this.itemsDefault.find(item => item.isFocused);
+
+    focusedItem?.onFocus();
+
+    this.emit(PopoverEvent.ActiveDescendantChanged, focusedItem?.id ?? null);


+    "test:e2e": "playwright test",
+    "test:e2e:ui": "playwright test --ui"


+    if (elements.length === 0) {
+      return;


+    /** Focus should return to whatever the popover was opened from */
+    this.previouslyFocusedElement = document.activeElement instanceof HTMLElement ? document.activeElement : null;


+    if (this.hasChildren) {
+      this.nodes.root.setAttribute('aria-haspopup', 'menu');
+      this.nodes.root.setAttribute('aria-expanded', 'false');


neSpecc · 2026-09-16T21:09:55Z

  private setCaret?: (item: HTMLElement) => void;

+  /**
+   * True if flipper should move real DOM focus to the current item


Explain when it’s expected to be set as “false” and “true”. Alternatively, remove this option and make it focusable by default.

neSpecc · 2026-09-16T21:48:40Z

Why there is no "pointer" hover on popover items?

The @Custom-Media definitions lived in variables.css, but PostCSS resolves them per file, so popover.css and popover-inline.css never saw them. Every @media (--mobile) and @media (--can-hover) block there went out unresolved, and browsers drop an invalid @media block as a whole: items had no pointer cursor nor hover background, and none of the mobile styles ever applied. This has been the case since the popover was migrated here. The definitions move to custom-media.css, which each stylesheet using them now imports. The only unresolved query left in the build is the tooltip's in @editorjs/helpers, which is out of scope here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Spells out when focusItems is expected to be true (a menu opened from a button, where screen readers only announce the element holding the real focus) and when false (a toolbar acting on a text selection, which Safari drops once a button takes the focus). The option stays, since the inline toolbar depends on it being off. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…tems The inline popover is a role="toolbar", which is navigated with ArrowLeft and ArrowRight, but its Flipper only listened to ArrowUp and ArrowDown. The horizontal arrows now move along the bar - only once the user has entered it, though: until then the focus is still in the text the bar formats, and the arrows have to keep collapsing the selection there. When Tab has put the real focus on a button, it travels along with the highlight. Html items were second-class in the keyboard navigation: - ArrowRight only opened the submenu of a default item - onFlip only looked among the default items, so highlighting an html item's control reported no active descendant at all, and those controls had no id to report either - getControls() only knew buttons and text inputs, leaving checkboxes, selects and links out of the navigation, and it disagreed with the mobile focus trap about what counts as a stop - aria-haspopup/aria-expanded sat on the role="none" wrapper, which assistive technologies ignore and which never takes the focus Flipper exposes its current item, which both the horizontal navigation and onFlip read now. getControls() and the mobile trap share one selector. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

- Closing the root popover while the focus was in a submenu left it on the body: the check for whether the focus is still inside looked at the container only, and nested popovers are appended next to it. A link tool submitting its URL from the nested input and closing the toolbar is exactly that case - show() on an already open popover replaced the element to return the focus to with the one focused inside, on desktop and mobile alike - Destroying an open mobile popover left its Flipper listening to the keys on the document. Block Settings in Editor.js does destroy it while open, and the mobile Flipper is new in this branch - A mobile popover built with flippable: false opened without moving the focus inside, and one with nothing focusable let Tab escape the modal - the dialog itself holds the focus in that case now - A submenu built with isFlippable: false left the focus on its trigger - Items of a flippable: false popover had no visible focus at all: the default ring was suppressed in favour of the --focused modifier, which only a Flipper applies Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

- Every inline toolbar button was exposed as a toggle button, aria-pressed included. Only the ones that have an on/off state (toggle or isActive) get it now, so e.g. a "Convert to" button is a plain button again - Leaving confirmation mode rebuilt the pressed/checked state from the params the item was constructed with, dropping whatever toggleActive() had changed since. It is synced back from the item's active class - Items filtered out by the search get the hidden attribute on top of the class, as the pull request description promises - Menus, toolbars and the mobile dialog fall back to a default accessible name ('Menu', 'Toolbar' for the inline popover) when no label is passed: the dialog in particular must be named, and neither Editor.js nor the document model passes one everywhere Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The Playwright suite was only ever run locally: the test workflow runs the root 'yarn test', and Vitest excludes the e2e directory. A separate job now builds the packages, typechecks the suite, installs Chromium and WebKit and runs it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

tabindex="-1" took the separator out of the Tab order, but also made it focusable, and a focusable separator is a splitter widget by ARIA, which requires aria-valuenow. axe flags every popover with a separator for it. The attribute was not needed in the first place: a plain div is not in the Tab order, and the popover only toggles the tabindex of flippable elements, which separators never are. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

gohabereg and others added 13 commits July 26, 2026 01:29

Keep e2e declarations out of the published types

8d6266b

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

gohabereg requested a review from Copilot July 29, 2026 17:40

Copilot started reviewing on behalf of gohabereg July 29, 2026 17:41 View session

Copilot AI reviewed Jul 29, 2026

gohabereg requested a review from Copilot July 29, 2026 17:42

Copilot started reviewing on behalf of gohabereg July 29, 2026 17:42 View session

Copilot AI reviewed Jul 29, 2026

View reviewed changes

gohabereg mentioned this pull request Jul 29, 2026

feat(ui,paragraph): ARIA semantics + accessibility e2e suites [BLOCKED on utils#40] editor-js/document-model#187

Draft

Reversean requested changes Aug 27, 2026

View reviewed changes

gohabereg requested a balanced review from Copilot August 29, 2026 17:54

Copilot started reviewing on behalf of gohabereg August 29, 2026 17:55 View session

gohabereg mentioned this pull request Aug 29, 2026

Keep search results and nested-level keys in sync on mobile #41

Open

Copilot AI reviewed Aug 29, 2026

View reviewed changes

neSpecc reviewed Sep 16, 2026

View reviewed changes

Reversean requested changes Sep 21, 2026

View reviewed changes

Comment thread packages/ui-kit/src/popover/popover-desktop.ts

Comment thread packages/ui-kit/src/popover/popover-inline.ts

Comment thread packages/ui-kit/src/popover/popover-desktop.ts

Comment thread ...ages/ui-kit/src/popover/components/popover-item/popover-item-default/popover-item-default.ts

gohabereg and others added 6 commits September 21, 2026 23:46

Reversean approved these changes Sep 22, 2026

View reviewed changes

Navigation Menu

Fix popover focus-management gaps and add active-descendant support - #40

Fix popover focus-management gaps and add active-descendant support#40
gohabereg wants to merge 21 commits into
mainfrom
feature/popover-a11y

gohabereg commented Jul 26, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Reversean left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

neSpecc Sep 16, 2026

Uh oh!

neSpecc commented Sep 16, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		"test:e2e": "playwright test",
		"test:e2e:ui": "playwright test --ui"

		/** Focus should return to whatever the popover was opened from */
		this.previouslyFocusedElement = document.activeElement instanceof HTMLElement ? document.activeElement : null;

Conversation

gohabereg commented Jul 26, 2026

Summary

A11y features added

Breaking UX/UI changes

API changes

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reversean left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (7)

Uh oh!

neSpecc Sep 16, 2026

Choose a reason for hiding this comment

Uh oh!

neSpecc commented Sep 16, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants