feat: per-page command registration API for command palette - #1710
Merged
1nonlypiece merged 1 commit intoJul 30, 2026
Merged
Conversation
Neither CommandPalette nor CommandPaletteProvider exist in this repo currently (both were among the ~2,500 files deleted in a prior incident; root layout.tsx already references <CommandPaletteProvider /> but doesn't compile for unrelated reasons). Building both fresh, with the registration API this issue actually asks for, rather than a hard-coded command list. - CommandPaletteProvider: holds open/close state, a small static navigation command set, and a registry of dynamically-registered commands. Merges both (deduped by id, dynamic wins on collision) into the commands exposed via context. Global Cmd/Ctrl+K toggles it. - useRegisterCommands(commands): registers on mount, unregisters on unmount. Re-registers only when the *set of ids* changes (not on every render) so passing a non-memoized array each render is safe -- this avoids a register -> state update -> re-render -> re-register loop that an id-agnostic (array-reference-based) design hits in practice (found via an actual OOM crash while writing this PR's own tests). Trade-off documented in the docstring and in docs/COMMAND_REGISTRATION.md. - CommandPalette: the searchable list UI, built on the existing Dialog primitive. Keyboard-navigable via aria-activedescendant (Up/Down/ Enter on the input); mouse click as a secondary affordance. Closes Commitlabs-Org#918
|
@mikewheeleer is attempting to deploy a commit to the 1nonly's projects Team on Vercel. A member of the Team first needs to authorize it. |
1nonlypiece
pushed a commit
that referenced
this pull request
Jul 30, 2026
Adds an accessible breadcrumb trail derived from the current route's path segments, for nested routes like /commitments/[id] -- there's no AppShellLayout to mount this in globally yet (see #921/#922's sibling PRs #1710/#1711 for the same constraint), so it's a small standalone component wired into the one real nested route in the app. - Breadcrumbs derives its trail from usePathname(), hides itself on top-level routes (0-1 segments) where a trail adds no value, renders non-final segments as real <Link>s (keyboard-focusable in document order for free) and the final segment as aria-current="page" (never a link, never duplicates the page H1). - currentLabel lets a page resolve its trailing id segment to a friendly label (e.g. "Balanced Commitment"); falls back to a truncated id for long opaque segments or a title-cased label for short/hyphenated ones. - Wired into the commitment detail page, passing the commitment's type as the friendly label. Closes #922
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.
Summary
Context: as noted on #924/#968, this repo has a large amount of previously-deleted UI infrastructure from a prior incident. Neither
CommandPalettenorCommandPaletteProviderexist currently -- both were among the deleted files (rootlayout.tsxalready references<CommandPaletteProvider />but doesn't compile, for unrelated reasons, same incident). Per maintainer guidance, building both fresh rather than resurrecting the old implementation, with the registration API this issue actually asks for from day one rather than a hard-coded list.What this adds
CommandPaletteProvider-- holds open/close state, a small static navigation command set, and a registry of dynamically-registered commands. Merges both into thecommandsarray exposed via context (deduped by id, dynamic registrations win on collision). GlobalCmd/Ctrl+Ktoggles the palette.useRegisterCommands(commands)-- registers on mount, unregisters on unmount.CommandPalette-- the searchable list UI, built on the existingDialogprimitive (focus trap, Escape-to-close,inertbackground). Keyboard-navigable via thearia-activedescendantpattern (Up/Down/Enter on the input, no need to tab into the list); mouse click is a secondary affordance.docs/COMMAND_REGISTRATION.md-- API reference, usage example, accessibility notes.A real bug found and fixed while building this
My first pass made
useRegisterCommands's effect depend on thecommandsarray reference (the "obvious" design). Writing this PR's own tests, I hit an actual out-of-memory crash: a component that both callsuseRegisterCommands([command])(a fresh array literal each render) and readscommandsback fromuseCommandPalette()enters an infinite loop -- register → state update → re-render → new array reference → re-register → ... This is an easy trap for real page code to fall into (e.g. a page showing "N commands available" alongside registering its own).Fixed by re-registering only when the set of ids changes (a stable primitive key derived from sorted ids), not on the array reference. A non-memoized array is now safe to pass every render. Documented the trade-off (in-place content mutation on an unchanged id won't take effect -- give the command a new id, or memoize with
useMemo, if that's needed) in the hook's docstring and in the doc file.Testing
useRegisterCommands.test.tsx-- 8 tests: static defaults present, register-on-mount merges with static set, unregister-on-unmount, id-based dedupe (via directregisterCommandscalls), multiple simultaneous registrants, manualregisterCommandsreturns a scoped unregister fn, open/close/toggle, throws outside a provider.CommandPalette.test.tsx-- 6 tests: closed renders nothing, lists static commands when open, filters by query, "no matches" state, Enter runs the highlighted command and closes, click runs a command.git stashA/B comparison that this diff adds zero new build errors and zero new test failures elsewhere (baseline: 22 failed test files / 37 failed tests / 488 passed, unchanged except my +14 passing).Closes #918