Repository navigation
feat(naked_ui): let NakedMenu use the caller's control as its trigger - #335
Closed
leoafarias wants to merge 3 commits into
Closed
leoafarias wants to merge 3 commits into
leoafarias wants to merge 3 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d18e1b75c8
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This was referenced Oct 27, 2026
leoafarias
force-pushed
the
feat/naked-ui-menu-open-on-tap
branch
from
October 29, 2026 19:44
7835f6e to
e2af2f0
Compare
leoafarias
pushed a commit
that referenced
this pull request
Oct 29, 2026
Add NakedMenu.openOnTap (default true). When false the menu draws no NakedButton and adds no tap, keyboard activation, or focus stop: the child or builder is the caller's own control, and opening and closing go through the controller. triggerFocusNode is passed only to RawMenuAnchor .childFocusNode, and Semantics(expanded:) still merges onto the control's node. Using it inside a NakedMenubar asserts.
…enOnTap guards With openOnTap false, merge the caller's control into one semantics node that carries the expanded state, so it lands on the button for Material buttons and for controls that are semantics boundaries, and so a NakedTooltip around the control stays attached to the tree. Assert that semanticLabel is null when openOnTap is false, and limit the menubar assertion to direct menubar items instead of every menu below the bar's scope. Correct the focus docs: focus returns to triggerFocusNode when the menu closes. Add focus-return tests for item selection, outside tap, and controller.close().
leoafarias
force-pushed
the
feat/naked-ui-menu-open-on-tap
branch
from
October 29, 2026 21:38
e2af2f0 to
cab015d
Compare
This was referenced Oct 29, 2026
Collaborator
Author
|
Closing as superseded by #354, which merged on Oct 30, 2026. |
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.
Description
Adds
NakedMenu.openOnTap(defaulttrue).NakedMenualways wrapped its trigger in its ownNakedButton, so a real button (a design-systemButton) as the trigger produced nested buttons: two focus stops, duplicate button semantics, and the inner tap winning the gesture arena.With
openOnTap: falsethe menu draws no trigger button:child/builderis the caller's own control: noNakedButton, no menu-owned tap or keyboard activation, no focus stop.controllerand attachestriggerFocusNodeto the control; focus returns to that node when the menu closes. The node goes only toRawMenuAnchor.childFocusNode, with no wrapperFocus.MergeSemantics(Semantics(expanded:))), which works forNakedButton-based controls, Material buttons, and controls wrapped inSemantics(container: true)orMergeSemantics; aNakedTooltiparound the control stays attached to the semantics tree. Callers do not need their ownMergeSemantics.semanticLabelmust be null in this mode (asserted); label the control.NakedMenubar; a menu inside a bar menu's dropdown panel is not affected.It mirrors
NakedPopover.openOnTap; the default is today's behavior. This is the Naked prerequisite for the stacked Remix PR addingRemixMenuTrigger.control.Adversarial and simplification review — 2026-10-28
A menu nested in a menubar panel incorrectly inherited direct bar membership: focusing a caller-owned nested button opened it and contaminated bar traversal. Resolve actual direct membership once and reuse for activation, registration, semantics and assertion. Two regressions fail before; 58 menu/menubar tests pass corrected. No new API.
Independent review found no additional actionable issues in this delta. The strict combined gate on exact
673409ab2passes all 8 CI legs plus bootstrap and registry build/drift, exits 0 with zero failed steps, and leaves the initial/final commit and clean tree unchanged (Flutter 3.44.0 pinned through child-tool PATH). Full-suite counts: naked_ui 1126 (3 existing skips), remix 3141, registry_source 2430; Vanilla/Fortal/HeroUI/Lucide fresh installs all pass, including the 17-test HeroUI fixture. These counts belong to the corrected combined tree; earlier counts below are historical owning-branch evidence. GitHub Checks separately validate this PR's own pushed head. Release/promotion requirements are unchanged.Earlier validation (before the 2026-10-28 review)
naked_menu_test.dart(control mode, focus return on Escape, item selection, outside tap, andcontroller.close(),semanticLabeland menubar asserts),naked_menu_semantics_test.dart(one button node with expanded false/true forTextButton,Semantics(container:), andMergeSemanticscontrols), andupdate_tree/naked_menu_semantics_update_test.dart(tooltip-wrapped controls with the fix(naked_ui): the select trigger's merged node is the traversal parent, not an inner container #261 detached-node spy). The semantics, assert, tooltip, and menubar-panel tests fail when their fix is reverted.dart format,melos run ci:checks;dart analyzeno new issues.Related Issues
Prerequisite for the Remix menu control trigger (stacked PR).
Checklist
///).Breaking Change