From 77c4530e128caded51ef66e3931cee11435dc5c4 Mon Sep 17 00:00:00 2001 From: Danilo Leal <67129314+danilo-leal@users.noreply.github.com> Date: Wed, 2 Jul 2025 19:48:49 -0300 Subject: [PATCH] Add refinements to the keymap UI (#33816) This includes mostly polishing up the keystroke editing modal, and some other bits like making the keystroke rendering function more composable. Release Notes: - Added refinements to the keymap UI design. --------- Co-authored-by: Ben Kunkle Co-authored-by: Ben Kunkle --- crates/settings_ui/src/keybindings.rs | 215 ++++++++++-------- crates/ui/src/components/keybinding.rs | 124 ++++++---- crates/ui/src/components/keybinding_hint.rs | 2 +- .../ui/src/components/stories/keybinding.rs | 55 +++-- 4 files changed, 233 insertions(+), 163 deletions(-) diff --git a/crates/settings_ui/src/keybindings.rs b/crates/settings_ui/src/keybindings.rs index 480614bcab..e725d2907f 100644 --- a/crates/settings_ui/src/keybindings.rs +++ b/crates/settings_ui/src/keybindings.rs @@ -8,8 +8,8 @@ use fs::Fs; use fuzzy::{StringMatch, StringMatchCandidate}; use gpui::{ AppContext as _, AsyncApp, Context, DismissEvent, Entity, EventEmitter, FocusHandle, Focusable, - FontWeight, Global, KeyContext, Keystroke, ModifiersChangedEvent, ScrollStrategy, StyledText, - Subscription, WeakEntity, actions, div, transparent_black, + Global, KeyContext, Keystroke, ModifiersChangedEvent, ScrollStrategy, StyledText, Subscription, + WeakEntity, actions, div, transparent_black, }; use language::{Language, LanguageConfig}; use settings::KeybindSource; @@ -18,7 +18,7 @@ use util::ResultExt; use ui::{ ActiveTheme as _, App, BorrowAppContext, ContextMenu, ParentElement as _, Render, SharedString, - Styled as _, Window, prelude::*, right_click_menu, + Styled as _, Tooltip, Window, prelude::*, right_click_menu, }; use workspace::{Item, ModalView, SerializableItem, Workspace, register_serializable_item}; @@ -145,7 +145,7 @@ impl KeymapEditor { let filter_editor = cx.new(|cx| { let mut editor = Editor::single_line(window, cx); - editor.set_placeholder_text("Filter action names...", cx); + editor.set_placeholder_text("Filter action names…", cx); editor }); @@ -248,7 +248,7 @@ impl KeymapEditor { let keystroke_text = ui::text_for_keystrokes(key_binding.keystrokes(), cx); let ui_key_binding = Some( - ui::KeyBinding::new(key_binding.clone(), cx) + ui::KeyBinding::new_from_gpui(key_binding.clone(), cx) .vim_mode(source == Some(settings::KeybindSource::Vim)), ); @@ -571,7 +571,7 @@ impl Render for KeymapEditor { let row_count = self.matches.len(); let theme = cx.theme(); - div() + v_flex() .key_context(self.dispatch_context(window, cx)) .on_action(cx.listener(Self::select_next)) .on_action(cx.listener(Self::select_previous)) @@ -586,9 +586,9 @@ impl Render for KeymapEditor { .bg(theme.colors().editor_background) .id("keymap-editor") .track_focus(&self.focus_handle) + .pt_4() .px_4() - .v_flex() - .pb_4() + .gap_4() .child( h_flex() .key_context({ @@ -596,12 +596,13 @@ impl Render for KeymapEditor { context.add("BufferSearchBar"); context }) - .w_full() - .h_12() - .px_4() - .my_4() - .border_2() + .h_8() + .pl_2() + .pr_1() + .py_1() + .border_1() .border_color(theme.colors().border) + .rounded_lg() .child(self.filter_editor.clone()), ) .child( @@ -742,7 +743,7 @@ impl RenderOnce for SyntaxHighlightedText { struct KeybindingEditorModal { editing_keybind: ProcessedKeybinding, - keybind_editor: Entity, + keybind_editor: Entity, fs: Arc, error: Option, } @@ -764,7 +765,7 @@ impl KeybindingEditorModal { _window: &mut Window, cx: &mut App, ) -> Self { - let keybind_editor = cx.new(KeybindInput::new); + let keybind_editor = cx.new(KeystrokeInput::new); Self { editing_keybind, fs, @@ -777,84 +778,65 @@ impl KeybindingEditorModal { impl Render for KeybindingEditorModal { fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { let theme = cx.theme().colors(); + return v_flex() - .gap_4() .w(rems(36.)) + .elevation_3(cx) .child( v_flex() - .items_center() - .text_center() - .bg(theme.background) - .border_color(theme.border) - .border_2() + .pt_2() .px_4() - .py_2() + .pb_4() + .gap_2() + .child(Label::new("Input desired keystroke, then hit save")) + .child(self.keybind_editor.clone()), + ) + .child( + h_flex() + .p_2() .w_full() + .gap_1() + .justify_end() + .border_t_1() + .border_color(cx.theme().colors().border_variant) .child( - div() - .text_lg() - .font_weight(FontWeight::BOLD) - .child("Input desired keybinding, then hit save"), + Button::new("cancel", "Cancel") + .on_click(cx.listener(|_, _, _, cx| cx.emit(DismissEvent))), ) .child( - h_flex() - .w_full() - .child(self.keybind_editor.clone()) - .child( - IconButton::new("backspace-btn", ui::IconName::Backspace).on_click( - cx.listener(|this, _event, _window, cx| { - this.keybind_editor.update(cx, |editor, cx| { - editor.keystrokes.pop(); + Button::new("save-btn", "Save Keybinding").on_click(cx.listener( + |this, _event, _window, cx| { + let existing_keybind = this.editing_keybind.clone(); + let fs = this.fs.clone(); + let new_keystrokes = this + .keybind_editor + .read_with(cx, |editor, _| editor.keystrokes.clone()); + if new_keystrokes.is_empty() { + this.error = Some("Keystrokes cannot be empty".to_string()); + cx.notify(); + return; + } + let tab_size = + cx.global::().json_tab_size(); + cx.spawn(async move |this, cx| { + if let Err(err) = save_keybinding_update( + existing_keybind, + &new_keystrokes, + &fs, + tab_size, + ) + .await + { + this.update(cx, |this, cx| { + this.error = Some(err.to_string()); cx.notify(); }) - }), - ), - ) - .child(IconButton::new("clear-btn", ui::IconName::Eraser).on_click( - cx.listener(|this, _event, _window, cx| { - this.keybind_editor.update(cx, |editor, cx| { - editor.keystrokes.clear(); - cx.notify(); - }) - }), - )), - ) - .child( - h_flex().w_full().items_center().justify_center().child( - Button::new("save-btn", "Save") - .label_size(LabelSize::Large) - .on_click(cx.listener(|this, _event, _window, cx| { - let existing_keybind = this.editing_keybind.clone(); - let fs = this.fs.clone(); - let new_keystrokes = this - .keybind_editor - .read_with(cx, |editor, _| editor.keystrokes.clone()); - if new_keystrokes.is_empty() { - this.error = Some("Keystrokes cannot be empty".to_string()); - cx.notify(); - return; + .log_err(); } - let tab_size = - cx.global::().json_tab_size(); - cx.spawn(async move |this, cx| { - if let Err(err) = save_keybinding_update( - existing_keybind, - &new_keystrokes, - &fs, - tab_size, - ) - .await - { - this.update(cx, |this, cx| { - this.error = Some(err.to_string()); - cx.notify(); - }) - .log_err(); - } - }) - .detach(); - })), - ), + }) + .detach(); + }, + )), ), ) .when_some(self.error.clone(), |this, error| { @@ -879,11 +861,13 @@ async fn save_keybinding_update( let keymap_contents = settings::KeymapFile::load_keymap_file(fs) .await .context("Failed to load keymap file")?; + let existing_keystrokes = existing .ui_key_binding .as_ref() - .map(|keybinding| keybinding.key_binding.keystrokes()) + .map(|keybinding| keybinding.keystrokes.as_slice()) .unwrap_or_default(); + let context = existing .context .as_ref() @@ -927,12 +911,12 @@ async fn save_keybinding_update( Ok(()) } -struct KeybindInput { +struct KeystrokeInput { keystrokes: Vec, focus_handle: FocusHandle, } -impl KeybindInput { +impl KeystrokeInput { fn new(cx: &mut Context) -> Self { let focus_handle = cx.focus_handle(); Self { @@ -1007,16 +991,18 @@ impl KeybindInput { } } -impl Focusable for KeybindInput { +impl Focusable for KeystrokeInput { fn focus_handle(&self, _cx: &App) -> FocusHandle { self.focus_handle.clone() } } -impl Render for KeybindInput { +impl Render for KeystrokeInput { fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { let colors = cx.theme().colors(); - return div() + + return h_flex() + .id("keybinding_input") .track_focus(&self.focus_handle) .on_modifiers_changed(cx.listener(Self::on_modifiers_changed)) .on_key_down(cx.listener(Self::on_key_down)) @@ -1025,16 +1011,55 @@ impl Render for KeybindInput { style.border_color = Some(colors.border_focused); style }) - .h_12() + .py_2() + .px_3() + .gap_2() + .min_h_8() .w_full() + .justify_between() .bg(colors.editor_background) - .border_2() - .border_color(colors.border) - .p_4() - .flex_row() - .text_center() - .justify_center() - .child(ui::text_for_keystrokes(&self.keystrokes, cx)); + .border_1() + .rounded_md() + .flex_1() + .overflow_hidden() + .child( + h_flex() + .w_full() + .min_w_0() + .justify_center() + .flex_wrap() + .gap(ui::DynamicSpacing::Base04.rems(cx)) + .children(self.keystrokes.iter().map(|keystroke| { + h_flex().children(ui::render_keystroke( + keystroke, + None, + Some(rems(0.875).into()), + ui::PlatformStyle::platform(), + false, + )) + })), + ) + .child( + h_flex() + .gap_0p5() + .flex_none() + .child( + IconButton::new("backspace-btn", IconName::Delete) + .tooltip(Tooltip::text("Delete Keystroke")) + .on_click(cx.listener(|this, _event, _window, cx| { + this.keystrokes.pop(); + cx.notify(); + })), + ) + .child( + IconButton::new("clear-btn", IconName::Eraser) + .tooltip(Tooltip::text("Clear Keystrokes")) + .on_click(cx.listener(|this, _event, _window, cx| { + this.keystrokes.clear(); + cx.notify(); + })), + ), + ); } } diff --git a/crates/ui/src/components/keybinding.rs b/crates/ui/src/components/keybinding.rs index 6da3d03ea1..1d91492f26 100644 --- a/crates/ui/src/components/keybinding.rs +++ b/crates/ui/src/components/keybinding.rs @@ -13,7 +13,7 @@ pub struct KeyBinding { /// More than one keystroke produces a chord. /// /// This should always contain at least one keystroke. - pub key_binding: gpui::KeyBinding, + pub keystrokes: Vec, /// The [`PlatformStyle`] to use when displaying this keybinding. platform_style: PlatformStyle, @@ -37,7 +37,7 @@ impl KeyBinding { return Self::for_action_in(action, &focused, window, cx); } let key_binding = window.highest_precedence_binding_for_action(action)?; - Some(Self::new(key_binding, cx)) + Some(Self::new_from_gpui(key_binding, cx)) } /// Like `for_action`, but lets you specify the context from which keybindings are matched. @@ -48,7 +48,7 @@ impl KeyBinding { cx: &App, ) -> Option { let key_binding = window.highest_precedence_binding_for_action_in(action, focus)?; - Some(Self::new(key_binding, cx)) + Some(Self::new_from_gpui(key_binding, cx)) } pub fn set_vim_mode(cx: &mut App, enabled: bool) { @@ -59,9 +59,9 @@ impl KeyBinding { cx.try_global::().is_some_and(|g| g.0) } - pub fn new(key_binding: gpui::KeyBinding, cx: &App) -> Self { + pub fn new(keystrokes: Vec, cx: &App) -> Self { Self { - key_binding, + keystrokes, platform_style: PlatformStyle::platform(), size: None, vim_mode: KeyBinding::is_vim_mode(cx), @@ -69,6 +69,10 @@ impl KeyBinding { } } + pub fn new_from_gpui(key_binding: gpui::KeyBinding, cx: &App) -> Self { + Self::new(key_binding.keystrokes().to_vec(), cx) + } + /// Sets the [`PlatformStyle`] for this [`KeyBinding`]. pub fn platform_style(mut self, platform_style: PlatformStyle) -> Self { self.platform_style = platform_style; @@ -92,15 +96,20 @@ impl KeyBinding { self.vim_mode = enabled; self } +} - fn render_key(&self, keystroke: &Keystroke, color: Option) -> AnyElement { - let key_icon = icon_for_key(keystroke, self.platform_style); - match key_icon { - Some(icon) => KeyIcon::new(icon, color).size(self.size).into_any_element(), - None => { - let key = util::capitalize(&keystroke.key); - Key::new(&key, color).size(self.size).into_any_element() - } +fn render_key( + keystroke: &Keystroke, + color: Option, + platform_style: PlatformStyle, + size: impl Into>, +) -> AnyElement { + let key_icon = icon_for_key(keystroke, platform_style); + match key_icon { + Some(icon) => KeyIcon::new(icon, color).size(size).into_any_element(), + None => { + let key = util::capitalize(&keystroke.key); + Key::new(&key, color).size(size).into_any_element() } } } @@ -108,17 +117,12 @@ impl KeyBinding { impl RenderOnce for KeyBinding { fn render(self, _window: &mut Window, cx: &mut App) -> impl IntoElement { let color = self.disabled.then_some(Color::Disabled); - let use_text = self.vim_mode - || matches!( - self.platform_style, - PlatformStyle::Linux | PlatformStyle::Windows - ); + h_flex() .debug_selector(|| { format!( "KEY_BINDING-{}", - self.key_binding - .keystrokes() + self.keystrokes .iter() .map(|k| k.key.to_string()) .collect::>() @@ -127,35 +131,56 @@ impl RenderOnce for KeyBinding { }) .gap(DynamicSpacing::Base04.rems(cx)) .flex_none() - .children(self.key_binding.keystrokes().iter().map(|keystroke| { + .children(self.keystrokes.iter().map(|keystroke| { h_flex() .flex_none() .py_0p5() .rounded_xs() .text_color(cx.theme().colors().text_muted) - .when(use_text, |el| { - el.child( - Key::new( - keystroke_text(&keystroke, self.platform_style, self.vim_mode), - color, - ) - .size(self.size), - ) - }) - .when(!use_text, |el| { - el.children(render_modifiers( - &keystroke.modifiers, - self.platform_style, - color, - self.size, - true, - )) - .map(|el| el.child(self.render_key(&keystroke, color))) - }) + .children(render_keystroke( + keystroke, + color, + self.size, + self.platform_style, + self.vim_mode, + )) })) } } +pub fn render_keystroke( + keystroke: &Keystroke, + color: Option, + size: impl Into>, + platform_style: PlatformStyle, + vim_mode: bool, +) -> Vec { + let use_text = vim_mode + || matches!( + platform_style, + PlatformStyle::Linux | PlatformStyle::Windows + ); + let size = size.into(); + + if use_text { + let element = Key::new(keystroke_text(&keystroke, platform_style, vim_mode), color) + .size(size) + .into_any_element(); + vec![element] + } else { + let mut elements = Vec::new(); + elements.extend(render_modifiers( + &keystroke.modifiers, + platform_style, + color, + size, + true, + )); + elements.push(render_key(&keystroke, color, platform_style, size)); + elements + } +} + fn icon_for_key(keystroke: &Keystroke, platform_style: PlatformStyle) -> Option { match keystroke.key.as_str() { "left" => Some(IconName::ArrowLeft), @@ -466,7 +491,7 @@ impl Component for KeyBinding { vec![ single_example( "Default", - KeyBinding::new( + KeyBinding::new_from_gpui( gpui::KeyBinding::new("ctrl-s", gpui::NoAction, None), cx, ) @@ -474,7 +499,7 @@ impl Component for KeyBinding { ), single_example( "Mac Style", - KeyBinding::new( + KeyBinding::new_from_gpui( gpui::KeyBinding::new("cmd-s", gpui::NoAction, None), cx, ) @@ -483,7 +508,7 @@ impl Component for KeyBinding { ), single_example( "Windows Style", - KeyBinding::new( + KeyBinding::new_from_gpui( gpui::KeyBinding::new("ctrl-s", gpui::NoAction, None), cx, ) @@ -496,9 +521,12 @@ impl Component for KeyBinding { "Vim Mode", vec![single_example( "Vim Mode Enabled", - KeyBinding::new(gpui::KeyBinding::new("dd", gpui::NoAction, None), cx) - .vim_mode(true) - .into_any_element(), + KeyBinding::new_from_gpui( + gpui::KeyBinding::new("dd", gpui::NoAction, None), + cx, + ) + .vim_mode(true) + .into_any_element(), )], ), example_group_with_title( @@ -506,7 +534,7 @@ impl Component for KeyBinding { vec![ single_example( "Multiple Keys", - KeyBinding::new( + KeyBinding::new_from_gpui( gpui::KeyBinding::new("ctrl-k ctrl-b", gpui::NoAction, None), cx, ) @@ -514,7 +542,7 @@ impl Component for KeyBinding { ), single_example( "With Shift", - KeyBinding::new( + KeyBinding::new_from_gpui( gpui::KeyBinding::new("shift-cmd-p", gpui::NoAction, None), cx, ) diff --git a/crates/ui/src/components/keybinding_hint.rs b/crates/ui/src/components/keybinding_hint.rs index 4c8c893636..d6dc094d41 100644 --- a/crates/ui/src/components/keybinding_hint.rs +++ b/crates/ui/src/components/keybinding_hint.rs @@ -216,7 +216,7 @@ impl Component for KeybindingHint { fn preview(window: &mut Window, cx: &mut App) -> Option { let enter_fallback = gpui::KeyBinding::new("enter", menu::Confirm, None); let enter = KeyBinding::for_action(&menu::Confirm, window, cx) - .unwrap_or(KeyBinding::new(enter_fallback, cx)); + .unwrap_or(KeyBinding::new_from_gpui(enter_fallback, cx)); let bg_color = cx.theme().colors().surface_background; diff --git a/crates/ui/src/components/stories/keybinding.rs b/crates/ui/src/components/stories/keybinding.rs index 1b47870468..594f70b6ab 100644 --- a/crates/ui/src/components/stories/keybinding.rs +++ b/crates/ui/src/components/stories/keybinding.rs @@ -18,16 +18,16 @@ impl Render for KeybindingStory { Story::container(cx) .child(Story::title_for::(cx)) .child(Story::label("Single Key", cx)) - .child(KeyBinding::new(binding("Z"), cx)) + .child(KeyBinding::new_from_gpui(binding("Z"), cx)) .child(Story::label("Single Key with Modifier", cx)) .child( div() .flex() .gap_3() - .child(KeyBinding::new(binding("ctrl-c"), cx)) - .child(KeyBinding::new(binding("alt-c"), cx)) - .child(KeyBinding::new(binding("cmd-c"), cx)) - .child(KeyBinding::new(binding("shift-c"), cx)), + .child(KeyBinding::new_from_gpui(binding("ctrl-c"), cx)) + .child(KeyBinding::new_from_gpui(binding("alt-c"), cx)) + .child(KeyBinding::new_from_gpui(binding("cmd-c"), cx)) + .child(KeyBinding::new_from_gpui(binding("shift-c"), cx)), ) .child(Story::label("Single Key with Modifier (Permuted)", cx)) .child( @@ -41,42 +41,59 @@ impl Render for KeybindingStory { .gap_4() .py_3() .children(chunk.map(|permutation| { - KeyBinding::new(binding(&(permutation.join("-") + "-x")), cx) + KeyBinding::new_from_gpui( + binding(&(permutation.join("-") + "-x")), + cx, + ) })) }), ), ) .child(Story::label("Single Key with All Modifiers", cx)) - .child(KeyBinding::new(binding("ctrl-alt-cmd-shift-z"), cx)) + .child(KeyBinding::new_from_gpui( + binding("ctrl-alt-cmd-shift-z"), + cx, + )) .child(Story::label("Chord", cx)) - .child(KeyBinding::new(binding("a z"), cx)) + .child(KeyBinding::new_from_gpui(binding("a z"), cx)) .child(Story::label("Chord with Modifier", cx)) - .child(KeyBinding::new(binding("ctrl-a shift-z"), cx)) - .child(KeyBinding::new(binding("fn-s"), cx)) + .child(KeyBinding::new_from_gpui(binding("ctrl-a shift-z"), cx)) + .child(KeyBinding::new_from_gpui(binding("fn-s"), cx)) .child(Story::label("Single Key with All Modifiers (Linux)", cx)) .child( - KeyBinding::new(binding("ctrl-alt-cmd-shift-z"), cx) + KeyBinding::new_from_gpui(binding("ctrl-alt-cmd-shift-z"), cx) .platform_style(PlatformStyle::Linux), ) .child(Story::label("Chord (Linux)", cx)) - .child(KeyBinding::new(binding("a z"), cx).platform_style(PlatformStyle::Linux)) + .child( + KeyBinding::new_from_gpui(binding("a z"), cx).platform_style(PlatformStyle::Linux), + ) .child(Story::label("Chord with Modifier (Linux)", cx)) .child( - KeyBinding::new(binding("ctrl-a shift-z"), cx).platform_style(PlatformStyle::Linux), + KeyBinding::new_from_gpui(binding("ctrl-a shift-z"), cx) + .platform_style(PlatformStyle::Linux), + ) + .child( + KeyBinding::new_from_gpui(binding("fn-s"), cx).platform_style(PlatformStyle::Linux), ) - .child(KeyBinding::new(binding("fn-s"), cx).platform_style(PlatformStyle::Linux)) .child(Story::label("Single Key with All Modifiers (Windows)", cx)) .child( - KeyBinding::new(binding("ctrl-alt-cmd-shift-z"), cx) + KeyBinding::new_from_gpui(binding("ctrl-alt-cmd-shift-z"), cx) .platform_style(PlatformStyle::Windows), ) .child(Story::label("Chord (Windows)", cx)) - .child(KeyBinding::new(binding("a z"), cx).platform_style(PlatformStyle::Windows)) - .child(Story::label("Chord with Modifier (Windows)", cx)) .child( - KeyBinding::new(binding("ctrl-a shift-z"), cx) + KeyBinding::new_from_gpui(binding("a z"), cx) + .platform_style(PlatformStyle::Windows), + ) + .child(Story::label("Chord with Modifier (Windows)", cx)) + .child( + KeyBinding::new_from_gpui(binding("ctrl-a shift-z"), cx) + .platform_style(PlatformStyle::Windows), + ) + .child( + KeyBinding::new_from_gpui(binding("fn-s"), cx) .platform_style(PlatformStyle::Windows), ) - .child(KeyBinding::new(binding("fn-s"), cx).platform_style(PlatformStyle::Windows)) } }