From 27691613c184fbef9d00ea144ab0da4980a06e20 Mon Sep 17 00:00:00 2001 From: "gcp-cherry-pick-bot[bot]" <98988430+gcp-cherry-pick-bot[bot]@users.noreply.github.com> Date: Thu, 17 Jul 2025 11:34:44 -0400 Subject: [PATCH] keymap_ui: Improve keybind display in menus (cherry-pick #34587) (#34632) Cherry-picked keymap_ui: Improve keybind display in menus (#34587) Closes #ISSUE Defines keybindings for `keymap_editor::EditBinding` and `keymap_editor::CreateBinding`, making sure those actions are used in tooltips. Release Notes: - N/A *or* Added/Fixed/Improved ... --------- Co-authored-by: Finn Co-authored-by: Ben Kunkle Co-authored-by: Finn --- assets/keymaps/default-linux.json | 4 +- assets/keymaps/default-macos.json | 4 +- crates/settings_ui/src/keybindings.rs | 118 +++++++++++++---------- crates/ui/src/components/context_menu.rs | 6 +- 4 files changed, 75 insertions(+), 57 deletions(-) diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json index da4d79eca1..29139aae1c 100644 --- a/assets/keymaps/default-linux.json +++ b/assets/keymaps/default-linux.json @@ -1118,7 +1118,9 @@ "ctrl-f": "search::FocusSearch", "alt-find": "keymap_editor::ToggleKeystrokeSearch", "alt-ctrl-f": "keymap_editor::ToggleKeystrokeSearch", - "alt-c": "keymap_editor::ToggleConflictFilter" + "alt-c": "keymap_editor::ToggleConflictFilter", + "enter": "keymap_editor::EditBinding", + "alt-enter": "keymap_editor::CreateBinding" } }, { diff --git a/assets/keymaps/default-macos.json b/assets/keymaps/default-macos.json index 962760098b..076f9da615 100644 --- a/assets/keymaps/default-macos.json +++ b/assets/keymaps/default-macos.json @@ -1217,7 +1217,9 @@ "use_key_equivalents": true, "bindings": { "cmd-alt-f": "keymap_editor::ToggleKeystrokeSearch", - "cmd-alt-c": "keymap_editor::ToggleConflictFilter" + "cmd-alt-c": "keymap_editor::ToggleConflictFilter", + "enter": "keymap_editor::EditBinding", + "alt-enter": "keymap_editor::CreateBinding" } }, { diff --git a/crates/settings_ui/src/keybindings.rs b/crates/settings_ui/src/keybindings.rs index 78636f7023..81960f0376 100644 --- a/crates/settings_ui/src/keybindings.rs +++ b/crates/settings_ui/src/keybindings.rs @@ -650,7 +650,7 @@ impl KeymapEditor { .detach_and_log_err(cx); } - fn dispatch_context(&self, _window: &Window, _cx: &Context) -> KeyContext { + fn key_context(&self) -> KeyContext { let mut dispatch_context = KeyContext::new_with_defaults(); dispatch_context.add("KeymapEditor"); dispatch_context.add("menu"); @@ -685,14 +685,19 @@ impl KeymapEditor { self.selected_index.take(); } - fn selected_keybind_idx(&self) -> Option { + fn selected_keybind_index(&self) -> Option { self.selected_index .and_then(|match_index| self.matches.get(match_index)) .map(|r#match| r#match.candidate_id) } + fn selected_keybind_and_index(&self) -> Option<(&ProcessedKeybinding, usize)> { + self.selected_keybind_index() + .map(|keybind_index| (&self.keybindings[keybind_index], keybind_index)) + } + fn selected_binding(&self) -> Option<&ProcessedKeybinding> { - self.selected_keybind_idx() + self.selected_keybind_index() .and_then(|keybind_index| self.keybindings.get(keybind_index)) } @@ -724,40 +729,41 @@ impl KeymapEditor { let selected_binding_is_unbound = selected_binding.keystrokes().is_none(); let context_menu = ContextMenu::build(window, cx, |menu, _window, _cx| { - menu.action_disabled_when( - selected_binding_is_unbound, - "Edit", - Box::new(EditBinding), - ) - .action("Create", Box::new(CreateBinding)) - .action_disabled_when( - selected_binding_is_unbound, - "Delete", - Box::new(DeleteBinding), - ) - .separator() - .action("Copy Action", Box::new(CopyAction)) - .action_disabled_when( - selected_binding_has_no_context, - "Copy Context", - Box::new(CopyContext), - ) - .entry("Show matching keybindings", None, { - let weak = weak.clone(); - let key_strokes = key_strokes.clone(); + menu.context(self.focus_handle.clone()) + .action_disabled_when( + selected_binding_is_unbound, + "Edit", + Box::new(EditBinding), + ) + .action("Create", Box::new(CreateBinding)) + .action_disabled_when( + selected_binding_is_unbound, + "Delete", + Box::new(DeleteBinding), + ) + .separator() + .action("Copy Action", Box::new(CopyAction)) + .action_disabled_when( + selected_binding_has_no_context, + "Copy Context", + Box::new(CopyContext), + ) + .entry("Show matching keybindings", None, { + let weak = weak.clone(); + let key_strokes = key_strokes.clone(); - move |_, cx| { - weak.update(cx, |this, cx| { - this.filter_state = FilterState::All; - this.search_mode = SearchMode::KeyStroke { exact_match: true }; + move |_, cx| { + weak.update(cx, |this, cx| { + this.filter_state = FilterState::All; + this.search_mode = SearchMode::KeyStroke { exact_match: true }; - this.keystroke_editor.update(cx, |editor, cx| { - editor.set_keystrokes(key_strokes.clone(), cx); - }); - }) - .ok(); - } - }) + this.keystroke_editor.update(cx, |editor, cx| { + editor.set_keystrokes(key_strokes.clone(), cx); + }); + }) + .ok(); + } + }) }); let context_menu_handle = context_menu.focus_handle(cx); @@ -847,22 +853,16 @@ impl KeymapEditor { } } - fn confirm(&mut self, _: &menu::Confirm, window: &mut Window, cx: &mut Context) { - self.open_edit_keybinding_modal(false, window, cx); - } - fn open_edit_keybinding_modal( &mut self, create: bool, window: &mut Window, cx: &mut Context, ) { - let Some((keybind_idx, keybind)) = self - .selected_keybind_idx() - .zip(self.selected_binding().cloned()) - else { + let Some((keybind, keybind_index)) = self.selected_keybind_and_index() else { return; }; + let keybind = keybind.clone(); let keymap_editor = cx.entity(); self.workspace .update(cx, |workspace, cx| { @@ -872,7 +872,7 @@ impl KeymapEditor { let modal = KeybindingEditorModal::new( create, keybind, - keybind_idx, + keybind_index, keymap_editor, workspace_weak, fs, @@ -1087,20 +1087,19 @@ impl Item for KeymapEditor { } impl Render for KeymapEditor { - fn render(&mut self, window: &mut Window, cx: &mut ui::Context) -> impl ui::IntoElement { + fn render(&mut self, _window: &mut Window, cx: &mut ui::Context) -> impl ui::IntoElement { let row_count = self.matches.len(); let theme = cx.theme(); v_flex() .id("keymap-editor") .track_focus(&self.focus_handle) - .key_context(self.dispatch_context(window, cx)) + .key_context(self.key_context()) .on_action(cx.listener(Self::select_next)) .on_action(cx.listener(Self::select_previous)) .on_action(cx.listener(Self::select_first)) .on_action(cx.listener(Self::select_last)) .on_action(cx.listener(Self::focus_search)) - .on_action(cx.listener(Self::confirm)) .on_action(cx.listener(Self::edit_binding)) .on_action(cx.listener(Self::create_binding)) .on_action(cx.listener(Self::delete_binding)) @@ -1214,6 +1213,18 @@ impl Render for KeymapEditor { "keystrokes-exact-match", IconName::Equal, ) + .tooltip(move |window, cx| { + Tooltip::for_action( + if exact_match { + "Partial match mode" + } else { + "Exact match mode" + }, + &ToggleExactKeystrokeMatching, + window, + cx, + ) + }) .shape(IconButtonShape::Square) .toggle_state(exact_match) .on_click( @@ -1261,9 +1272,9 @@ impl Render for KeymapEditor { .icon_color(Color::Warning) .tooltip(|window, cx| { Tooltip::with_meta( - "Edit Keybinding", - None, - "Use alt+click to show conflicts", + "View conflicts", + Some(&ToggleConflictFilter), + "Use alt+click to show all conflicts", window, cx, ) @@ -1288,7 +1299,10 @@ impl Render for KeymapEditor { .unwrap_or_else(|| { base_button_style(index, IconName::Pencil) .visible_on_hover(row_group_id(index)) - .tooltip(Tooltip::text("Edit Keybinding")) + .tooltip(Tooltip::for_action_title( + "Edit Keybinding", + &EditBinding, + )) .on_click(cx.listener(move |this, _, window, cx| { this.select_index(index, cx); this.open_edit_keybinding_modal(false, window, cx); @@ -2474,6 +2488,8 @@ impl KeystrokeInput { if self.keystrokes.len() < Self::KEYSTROKE_COUNT_MAX { self.keystrokes.push(Self::dummy(keystroke.modifiers)); } + } else if close_keystroke_result != CloseKeystrokeResult::Partial { + self.clear_keystrokes(&ClearKeystrokes, window, cx); } } self.keystrokes_changed(cx); diff --git a/crates/ui/src/components/context_menu.rs b/crates/ui/src/components/context_menu.rs index 3ba73f6dff..467dd226fb 100644 --- a/crates/ui/src/components/context_menu.rs +++ b/crates/ui/src/components/context_menu.rs @@ -972,12 +972,10 @@ impl ContextMenu { .children(action.as_ref().and_then(|action| { self.action_context .as_ref() - .map(|focus| { + .and_then(|focus| { KeyBinding::for_action_in(&**action, focus, window, cx) }) - .unwrap_or_else(|| { - KeyBinding::for_action(&**action, window, cx) - }) + .or_else(|| KeyBinding::for_action(&**action, window, cx)) .map(|binding| { div().ml_4().child(binding.disabled(*disabled)).when( *disabled && documentation_aside.is_some(),