Skip to content

fix(naked_ui): keep focus on the caller's control in a popover opened without tap - #338

Open
leoafarias wants to merge 3 commits into
fix/slider-lifecycle-geometryfrom
fix/naked-ui-popover-trigger-focus
Open

leoafarias wants to merge 3 commits into
fix/slider-lifecycle-geometryfrom
fix/naked-ui-popover-trigger-focus

Conversation

@leoafarias

@leoafarias leoafarias commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Description

NakedPopover(openOnTap: false) wrapped its child in an internal Focus(focusNode: returnNode). With a caller's button as the child, that wrapper added a tab stop ahead of the button, took focus back when the popover closed instead of the button, and made triggerFocusNode and the button's own focusNode collide (passing one node as both threw "Cannot get renderObject of inactive element"). RemixPopover(openOnTap: false, child: RemixButton(...)) inherited all of it, and a screen reader landed on an unlabelled focusable wrapper after the popover closed.

With openOnTap: false the child is now the caller's own control, mirroring NakedMenu.openOnTap: false (#335): the popover adds no Focus, and triggerFocusNode (or a Focus child's node) goes only to RawMenuAnchor.childFocusNode. Attach triggerFocusNode to the control, using the same node as its focusNode. Without a node, closing returns focus to whatever had it before the popover opened; inside another popover or menu overlay, RawMenuAnchor focuses the node only for the outermost anchor. The default openOnTap: true is unchanged.

Documents the pattern in NakedPopover and docs/components/popover.mdx, and adds a naked_ui CHANGELOG fix entry. Caller-owned mode explicitly does not promise automatic expanded/collapsed semantics. The Remix adapter documentation and its three button regressions are preserved in draft #352 for Agent B’s #339 batch.

Merge note: sharing one node between triggerFocusNode and the control's focusNode throws on published naked_ui 1.2.3, and the docs now recommend it (with a "needs naked_ui 1.3.0 or later" note). Merge this right before the naked_ui 1.3.0 release, and raise remix's naked_ui floor with that release.

Validation

Pinned Flutter 3.44.0 validation on the rebuilt release stack: full naked_ui suite 1,157 passed / 3 skipped, all melos run ci:checks passed, and git diff --check passed. The isolated Remix draft applied above this stack passes 204 adapter tests.

Mutation evidence (all runtime failures, no compile failures): ignoring the marked rail fails all 9 geometry tests; retaining the pointer transaction through configuration changes fails 11 lifecycle tests; removing excluded-semantics focus fails its keyboard test (expected [51], got no change); restoring the old focus-list comparison fails 5 lifecycle tests; restoring the old popover docs promise fails the docs/runtime probe (documented collapsed state versus runtime Tristate.none). Restoring the fixes passes all 48 focused tests. Mutation probes and logs remain uncommitted under .context/.

  • dart analyze (no new issues), root dart format, melos run ci:checks, melos run docs:check.

Related Issues

Found while building the Remix menu control trigger (#337).


Checklist

  • My PR includes unit or integration tests for all changed/updated/fixed behaviors.
  • I have updated or added relevant documentation (doc comments with ///).
  • I am prepared to follow up on review comments in a timely manner.

Breaking Change

  • Yes, this is a breaking change.
  • No, this is not a breaking change.

No API change. With openOnTap: false the popover no longer attaches triggerFocusNode itself, so a child that cannot take focus (such as Text, or a disabled button the wrapper used to make focusable) is no longer a tab stop, a node not attached to the control gets no focus, and with a Focus child and a different triggerFocusNode the child's own node is the tab stop. These match the existing meaning of openOnTap: false (the caller owns activation and focus).

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-29T21:42:18.897194Z c976353 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

leoafarias and others added 3 commits October 29, 2026 21:40
… without tap

NakedPopover(openOnTap: false) wrapped its child in an internal Focus bound
to the return node. A button passed as the child therefore had an extra tab
stop ahead of it, focus returned to the wrapper instead of the button when
the popover closed, and passing one node as both triggerFocusNode and the
button's focusNode threw "Cannot get renderObject of inactive element".

With openOnTap: false the child is now the caller's own control: the popover
adds no Focus, and triggerFocusNode (or a Focus child's node) goes only to
RawMenuAnchor.childFocusNode, as NakedMenu does. Without a node, closing
returns focus to whatever had it before opening. The default openOnTap: true
is unchanged. RemixPopover(openOnTap: false) inherits the fix.
…control

Scope the triggerFocusNode doc to openOnTap: false and note that a nested
popover only gets its node focused by the outermost RawMenuAnchor. Name the
behavior changes in the CHANGELOG, hold the example's FocusNode in a State and
dispose it, drop a test the other shared-node tests already cover, and note
that sharing one node needs naked_ui 1.3.0 or later.

This branch has not been deployed

No deployments
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