⌨️ fix: Make Scope Selector Operable with the Enter Key - #121
Open
dustinhealy wants to merge 4 commits into
Open
⌨️ fix: Make Scope Selector Operable with the Enter Key#121dustinhealy wants to merge 4 commits into
dustinhealy wants to merge 4 commits into
Conversation
Inline @clickhouse/click-ui in vitest server deps and exclude .claude/** from test discovery. click-ui 0.9.1 ships ESM that imports .css files, which Node's loader cannot parse, so Vitest must process the package through Vite.
The whole selector rendered inside cmdk's Command.Dialog, whose root keydown handler calls preventDefault on every Enter press and redispatches it to the aria-selected item. preventDefault cancels native button activation, so Enter did nothing on any button, and in the list view the redispatch activated the auto-highlighted first item. Since that item is Base configuration, pressing Enter on the Create button or in the empty search input silently switched the active scope to Base and closed the dialog, discarding the user's scope context. The creation and delete confirmation views contain no cmdk items or input, so they now render in a plain click-ui Dialog with Dialog.Content, dropping the VisuallyHidden title and description shims and letting buttons activate natively. The delete confirmation focuses the Delete button on open and keeps Escape and overlay dismissal closing the whole selector as before, while Cancel returns to the list view. The list view keeps cmdk for search and arrow navigation. The Create button and the per-scope delete buttons stop propagation of Enter and Space in their own keydown handlers, so native activation proceeds and the key never reaches the Command root. Enter in the search input is only forwarded to cmdk after the user has typed a query or navigated with the keyboard, which removes the silent switch to Base while preserving Enter-to-select on real list items. Adds jsdom regression tests covering all four Enter paths plus arrow navigation, using the real click-ui and cmdk packages with only the server layer mocked, and adds @testing-library/user-event as a dev dependency.
dustinhealy
changed the title
dustinhealy
marked this pull request as ready for review
August 11, 2026 23:01
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
Author
|
@codex review |
dustinhealy
marked this pull request as draft
August 14, 2026 15:03
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
cmdk moves its highlight on pointer movement over an item, not just on keyboard navigation, so hovering a scope and pressing Enter in the empty search was swallowed by the empty-input guard and never selected anything. Track pointer movement over items alongside keyboard navigation so any explicit highlight lets Enter through. The guard still blocks Enter when nothing was explicitly highlighted, preserving the original fix for the auto-highlighted first item closing the dialog on a stray Enter.
dustinhealy
marked this pull request as ready for review
August 14, 2026 15:43
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 284dada. Configure here.
Cancelling a delete, finishing a delete, or backing out of the create view remounts the cmdk list with the first item auto-highlighted while the hover flag from the previous view survived, so a stray Enter in the empty search selected Base. Every path back to the list now clears the flag through a shared returnToList handler.
Contributor
Author
|
@codex re-review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
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
1 participant
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
Four Enter-key bugs in the configuration scope selector, all one root cause and all pre-existing (not click-ui 0.9.1 regressions): the whole selector renders inside cmdk's
Command.Dialog, and cmdk's rootonKeyDowncallspreventDefault()unconditionally for Enter and re-dispatches to the highlighted[cmdk-item]. That cancels native button activation for every button inside the dialog.Symptoms fixed:
Fixes, two shapes for one cause: the creation view and the delete confirmation use no cmdk features, so they moved out of
Command.Dialoginto click-uiDialog/Dialog.Content(house pattern fromDeleteProfileValueModal; the confirmation autofocuses its Delete button). For buttons that remain inside the cmdk root (Create, per-scope delete), Enter/Space keydowns stop propagation so native activation proceeds while cmdk never sees the key. The search input additionally stops Enter only while empty and unnavigated, so Enter-to-select after filtering or arrow navigation is unchanged.Also: the icon-only back button in the creation view gained an aria-label (new
com_ui_backkey), and@testing-library/user-eventwas added as a devDependency for the new tests.Change Type
Testing
7 new Vitest cases (real click-ui and cmdk, server layer mocked); 6 fail against the unfixed component: Enter on Create opens the creation view without selecting/closing; Enter in the empty search does not select the Base scope; Enter on a role button creates the configuration; Enter on a delete button opens the confirmation without selecting; Enter on Delete deletes exactly once; Enter on Cancel returns without deleting; arrow navigation plus Enter still selects a scope.
Manually verified in the app: full keyboard-only pass through create, select, and delete flows.
Behavioral proof: 6 of the 7 new Enter-key cases fail against the unfixed component and pass with this change, driven by real keyboard events via user-event.
Test Configuration:
Checklist