fix(ui): make command palette instant - #500
Conversation
Greptile SummaryThe PR makes the command palette appear and disappear without popup or list-resize animations while preserving the static backdrop. It also adds palette-local Command+K dismissal, simplifies immediate unmounting, removes the workbench’s unused Motion dependency, and adds focused keyboard and animation tests. Confidence Score: 5/5The PR appears safe to merge with no actionable correctness, security, or dependency-boundary issues identified. The local shortcut reads its popup owner correctly at dispatch time, the inert workbench owner prevents competing Command+K handling, and the removed Motion dependency has no remaining consumers in the workbench package.
|
| Filename | Overview |
|---|---|
| packages/presentation/ui/src/shell/command-palette.tsx | Removes Motion-based dialog and list transitions and adds a popup-scoped Command+K close binding without exposing a concrete regression. |
| packages/client/workbench/src/palette/command-palette.tsx | Replaces deferred AnimatePresence unmounting with direct conditional rendering so closure immediately resets palette-local state. |
| packages/presentation/ui/src/shell/tests/command-palette.test.tsx | Covers animation-class removal, Escape dismissal, and repeated Command+K dismissal. |
| packages/client/workbench/package.json | Removes the now-unused direct Motion dependency after eliminating the package’s final Motion import. |
| pnpm-lock.yaml | Keeps the lockfile importer aligned with the workbench manifest dependency removal. |
Reviews (1): Last reviewed commit: "fix(ui): make command palette instant" | Re-trigger Greptile
There was a problem hiding this comment.
Pull request overview
This PR makes the Command+K command palette feel instant by removing motion-driven open/close/list resize transitions, while keeping the dimmed backdrop, and updates keyboard handling so Command+K toggles the palette closed (with Escape still dismissing it). It also removes the workbench’s now-unused motion dependency.
Changes:
- Removed
motion/react-based backdrop/popup/list animations from the sharedCommandPaletteUI component and added a palette-local Command+K shortcut binding to close it. - Simplified the workbench palette container to mount/unmount directly (no
AnimatePresence) and dropped themotiondependency from the workbench package. - Added component-level tests verifying no animation classes, Escape dismissal, and Command+K toggle-close behavior.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Updates lockfile to reflect removal of motion from the workbench importer. |
| packages/presentation/ui/src/shell/command-palette.tsx | Removes motion transitions and adds palette-local Command+K close binding. |
| packages/presentation/ui/src/shell/tests/command-palette.test.tsx | Adds tests covering no-animation rendering and close behaviors (Escape, Command+K). |
| packages/client/workbench/src/palette/command-palette.tsx | Removes AnimatePresence and unmounts palette immediately on close. |
| packages/client/workbench/package.json | Removes motion dependency from the workbench package. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
ℹ️ Behavior looks correct — two rough edges worth a look, neither blocking.
Reviewed changes — read the full diff for all 5 files, then traced the keyboard-shortcut registry, the Base UI dialog internals, and the coss-ui flex/scroll chain, and ran the new test file both as-is and with the animations restored.
- Motion stripped from the palette dialog —
render={<motion.div/>}dropped from the backdrop and popup,useReducedMotion/dialogTransitionremoved, andpopupRefmoved ontoCommandDialogPrimitive.Popupas a plainref. - List-resize animation removed — the
useMeasuredHeightResizeObserverhook is deleted and the height-animating wrapper becomes a plain<div className="min-h-0">. - ⌘K now toggles the open palette closed — a new popup-local
useKeyboardShortcut(owner: popupRef) callsonOpenChange(false), relying on the workbench owner beingdata-base-ui-inertwhile the dialog is open. - Container simplified —
AnimatePresencein the workbench palette container becomes a plain ternary, andmotionis dropped from@linkcode/workbench's manifest and the lockfile. - New component tests — 3 jsdom tests covering backdrop rendering, Escape dismissal, and repeated ⌘K.
Things I checked that are clean, so nobody needs to re-litigate them:
- No
motionimports remain anywhere inpackages/client/workbench/src, so dropping the dependency is safe. (nodeLinker: hoistedmeans a stray import would still have resolved — the grep is the proof, not the install.) refonCommandDialogPrimitive.Popupresolves to the same<div>the oldrenderelement's ref did (DialogPopupmergesforwardedReffirst, anduseRenderElementdefaults to'div'). This matters because that ref is the shortcutowner.- Removing
renderdoes not reintroduce coss-ui'stransition-all duration-200 data-ending-style:opacity-0chrome — this file usesCommandDialogPrimitive.*directly, never theCommandDialogBackdropwrapper. - No scroll-lock,
inert, or focus-restore leak from unmounting rather than animating out; the wholeDialogRootsubtree is parent-unmounted, so effects clean up synchronously. - Removing the measured height does not break list scrolling. The deleted
heightcame fromCommandList's ownoffsetHeight, so it could never constrain the content more tightly than the content itself. What makes the list scroll is the flex-shrink chain, which this PR leaves untouched: popupflex flex-col max-h-105 min-h-0→ coss-uiCommandPanel(min-h-0) → the newmin-h-0div →ScrollArea.Root(size-full min-h-0) → viewport (h-full). Thatmin-h-0div is load-bearing, not leftover markup.
Claude Opus | 𝕏


Summary
CODE-648
Verification
pnpm check:cipnpm test— 3043 passed, 1 skippedChecklist
pnpm check:ciandpnpm testboth pass (pluscargo fmt/clippy/testfor Rust changes)WIRE_PROTOCOL_VERSIONis bumped