Repository navigation
fix(palette): host the command palette on gpui-base Dialog - #569
Merged
Merged
Conversation
The palette rebuilt a modal by hand: a viewport overlay carrying its own focus, Escape handling and outside-press dismissal, with no focus trap. gpui_base::Dialog now owns the host, the focus trap, Escape and the backdrop press; the palette keeps its card, the list keys and the release guard, armed from the dialog's close request.
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.
Changed behaviour
CommandPaletterendered its own modal: a full-viewport overlay that tracked focus, matchedescape/up/downby key name, dismissed on an outside press through its own handler, and had no focus trap. It now renders agpui_base::Dialogwith the scrim as backdrop and the card as popup. The dialog owns the host, the focus trap, Escape (Cancel) and the backdrop press;request_closecloses the palette and arms theOutsideDismissalrelease guard the shell mounts, as #565 set up for sheets. The card keeps the palette's own focus handle and the list keys (up/down), so they work from the host focus and from the query input alike; Enter stays with the query input (on_okreturns false). The host is laid out throughDialog'sStyledthe way the overlay was (bottom sheet on compact, 96px below the top inset on desktop).Per Principle 7 (#564).
Tests
escape_from_the_query_and_a_backdrop_press_close_the_palette: Escape pressed while the query input is focused closes the palette (the base input bindsescapein its own context, so this checks it propagates through to the dialog), and a backdrop press closes it. The palette test harness now renders the palette while the window state says it is open.shell::tests::compact_palette_is_reachable_from_threads_and_thread(existing) covers the compact sheet geometry, backdrop press, Back handling and focus.empty_query_lists_commands_and_arrow_keys_keep_query_focus(existing) covers the list keys.Checks run
cargo fmt --all --check,cargo clippy --workspace --all-targets --locked -- -D warningscargo nextest run -p tcode-ui --locked— all passedNot verified visually (no Accessibility/Screen Recording permission for the computer-use tools on this machine). Geometry is asserted by the shell test for the compact sheet; the desktop placement is the same inset arithmetic as before.