Repository navigation
feat(remix)!: let a real button trigger RemixMenu - #337
Open
leoafarias wants to merge 2 commits into
Open
leoafarias wants to merge 2 commits into
leoafarias wants to merge 2 commits into
Conversation
Add RemixMenuTrigger.control, whose builder receives a RemixMenuTriggerHandle (isOpen, controller, focusNode, toggle()) so a styled button such as RemixButton can be the menu trigger. RemixMenu passes openOnTap: false to NakedMenu, owns a FocusNode for the control when the caller supplies none, and merges the expanded state onto the control's semantics. Overlay, Escape, outside taps, focus return, the controller, and onOpen/onClose/onSelected are unchanged. RemixMenuTrigger.label becomes String? because a control trigger has no label; the unnamed and .builder constructors still require one. MenuStyler.trigger and RemixMenu.semanticLabel do not apply to a control trigger, and a semanticLabel asserts. Also pin RemixPopover(openOnTap: false) with a button child: the wrapper Focus is an extra tab stop and takes focus back when the popover closes.
NakedMenu now merges the openOnTap: false control's semantics and the expanded state into one node itself, so RemixMenu no longer wraps the menu in MergeSemantics; the outer wrapper double-merged. Document that a control trigger's builder must return a single control, correct the focusNode docs (it receives focus when the menu closes; on open focus moves into the menu panel), and say the control field holds the builder. Replace the hollow keyboard half of the disabled-control test with a check that the control cannot take focus, and make the caller-supplied-node tests use the supplied node instead of an always-true identity check. Drop the RemixPopover(openOnTap: false) pin tests: a separate change fixes that naked_ui bug and asserts the correct behavior.
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. |
4 of 5 tasks
This branch has not been deployed
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
Stacked on #335 (
feat/naked-ui-menu-open-on-tap, addsNakedMenu.openOnTap); review and merge that first, then retarget this tomain.Adds
RemixMenuTrigger.controlso a real button, for example a preset's styledRemixButton, can be the menu trigger. Until nowNakedMenualways wrapped the trigger in its ownNakedButton, so a button returned fromRemixMenuTrigger.builderproduced two focus stops and two button nodes.RemixMenuTriggerHandlewithisOpen,controller,focusNode, andtoggle().RemixMenupassesopenOnTap: falseand the effective focus node toNakedMenu, creating and disposing a node only when the caller supplies notriggerFocusNode.focusNodeis the node that receives focus when the menu closes; on open, focus moves into the menu panel.NakedMenumerges the control's semantics and the expanded state into one node, so a screen reader sees one expandable button. Do not wrap the control in your ownMergeSemantics, and return a single control from the builder: everything it returns is merged into one node, so a split button or a loading button would merge its labels and actions.onOpen/onClose/onSelectedare unchanged.RemixMenuTrigger.labelbecomesString?because a control trigger has no label; the unnamed and.builderconstructors still require one.MenuStyler.triggerandRemixMenu.semanticLabeldo not apply to a control trigger; asemanticLabelasserts.triggeruntouched, so there is no codegen and no preset change.Release: the remix beta that ships this needs naked_ui 1.3.0 published first; the
naked_uifloor bump belongs to the naked_ui release PR.Validation
packages/remixfull suite (2850), the naked_ui menu, menubar, and semantics suites,dart analyze(remix, registry_source), rootdart format,melos run ci:checks,melos run docs:check.RemixButtonand a MaterialTextButtonthat forms its own semantics node); a disabled control does not open and cannot take focus; owned node disposed and a supplied one left alone;semanticLabelasserts. Each fails when its source line is reverted or mutated.Related Issues
Depends on #335.
Checklist
///).Breaking Change
RemixMenuTrigger.labelis nowString?. Code that readstrigger.labeland needs a non-null value must handle null (listed under Breaking changes in the CHANGELOG).