fix(components): action:menu consumes autoTrigger, so an overflowed deep link still runs (#4162) - #4195
Merged
Merged
Conversation
…eep link still runs (#4162) `autoTrigger` was consumed only by `action:button`. `action:bar` splits its post-gate list at `maxVisible` (3 desktop / 1 mobile) and hands the tail to `action:menu`, which had no handling for the flag — so an auto-triggered action that sorted past the threshold was rendered as an ordinary "More" entry and never ran, while the caller had already spent the one-shot signal it stood for (measured `urlParam=null execute=0`, unrecoverable because the URL strip IS the consumption). Per the ruling on the card, the flag's contract is "execute once on mount by whichever renderer receives the action": the menu now consumes it by executing, through the same path a click takes, without opening the dropdown. The consumption point is the menu renderer (where the action provably arrives), not the items — Radix mounts those only once the dropdown opens. Once-ness has one implementation, lifted to `renderers/action/auto-trigger.ts` and shared by both renderers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
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
2 participants
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.
Fixes #4162
autoTrigger— the client-composed "run this action as soon as a renderer receives it" flag behind the #844 welcome-page deep link — was consumed only byaction:button.action:barsplits its post-gate list atmaxVisible(3 desktop / 1 mobile) and hands the tail toaction:menu, which had no handling for the flag at all. An auto-triggered action that sorted past that threshold was therefore rendered as an ordinary "More" entry and never ran, while the caller had already spent the one-shot signal it stood for: consumption of?runAction=create_environmentis modelled as stripping it from the URL, so the end state wasurlParam=null execute=0— no dialog, and no URL left to retry from.This is #4123's signature one layer deeper, and PR #4166 does not close it: arming keys on the post-gate list, which INCLUDES the actions the bar is about to move into the menu, so arming is correct and the strip happens exactly as intended. Verified on this branch's tip before implementing — the app-shell probe in this PR fails on
origin/mainwith the card's measurement.The ruling, as implemented
The delegated ruling on the card: the flag's contract is "execute once on mount by whichever renderer receives the action". So
action:menuconsumes it by EXECUTING, through the samehandleExecutea click on that item takes — not by rendering an affordance and hoping, and not by opening the dropdown, so a transport flag never moves what the user sees.No safety reason against execution-from-menu turned up in the structure, so the ruling's premise stands: the menu already owns a full execute path (confirm text, params,
resultDialog, toasts all forwarded), and the runner applies its own confirm / param-collection / entitlement gates identically for both renderers.Two placement decisions the structure forced, both documented at the call site:
DropdownMenuContent, which Radix mounts only when the dropdown OPENS — an effect there would wait on the very click the flag exists to avoid, and would make the trigger's open state, not the action, decide whether a deep link runs.packages/components/src/renderers/action/auto-trigger.ts;action:buttonnow calls the sameuseAutoTriggerOnce, and the menu instantiates it per action through a headlessActionAutoTriggerthat renders nothing (hooks cannot be called in a loop). One guard ref per action, mounted for every action rather than only the flagged ones, so the ref outlives a flag that flips — which is what keeps atrue → false → trueaction firing once, as the button's long-lived ref does.Where the boundary sits: container visibility governs mounting (a hidden
action:baroraction:menurenders no children and auto-triggers nothing), while the action's ownvisiblegate does not suppress the trigger. That second half is parity, not a new choice —action:buttondeclares its effect before itsvisibleearly return, so a gated-invisible action executes anyway (measured onorigin/main:rendered="" execute=1). Diverging here would have re-created this card's defect one predicate over, so the two renderers now agree and the question of whether what they agree on is right is filed as #4191.Tests
New pins in
packages/components/src/renderers/action/__tests__/action-overflow-autotrigger.test.tsx(12) andpackages/app-shell/src/environment/__tests__/EnvironmentListToolbar.deepLinkOverflow.test.tsx(4), the latter driving the real toolbar end-to-end on the card's exact shape (create action 4th,add_developmentstate so novariant: 'primary'floats it up). #4166's arming file is untouched and still green.Covered: the overflow probe executes exactly once; the dropdown stays closed (
aria-expanded="false", no menu content in the DOM); re-renders do not re-fire; a flag flipping true later fires once;systemActions(always in the menu, whatever the viewport) are honoured; the guard is per action, so two flagged actions each run once; and the refusal half — an overflow action WITHOUT the flag runs nothing, no deep link runs nothing, the inline path still belongs toaction:buttonalone (no double-fire).Reverse verification — the menu consumption reverted to
origin/main, everything else kept:The failure is the filing's signature verbatim —
AssertionError: expected "vi.fn()" to be called 1 times, but got 0 timeson the app-shell probe, i.e.execute=0with the param already stripped. Only the menu-side assertions move; every control pin stays green in both directions.Local runs (repo root,
--maxWorkers=2under the shared verification lock):Out of scope, filed
autoTriggerexecutes an action whose own declaredvisiblegate hides it — in both renderers #4191 —autoTriggerexecutes an action whose own declaredvisiblegate hides it, in both renderers (observation-class; the parity pin here points at it).action:menu's execute payload is narrower thanaction:button's:label,description,undoable,recordIdFieldare dropped, measured. It affects ordinary clicks on any overflow action, not just auto-triggered ones, so it is a pre-existing defect rather than something this change introduces. It does not blunt this fix — the runner accepts an arrayparamsas the collection definition, so the create dialog still opens; its title is the generic one.Changeset:
@object-ui/componentspatch. The app-shell change is a test file only.Generated by Claude Code