Skip to content

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

Open
gohabereg wants to merge 1 commit into
feature/popover-a11yfrom
feature/popover-search-and-flippable
Open

Keep search results and nested-level keys in sync on mobile#41
gohabereg wants to merge 1 commit into
feature/popover-a11yfrom
feature/popover-search-and-flippable

Conversation

@gohabereg

Copy link
Copy Markdown
Member

Stacked on #40 — review that one first, this diff only makes sense on top of it.

Two gaps I reported while reviewing #40 and deliberately left out of it, since that branch is already large enough that the reviewer asked for it to be split.

Search results went stale when the item list changed

addItem() / removeItemByName() updated the searchable set but never re-ran the query, so an item added while a query was typed joined the results whether it matched or not, and the announced count went with it. Note this predates #40 — the old addItem appended the element directly and was equally unfiltered.

SearchInput.reapplyQuery() re-runs whatever is already in the field, and PopoverDesktop calls it after either mutation.

PopoverMobile ignored children.isFlippable

Mobile renders nested levels into the same panel rather than into a popover of their own, so the Flipper navigating the root list carried on claiming the arrows and Enter after drilling in. An item built around a text input — the link-tool URL field pattern — could never receive them: preventDefault() on the arrows moved focus off the field, and Enter clicked whatever the cursor was on instead of submitting.

PopoverDesktop has honoured this since before #40, by passing flippable: item.isChildrenFlippable to the nested popover it creates. Mobile had no keyboard navigation at all until #40, so nothing regressed — the flag simply had nowhere to apply. It now travels with the level through PopoverStatesHistory, and a level that opts out leaves the Flipper deactivated, its items becoming individual stops of the panel's Tab trap instead.

Tests

184 passing across chromium and webkit. Two new, plus a mobile-nested-input fixture.

Both were checked against the parent branch's source to confirm they fail without the fix — the isFlippable one initially passed regardless, because the nested level had a single item and the Flipper's arrow simply wrapped back to it; the fixture now has a second stop so the focus has somewhere to be stolen to.

🤖 Generated with Claude Code

Two gaps reported while reviewing the a11y work, left out of that branch to
keep it reviewable.

Search results went stale whenever the item list changed underneath them:
addItem()/removeItemByName() updated the searchable set but never re-ran the
query, so a newcomer joined the results whether it matched or not and the
announced count went with it. SearchInput.reapplyQuery() re-runs whatever is
already typed, and PopoverDesktop calls it after either mutation.

The mobile popover ignored children.isFlippable. It renders nested levels
into the same panel rather than into a popover of their own, so the Flipper
navigating the root list carried on claiming the arrows and Enter after
drilling in - an item built around a text input could never receive them.
The flag now travels with the level through PopoverStatesHistory, and a
level that opted out leaves the Flipper deactivated, its items becoming
individual stops of the panel's Tab trap instead.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F3JEisQDVD82VYAp55Lhcp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant