From 0797f7b66e4187a599b88532545da46071f854f4 Mon Sep 17 00:00:00 2001 From: Ben Kunkle Date: Fri, 11 Jul 2025 13:07:04 -0500 Subject: [PATCH] keymap_ui: Show existing keystrokes as placeholders in edit modal (#34307) Closes #ISSUE Previously, the keystroke input would be empty, even when editing an existing binding. This meant you had to re-enter the bindings even if you just wanted to edit the context. Now, the existing keystrokes are rendered as a placeholder, are re-shown if newly entered keystrokes are cleared, and will be returned from the `KeystrokeInput::keystrokes()` method if no new keystrokes were entered. Additionally fixed a bug in `KeymapFile::update_keybinding` where semantically identical contexts would be treated as unequal due to formatting differences. Release Notes: - N/A *or* Added/Fixed/Improved ... --- crates/settings/src/keymap_file.rs | 7 +++- crates/settings_ui/src/keybindings.rs | 60 +++++++++++++++++++++------ 2 files changed, 54 insertions(+), 13 deletions(-) diff --git a/crates/settings/src/keymap_file.rs b/crates/settings/src/keymap_file.rs index 98dbe4d02a..102522f71d 100644 --- a/crates/settings/src/keymap_file.rs +++ b/crates/settings/src/keymap_file.rs @@ -783,8 +783,12 @@ impl KeymapFile { target: &KeybindUpdateTarget<'a>, target_action_value: &Value, ) -> Option<(usize, &'b str)> { + let target_context_parsed = + KeyBindingContextPredicate::parse(target.context.unwrap_or("")).ok(); for (index, section) in keymap.sections().enumerate() { - if section.context != target.context.unwrap_or("") { + let section_context_parsed = + KeyBindingContextPredicate::parse(§ion.context).ok(); + if section_context_parsed != target_context_parsed { continue; } if section.use_key_equivalents != target.use_key_equivalents { @@ -835,6 +839,7 @@ pub enum KeybindUpdateOperation<'a> { }, } +#[derive(Debug)] pub struct KeybindUpdateTarget<'a> { pub context: Option<&'a str>, pub keystrokes: &'a [Keystroke], diff --git a/crates/settings_ui/src/keybindings.rs b/crates/settings_ui/src/keybindings.rs index 993ed1bbbf..1718f3d283 100644 --- a/crates/settings_ui/src/keybindings.rs +++ b/crates/settings_ui/src/keybindings.rs @@ -283,7 +283,7 @@ impl KeymapEditor { let table_interaction_state = TableInteractionState::new(window, cx); let keystroke_editor = cx.new(|cx| { - let mut keystroke_editor = KeystrokeInput::new(window, cx); + let mut keystroke_editor = KeystrokeInput::new(None, window, cx); keystroke_editor.highlight_on_focus = false; keystroke_editor }); @@ -632,6 +632,11 @@ impl KeymapEditor { Box::new(EditBinding), ) .action("Create", Box::new(CreateBinding)) + .action_disabled_when( + selected_binding_is_unbound, + "Delete", + Box::new(DeleteBinding), + ) .action("Copy action", Box::new(CopyAction)) .action_disabled_when( selected_binding_has_no_context, @@ -1298,7 +1303,16 @@ impl KeybindingEditorModal { window: &mut Window, cx: &mut App, ) -> Self { - let keybind_editor = cx.new(|cx| KeystrokeInput::new(window, cx)); + let keybind_editor = cx.new(|cx| { + KeystrokeInput::new( + editing_keybind + .ui_key_binding + .as_ref() + .map(|keybinding| keybinding.keystrokes.clone()), + window, + cx, + ) + }); let context_editor = cx.new(|cx| { let mut editor = Editor::single_line(window, cx); @@ -1802,6 +1816,7 @@ async fn remove_keybinding( struct KeystrokeInput { keystrokes: Vec, + placeholder_keystrokes: Option>, highlight_on_focus: bool, focus_handle: FocusHandle, intercept_subscription: Option, @@ -1809,7 +1824,11 @@ struct KeystrokeInput { } impl KeystrokeInput { - fn new(window: &mut Window, cx: &mut Context) -> Self { + fn new( + placeholder_keystrokes: Option>, + window: &mut Window, + cx: &mut Context, + ) -> Self { let focus_handle = cx.focus_handle(); let _focus_subscriptions = [ cx.on_focus_in(&focus_handle, window, Self::on_focus_in), @@ -1817,6 +1836,7 @@ impl KeystrokeInput { ]; Self { keystrokes: Vec::new(), + placeholder_keystrokes, highlight_on_focus: true, focus_handle, intercept_subscription: None, @@ -1904,6 +1924,11 @@ impl KeystrokeInput { } fn keystrokes(&self) -> &[Keystroke] { + if let Some(placeholders) = self.placeholder_keystrokes.as_ref() + && self.keystrokes.is_empty() + { + return placeholders; + } if self .keystrokes .last() @@ -1913,6 +1938,25 @@ impl KeystrokeInput { } return &self.keystrokes; } + + fn render_keystrokes(&self) -> impl Iterator { + let (keystrokes, color) = if let Some(placeholders) = self.placeholder_keystrokes.as_ref() + && self.keystrokes.is_empty() + { + (placeholders, Color::Placeholder) + } else { + (&self.keystrokes, Color::Default) + }; + keystrokes.iter().map(move |keystroke| { + h_flex().children(ui::render_keystroke( + keystroke, + Some(color), + Some(rems(0.875).into()), + ui::PlatformStyle::platform(), + false, + )) + }) + } } impl EventEmitter<()> for KeystrokeInput {} @@ -1958,15 +2002,7 @@ impl Render for KeystrokeInput { .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, - )) - })), + .children(self.render_keystrokes()), ) .child( h_flex()