ref(cmdk): Remove onAction prop in favor of internal action execution#112648
Merged
ref(cmdk): Remove onAction prop in favor of internal action execution#112648
Conversation
…gistration Introduces a makeCollection<T>() factory in ui/collection.tsx that creates isolated, fully-typed collection instances. Each instance tracks a flat node map and a parent→children index, exposing a tree(rootKey?) API for structured traversal. A node becomes a group by wrapping its children in GroupContext.Provider — there is no separate group/item type distinction. Data fields are spread directly onto tree nodes so consumers can access them without any cast. ui/cmdk.tsx builds the CMDK-specific layer on top: CMDKCollection, CMDKGroup, and CMDKAction. Groups and actions share a single CMDKActionData union type. CMDK_PLAN.md tracks the remaining migration steps from the old reducer-based registration system. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Wire CMDKQueryContext and CMDKProvider so async CMDKGroup nodes can read
the current search query. Mount CMDKProvider inside CommandPaletteStateProvider
in the existing CommandPaletteProvider so the collection store is live for the
full app lifetime.
Add GlobalCommandPaletteActions component that registers all global actions
into the CMDK collection via JSX (CMDKGroup / CMDKAction) rather than the old
useCommandPaletteActionsRegister reducer hook. Both systems run in parallel
during this transition step.
Swap the CommandPalette UI to read from CMDKCollection.useStore() instead of
the old useCommandPaletteActions() reducer pipeline. Remove collectResourceActions,
asyncQueries, and asyncChildrenMap — async fetching is now handled inside
CMDKGroup before nodes appear in the tree. Rewrite scoreTree and flattenActions
to work with CollectionTreeNode<CMDKActionData> using direct field access.
Replace the linked-list navigation stack (which stored full action objects)
with CMDKNavStack which stores only the group key and label. Push action now
dispatches {key, label} instead of a full CommandPaletteActionWithKey.
Update modal.tsx, stories, analytics hook, and tests to use the new types.
Fix pre-existing duplicate Item declaration in commandPalette.tsx that caused
a Babel parse error in the test suite.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Remove CMDK_PLAN.md (internal planning doc, not production code) - Rename poc.spec.tsx → collection.spec.tsx (no longer a PoC) - Remove dead useGlobalCommandPaletteActions() call from navigation — commandPalette.tsx now reads from CMDKCollection.useStore(), so the old context registration had no effect - Rewrite stories demo to use CMDKAction/CMDKGroup JSX API; the old useCommandPaletteActionsRegister path fed CommandPaletteActionsContext which nothing reads anymore, so the demo was showing an empty palette Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
GlobalCommandPaletteActions covers everything the old hook registered. The hook had no remaining callers after the previous commit removed the navigation/index.tsx call site. Also cleans up cmdk.tsx comments and renames GroupContext to Context in the collection factory API. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
The old context.tsx served as a registration hub for the reducer-based action system. With that system removed, it only wrapped two providers. Move CommandPaletteProvider directly into cmdk.tsx alongside the other palette primitives and delete the now-empty file. Also remove unused WithKey type variants from types.tsx. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
Move GlobalCommandPaletteActions from globalActions.tsx into the ui/ directory alongside the other palette files, and render it inside the modal rather than in the navigation layout. The global actions now mount and unmount with the palette itself instead of running in the navigation tree at all times. CommandPalette stays a generic rendering component with no app-specific dependencies, keeping it fully testable in isolation. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
Add a children prop so callers can pass CMDKGroup/CMDKAction trees directly to CommandPalette. The modal uses this to inject GlobalCommandPaletteActions, keeping the component itself free of app-specific dependencies. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
CMDKAction now accepts LocationDescriptor for the to prop, so String() is no longer needed and was triggering a no-base-to-string lint error. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
…onContext The modal renders outside SecondaryNavigationContextProvider, so useSecondaryNavigation() threw on open. Read the context directly and skip the sidebar toggle action when the context isn't available. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
Moving it into the modal changed its React scope — the modal renders in a portal outside SecondaryNavigationContextProvider, causing a crash. Since CommandPaletteProvider is at the app root, CMDKCollection is shared across the whole tree. GlobalCommandPaletteActions can register from the navigation (where it has proper context) and the modal reads the same store. The children prop on CommandPalette is still useful for page-scoped actions. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
…ing providers Add a test asserting that page slot actions appear before global actions in the command palette list. The test currently fails — it documents the known bug where collection insertion order (driven by React mount order) determines list position instead of slot priority. Also fixes two pre-existing issues uncovered while writing the test: - GlobalActionsComponent in the spec was missing CommandPaletteSlot.Provider, causing all existing tests to throw 'SlotContext not found' at render time. - GlobalCommandPaletteActions was registering actions directly into the CMDKCollection without going through a slot consumer. It now wraps its output in <CommandPaletteSlot name="global">, making all action registration slot-aware and setting up the path toward DOM-order-based priority sorting. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Documents the approach for fixing slot ordering in the command palette: store a ref to the outlet DOM element on each registered action node, then pre-sort the collection output via compareDocumentPosition before passing to flattenActions. Covers the ref-threading chain (shared SlotRefsContext → outlet populates → consumer wrapper injects → node stores), the new commandPaletteSlotRefs.tsx file needed to avoid circular imports, and all six files that need changing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Pre-sort the root-level CMDK collection nodes by the DOM position of their slot outlet element before flattening, so that task > page > global priority is preserved regardless of React mount order. The slot library is extended with two additions: - SlotConsumer now provides OutletNameContext so portaled children see which slot they belong to (previously only the outlet provided it) - useSlotOutletRef() hook on SlotModule returns a stable RefObject whose .current is always the current slot's outlet element CommandPaletteSlot is extracted to commandPaletteSlot.tsx to avoid a circular import between cmdk.tsx (which needs the hook) and commandPalette.tsx (which imports the collection). CMDKAction and CMDKGroup call CommandPaletteSlot.useSlotOutletRef() and store the result as ref in their node data. presortBySlotRef in commandPalette.tsx then uses compareDocumentPosition on those elements to establish the correct order. Nodes outside any slot wrapper (ref is null) sort after all slotted nodes and preserve their relative order. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extend CMDKActionData with a ref field that stores the slot outlet element. CMDKAction and CMDKGroup call useSlotOutletRef and include it in their registered node data. commandPalette.tsx applies presortBySlotRef to collection nodes before flattening, ordering by the outlet DOM position (task > page > global). Callers use CommandPaletteSlot directly — no wrapper components needed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CMDKGroup was overwriting any enabled field returned by the resource function with a static `!!resource` check. This meant the DSN lookup query always fired regardless of whether the query matched a DSN pattern. Fix CMDKGroup to respect the resource's own enabled field, then add `enabled: DSN_PATTERN.test(query)` to the DSN lookup resource so the API call only fires when the query looks like a DSN. Also remove an unused GlobalCommandPaletteActions import in navigation/index.tsx. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the useCommandPaletteActions hook example with the new JSX component API using CMDKAction, CMDKGroup, and CommandPaletteProvider. Also adds a section documenting the async resource pattern. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…onent (#112563) ## Changes Merges `CMDKGroup` and `CMDKAction` into a single `CMDKAction` component. The distinction between the two was artificial — a node becomes a group simply by having children registered under it, which is already documented in the `CMDKActionData` type comment. Keeping them separate contradicted that stated design intent. The merged `CMDKAction` covers all four cases: | Usage | How | |-------|-----| | Navigation | `<CMDKAction display={...} to="/path/" />` | | Callback | `<CMDKAction display={...} onAction={fn} />` | | Group with children | `<CMDKAction display={...}><CMDKAction .../></CMDKAction>` | | Async resource group | `<CMDKAction display={...} resource={queryFn}>{data => ...}</CMDKAction>` | `CMDKActionDataTo.to` is widened from `string` to `LocationDescriptor` to match `CommandPaletteAsyncResult` and the existing callsites. Co-authored-by: Claude Sonnet 4 <noreply@anthropic.com>
Two small fixes for the JSX-powered command palette. **Empty groups are now omitted from the list.** A `CMDKGroup` that renders no children ends up with zero registered nodes in the collection. In browse mode, `flattenActions` was treating such nodes as leaf actions and showing them. These are headless section headers with nothing to show — they should be invisible. The fix skips any node that has no children and no executable action (`to` / `onAction`). **Scroll resets to the top when the query changes.** Typing a new search query now scrolls the results list back to the top. The tricky part: `ResultsList` is not the actual scroll element — `ListBox` creates its own inner `div` that it hands to TanStack Virtual as the scroll container. The fix adds a `scrollContainerRef` prop to `ListBox` that gets merged into that inner container's refs, then wires it up in the `onChange` handler. Stacked on #112262. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
When a CMDKAction has both an onAction callback and children, selecting it now invokes the callback immediately and keeps the modal open so the user can choose a secondary action from the children. Previously, actions with children were treated purely as navigation groups — the onAction callback was silently ignored. Now the callback fires before the palette pushes into the child list. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The modal was calling the imported closeModal() from actionCreators/modal
directly, which goes straight to ModalStore.closeModal() and bypasses
GlobalModal's internal closeModal wrapper. That wrapper is the only place
options.onClose is invoked — which is the closeCommandPaletteModal callback
that dispatches {type: 'toggle modal'} to reset state.open.
With state.open stuck at true after an action closed the palette, the next
hotkey press would see open===true and treat it as a close request rather
than an open request, requiring a second press to actually reopen.
Fix by destructuring closeModal from ModalRenderProps and using that
instead of the imported version.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Instead of joining label, details, and keywords into a single string and running fzf once, score each field independently and return the best match. This has two benefits: - Avoids false cross-field subsequence matches that could arise when the tail of one field and the head of the next happen to satisfy a pattern across the join boundary. - Allows fzf's built-in exact-match bonus to fire naturally (e.g. when the query equals the label exactly), without needing a manual boost at the call site. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
…coercion - Restore admin/superuser action group (Open _admin, Open org in _admin, Open Superuser Modal, Exit Superuser) for staff users; these existed in the old useGlobalCommandPaletteActions hook but were not ported to the new GlobalCommandPaletteActions component - Add missing IconList to DSN_ICONS so the third getDsnNavTargets result (Client Keys/DSN) renders with an icon - Remove String(action.to) coercion in modal.tsx and the test helper; both normalizeUrl and navigate already accept LocationDescriptor, and String() would corrupt object-form descriptors to "[object Object]" - Remove redundant CommandPaletteSlot.Provider from the test helper since CommandPaletteProvider already wraps children in one Co-Authored-By: Claude <noreply@anthropic.com>
…alias Replace the ActionsToJSX bridge component in tests with direct JSX CMDKAction registrations. Remove the CommandPaletteAsyncResult type alias in favour of CommandPaletteAction, which is the same shape and avoids the unnecessary split. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add modifier key context to command palette actions so internal links can open in a new tab when selected with Shift. Keep link indicator rendering and selection handling aligned with the new action metadata. Split the tests so commandPalette covers modifier forwarding and modal covers the actual link-opening behavior. This avoids duplicating modal URL handling logic inside the palette tests. Co-Authored-By: OpenAI Codex <noreply@openai.com>
Remove the click-specific shift coverage and rely on the simpler global modifier key tracking in the command palette. This keeps the tests focused on the supported keyboard-driven selection path. Also drop the now-unneeded click capture handlers from the palette so modifier handling stays centralized in the key-state effect. Co-Authored-By: OpenAI Codex <noreply@openai.com>
The /_admin/ actions were converted to 'to' props, causing isExternalLocation to return false (same origin) and navigate() to attempt client-side routing to Django-served admin pages. Switch both admin CMDKActions to onAction with window.open so they open in a new tab as originally intended. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ocationUtils Both commandPalette.tsx and modal.tsx contained identical copies of these helpers. Move them to a shared locationUtils.ts module to eliminate duplication. Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
CommandPalette now handles all action execution internally — navigating for 'to' actions (with shift-key new-tab support) and invoking the action-defined onAction callback for callback actions. Callers no longer need to implement action dispatch logic; the only integration point is the optional closeModal prop. This removes the indirection of consumers inspecting CollectionTreeNode shapes at call sites and centralises the execution model inside the component. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
github-actions
bot
added
the
Scope: Frontend
…Palette Both helpers were only used in commandPalette.tsx after the onAction removal. The separate module added indirection without reuse benefit, so inline the two functions and delete locationUtils.ts. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rbro112
approved these changes
Apr 11, 2026
JonasBa
force-pushed
the
jb/cmdk/item-links
branch
from
c42400e to
41c1b04
Compare
Key resolutions: - Keep internal action handling in CommandPalette (no onAction prop) - Take master's prompt support, cmdkQueryOptions, useIsFetching, isEmptyResourceNode - Take master's slot() providers option and refactored makeSlotConsumer API - Adapt tests to preserve our internal-navigation approach
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit aa36d4b. Configure here.
…rchitecture Outlets are now rendered in navigation/index.tsx (via CommandPaletteSlotOutlets), not inside CommandPalette. CommandPalette no longer accepts children. Updated tests to render SlotOutlets separately and updated stories/docs to reflect the new structure.
wedamija
pushed a commit
that referenced
this pull request
Apr 13, 2026
…#112648) Removes the onAction prop from commandPalette which was requiring us to redefine the test and prod logic and diminishing the value of our tests. With this change, the action.onAction or action.to are invoked internally by the command palette. --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: OpenAI Codex <noreply@openai.com>
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.
Removes the onAction prop from commandPalette which was requiring us to redefine the test and prod logic and diminishing the value of our tests. With this change, the action.onAction or action.to are invoked internally by the command palette.