From 5e600478453eb2703ae13e63f75a8bdf08019a12 Mon Sep 17 00:00:00 2001 From: Anthony Eid <56899983+Anthony-Eid@users.noreply.github.com> Date: Sat, 2 May 2026 22:50:51 +0200 Subject: [PATCH] git_graph: Fix search bar Vim key handling (#55510) #53609 introduced a regression where Git Graph keybindings could take precedence over the search bar. As a result, typing characters like `j` or `k` in the search field could move the table selection instead of updating the search query. This PR fixes that regression by scoping Vim table navigation bindings away from the search bar. It also adds dedicated `tab` and `shift-tab` handling for Git Graph focus traversal, with the search bar and graph table participating as separate tab groups. Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [ ] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - N/A --- assets/keymaps/default-linux.json | 14 ++++++ assets/keymaps/default-macos.json | 14 ++++++ assets/keymaps/default-windows.json | 14 ++++++ assets/keymaps/vim.json | 14 ++++++ crates/editor/src/editor.rs | 4 ++ crates/git_graph/src/git_graph.rs | 68 +++++++++++++++++++++++++++-- 6 files changed, 125 insertions(+), 3 deletions(-) diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json index ba7f514766..66f527ef02 100644 --- a/assets/keymaps/default-linux.json +++ b/assets/keymaps/default-linux.json @@ -1536,4 +1536,18 @@ "ctrl-shift-backspace": "worktree_picker::DeleteWorktree", }, }, + { + "context": "GitGraph", + "bindings": { + "tab": "git_graph::FocusNextTabStop", + "shift-tab": "git_graph::FocusPreviousTabStop", + }, + }, + { + "context": "GitGraphSearchBar > Editor", + "bindings": { + "tab": "git_graph::FocusNextTabStop", + "shift-tab": "git_graph::FocusPreviousTabStop", + }, + }, ] diff --git a/assets/keymaps/default-macos.json b/assets/keymaps/default-macos.json index 11750aa741..8e115985db 100644 --- a/assets/keymaps/default-macos.json +++ b/assets/keymaps/default-macos.json @@ -1627,4 +1627,18 @@ "escape": "notebook::EnterCommandMode", }, }, + { + "context": "GitGraph", + "bindings": { + "tab": "git_graph::FocusNextTabStop", + "shift-tab": "git_graph::FocusPreviousTabStop", + }, + }, + { + "context": "GitGraphSearchBar > Editor", + "bindings": { + "tab": "git_graph::FocusNextTabStop", + "shift-tab": "git_graph::FocusPreviousTabStop", + }, + }, ] diff --git a/assets/keymaps/default-windows.json b/assets/keymaps/default-windows.json index 2526a9412c..9ac7ed46cf 100644 --- a/assets/keymaps/default-windows.json +++ b/assets/keymaps/default-windows.json @@ -1553,4 +1553,18 @@ "escape": "notebook::EnterCommandMode", }, }, + { + "context": "GitGraph", + "bindings": { + "tab": "git_graph::FocusNextTabStop", + "shift-tab": "git_graph::FocusPreviousTabStop", + }, + }, + { + "context": "GitGraphSearchBar > Editor", + "bindings": { + "tab": "git_graph::FocusNextTabStop", + "shift-tab": "git_graph::FocusPreviousTabStop", + }, + }, ] diff --git a/assets/keymaps/vim.json b/assets/keymaps/vim.json index 188ea2e483..6e52ac3ce6 100644 --- a/assets/keymaps/vim.json +++ b/assets/keymaps/vim.json @@ -1035,6 +1035,20 @@ }, { "context": "GitGraph", + "bindings": { + "tab": "git_graph::FocusNextTabStop", + "shift-tab": "git_graph::FocusPreviousTabStop", + }, + }, + { + "context": "GitGraphSearchBar > Editor", + "bindings": { + "tab": "git_graph::FocusNextTabStop", + "shift-tab": "git_graph::FocusPreviousTabStop", + }, + }, + { + "context": "GitGraph && !GitGraphSearchBar", "bindings": { "j": "vim::MenuSelectNext", "k": "vim::MenuSelectPrevious", diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index e523ae2703..c625a3b97a 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -3536,6 +3536,10 @@ impl Editor { cx.notify(); } + pub fn show_cursor(&mut self, cx: &mut Context) { + self.blink_manager.update(cx, BlinkManager::show_cursor); + } + pub fn cursor_shape(&self) -> CursorShape { self.cursor_shape } diff --git a/crates/git_graph/src/git_graph.rs b/crates/git_graph/src/git_graph.rs index e92204c5f1..1f7d4b8866 100644 --- a/crates/git_graph/src/git_graph.rs +++ b/crates/git_graph/src/git_graph.rs @@ -282,6 +282,10 @@ actions!( OpenCommitView, /// Focuses the search field. FocusSearch, + /// Focuses the next git graph tab stop. + FocusNextTabStop, + /// Focuses the previous git graph tab stop. + FocusPreviousTabStop, ] ); @@ -1110,7 +1114,11 @@ impl GitGraph { editor }); - let table_interaction_state = cx.new(|cx| TableInteractionState::new(cx)); + let table_interaction_state = cx.new(|cx| { + let mut state = TableInteractionState::new(cx); + state.focus_handle = state.focus_handle.tab_index(1).tab_stop(true); + state + }); let column_widths = if matches!(log_source, LogSource::Path(_)) { cx.new(|_cx| { @@ -1602,6 +1610,39 @@ impl GitGraph { self.search(query, cx); } + fn activate_search_editor_if_focused(&self, window: &mut Window, cx: &mut Context) { + self.search_state.editor.update(cx, |editor, cx| { + if editor.is_focused(window) { + editor.select_all(&Default::default(), window, cx); + editor.show_cursor(cx); + } + }); + } + + fn focus_next_tab_stop( + &mut self, + _: &FocusNextTabStop, + window: &mut Window, + cx: &mut Context, + ) { + window.focus_next(cx); + self.activate_search_editor_if_focused(window, cx); + cx.stop_propagation(); + cx.notify(); + } + + fn focus_previous_tab_stop( + &mut self, + _: &FocusPreviousTabStop, + window: &mut Window, + cx: &mut Context, + ) { + window.focus_prev(cx); + self.activate_search_editor_if_focused(window, cx); + cx.stop_propagation(); + cx.notify(); + } + fn select_entry( &mut self, idx: usize, @@ -1783,7 +1824,12 @@ impl GitGraph { fn render_search_bar(&self, cx: &mut Context) -> impl IntoElement { let color = cx.theme().colors(); - let query_focus_handle = self.search_state.editor.focus_handle(cx); + let query_focus_handle = self + .search_state + .editor + .focus_handle(cx) + .tab_index(1) + .tab_stop(true); let search_options = { let mut options = SearchOptions::NONE; options.set( @@ -1794,6 +1840,10 @@ impl GitGraph { }; h_flex() + .key_context("GitGraphSearchBar") + .tab_index(1) + .tab_group() + .tab_stop(false) .w_full() .p_1p5() .gap_1p5() @@ -1806,6 +1856,7 @@ impl GitGraph { .min_w_0() .px_1p5() .gap_1() + .track_focus(&query_focus_handle) .border_1() .border_color(color.border_variant) .rounded_md() @@ -2811,6 +2862,8 @@ impl Render for GitGraph { let hovered_entry_idx = self.hovered_entry_idx; let weak_self = cx.weak_entity(); let focus_handle = self.focus_handle.clone(); + let table_focus_handle = + self.table_interaction_state.read(cx).focus_handle.clone(); let graph_canvas = div() .id("graph-canvas") @@ -2840,7 +2893,9 @@ impl Render for GitGraph { .map_row(move |(index, row), window, cx| { let is_selected = selected_entry_idx == Some(index); let is_hovered = hovered_entry_idx == Some(index); - let is_focused = focus_handle.is_focused(window); + let table_focus_handle = table_focus_handle.clone(); + let is_focused = focus_handle.is_focused(window) + || table_focus_handle.is_focused(window); let weak = weak_self.clone(); let weak_for_hover = weak.clone(); @@ -2873,6 +2928,7 @@ impl Render for GitGraph { }) .on_click(move |event, window, cx| { let click_count = event.click_count(); + table_focus_handle.focus(window, cx); weak.update(cx, |this, cx| { this.select_entry( index, @@ -2914,6 +2970,9 @@ impl Render for GitGraph { }) .child( div() + .tab_index(2) + .tab_group() + .tab_stop(false) .w(DefiniteLength::Fraction(table_fraction)) .h_full() .min_w_0() @@ -2958,12 +3017,15 @@ impl Render for GitGraph { this.search_state .editor .update(cx, |editor, cx| editor.focus_handle(cx).focus(window, cx)); + this.activate_search_editor_if_focused(window, cx); })) .on_action(cx.listener(Self::select_first)) .on_action(cx.listener(Self::select_prev)) .on_action(cx.listener(Self::select_next)) .on_action(cx.listener(Self::select_last)) .on_action(cx.listener(Self::confirm)) + .on_action(cx.listener(Self::focus_next_tab_stop)) + .on_action(cx.listener(Self::focus_previous_tab_stop)) .on_action(cx.listener(|this, _: &SelectNextMatch, _window, cx| { this.select_next_match(cx); }))