Skip to content

fix(cmdk): Keep CMDKAction nodes mounted across modal open/close cycles#112650

Merged
JonasBa merged 2 commits intojb/cmdk/jsx-pocfrom
jb/cmdk/stable-register
Apr 10, 2026
Merged

fix(cmdk): Keep CMDKAction nodes mounted across modal open/close cycles#112650
JonasBa merged 2 commits intojb/cmdk/jsx-pocfrom
jb/cmdk/stable-register

Conversation

@JonasBa
Copy link
Copy Markdown
Member

@JonasBa JonasBa commented Apr 10, 2026

Move CommandPalette Slot outside of the conditionally rendered CommandPalette which makes the action registration stable and enables the user to toggle the modal on/off without losing state

Selecting a group action and then closing and reopening the command palette
caused the palette to show an empty state. The nav stack stored a useId()
key that was tied to the CMDKAction component instance. When the modal
closed, those instances unmounted and unregistered their keys from the
collection store. On reopen, new instances generated new keys, leaving
the stored key pointing to nothing.

Fix by moving the slot outlets from CommandPalette (mounted only when the
modal is open) into CommandPaletteProvider (always mounted). The outlets
are rendered in a display:none container in task → page → global DOM
order so that presortBySlotRef's compareDocumentPosition ordering is
preserved. GlobalCommandPaletteActions is moved to the navigation
component where it has org context and is always mounted, portaling into
the provider's persistent global outlet. As a side effect, page-slot
actions registered from page components are also stable across modal
cycles for the same reason.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Apr 10, 2026
CommandPaletteProvider previously rendered a hidden div with three slot
outlet elements. Because the global test wrapper includes
CommandPaletteProvider, that hidden div showed up in every test's
container, breaking tests that assert toBeEmptyDOMElement().

Move the outlets into UserAndOrganizationNavigation in navigation/index.tsx
so they only exist when the full nav is mounted. CommandPaletteProvider
becomes a pure context provider with no DOM output. Slot consumers and
presortBySlotRef already degrade gracefully when outlets are absent, so
tests unrelated to CMDK are unaffected.

Slot-specific tests in commandPalette.spec.tsx and modal.spec.tsx now
render a local SlotOutlets component explicitly, making their dependency
on outlets visible rather than relying on a hidden side-effect in the
provider.

Co-Authored-By: Claude <noreply@anthropic.com>
@JonasBa JonasBa marked this pull request as ready for review April 10, 2026 08:24
@JonasBa JonasBa requested a review from a team as a code owner April 10, 2026 08:24
@JonasBa JonasBa merged commit 88b2723 into jb/cmdk/jsx-poc Apr 10, 2026
59 checks passed
@JonasBa JonasBa deleted the jb/cmdk/stable-register branch April 10, 2026 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant