Skip to content

fix(ui): make command palette instant - #500

Merged
lucas77778 merged 1 commit into
masterfrom
chenyu/code-648
Sep 3, 2026
Merged

fix(ui): make command palette instant#500
lucas77778 merged 1 commit into
masterfrom
chenyu/code-648

Conversation

@lucas77778

Copy link
Copy Markdown
Member

Summary

  • remove open, close, and list-resize motion from the Command+K palette while retaining backdrop dimming
  • let the open palette handle Command+K itself so the shortcut toggles it closed; preserve Escape dismissal
  • remove the workbench's now-unused Motion dependency

CODE-648

Verification

  • pnpm check:ci
  • pnpm test — 3043 passed, 1 skipped
  • command-palette component tests — backdrop/no-animation, Escape close, repeated Command+K close
  • browser acceptance — zero popup/backdrop animations; Command+K toggle-close and Escape close observed

Checklist

  • pnpm check:ci and pnpm test both pass (plus cargo fmt / clippy / test for Rust changes)
  • I ran the affected surface and observed the change working
  • If a wire message changed: WIRE_PROTOCOL_VERSION is bumped
  • New code and assets are my own work, or their origin and license compatibility are noted above
  • Docs and comments are updated where behavior changed

Copilot AI lite review requested due to automatic review settings September 1, 2026 03:54
@linear-code

linear-code Bot commented Sep 1, 2026

Copy link
Copy Markdown

CODE-648

@greptile-apps

greptile-apps Bot commented Sep 1, 2026

Copy link
Copy Markdown

Greptile Summary

The 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/5

The 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.

Important Files Changed

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 shared CommandPalette UI 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 the motion dependency 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.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ 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 dialogrender={<motion.div/>} dropped from the backdrop and popup, useReducedMotion/dialogTransition removed, and popupRef moved onto CommandDialogPrimitive.Popup as a plain ref.
  • List-resize animation removed — the useMeasuredHeight ResizeObserver hook 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) calls onOpenChange(false), relying on the workbench owner being data-base-ui-inert while the dialog is open.
  • Container simplifiedAnimatePresence in the workbench palette container becomes a plain ternary, and motion is 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 motion imports remain anywhere in packages/client/workbench/src, so dropping the dependency is safe. (nodeLinker: hoisted means a stray import would still have resolved — the grep is the proof, not the install.)
  • ref on CommandDialogPrimitive.Popup resolves to the same <div> the old render element's ref did (DialogPopup merges forwardedRef first, and useRenderElement defaults to 'div'). This matters because that ref is the shortcut owner.
  • Removing render does not reintroduce coss-ui's transition-all duration-200 data-ending-style:opacity-0 chrome — this file uses CommandDialogPrimitive.* directly, never the CommandDialogBackdrop wrapper.
  • No scroll-lock, inert, or focus-restore leak from unmounting rather than animating out; the whole DialogRoot subtree is parent-unmounted, so effects clean up synchronously.
  • Removing the measured height does not break list scrolling. The deleted height came from CommandList's own offsetHeight, 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: popup flex flex-col max-h-105 min-h-0 → coss-ui CommandPanel (min-h-0) → the new min-h-0 div → ScrollArea.Root (size-full min-h-0) → viewport (h-full). That min-h-0 div is load-bearing, not leftover markup.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread packages/presentation/ui/src/shell/command-palette.tsx

@xiaoland xiaoland left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Image

@lucas77778
lucas77778 merged commit 16767b2 into master Sep 3, 2026
14 checks passed
@lucas77778
lucas77778 deleted the chenyu/code-648 branch September 3, 2026 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants