From 45be23c56c2bfac6573920be7ad5a21943ec09c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Soares?= <37777652+Dnreikronos@users.noreply.github.com> Date: Tue, 24 Mar 2026 08:52:13 -0300 Subject: [PATCH] Fix git panel context menu keybinding jitter (#52217) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Context The git panel's context menu caused visual jitter (flickering/jumping) when opened via right-click on a tracked file. The root cause was in `dispatch_context()`: it used `self.focus_handle == focused` to check if the panel itself was directly focused, but when a context menu opened, focus moved to the menu (a child element), causing the `"menu"` and `"ChangesList"` key contexts to be dropped. This triggered a re-render with different keybindings, which re-added them, creating a loop of jitter. The fix replaces the direct focus equality check with `self.focus_handle.contains_focused(window, cx)`, which returns `true` when any child element (including the context menu) holds focus within the panel's focus tree. This is consistent with how other panels (project panel, outline panel, collab panel) handle focus-based dispatch contexts. Closes #51813 ## Demo **Before fix:** https://github.com/user-attachments/assets/e18d49b2-72a6-4411-8ec5-519e36628f29 **After fix:** https://github.com/user-attachments/assets/94c936d2-1e81-4d28-a86a-8b1ed76ddde1 ## How to Review This is a small, focused change in a single file: `crates/git_ui/src/git_panel.rs`. 1. **The fix** (~line 974): `dispatch_context()` method — the old code checked direct focus equality (`self.focus_handle == focused`), the new code uses `self.focus_handle.contains_focused(window, cx)` and restructures the conditionals so `CommitEditor` is checked first via `if/else if`. 2. **The test** (~line 7871): `test_dispatch_context_with_focus_states` — verifies 4 focus state transitions: commit editor focused, changes list focused, back to commit editor, and back to changes list. Each case asserts the correct key contexts are present/absent. ## Self-Review Checklist - [x] I've reviewed my own diff for quality, security, and reliability - [ ] 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) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - Fixed git panel context menu jitter caused by keybinding dispatch context flickering when right-clicking on tracked files (#51813) --- crates/git_ui/src/git_panel.rs | 142 ++++++++++++++++++++++++++++++--- 1 file changed, 133 insertions(+), 9 deletions(-) diff --git a/crates/git_ui/src/git_panel.rs b/crates/git_ui/src/git_panel.rs index d8476555f555134a78d06d9d22970fb3a111b2ea..3eb118f82943a26a6a35a8ebac8916d5a5a10ad0 100644 --- a/crates/git_ui/src/git_panel.rs +++ b/crates/git_ui/src/git_panel.rs @@ -975,16 +975,11 @@ impl GitPanel { let mut dispatch_context = KeyContext::new_with_defaults(); dispatch_context.add("GitPanel"); - if window - .focused(cx) - .is_some_and(|focused| self.focus_handle == focused) - { - dispatch_context.add("menu"); - dispatch_context.add("ChangesList"); - } - if self.commit_editor.read(cx).is_focused(window) { dispatch_context.add("CommitEditor"); + } else if self.focus_handle.contains_focused(window, cx) { + dispatch_context.add("menu"); + dispatch_context.add("ChangesList"); } dispatch_context @@ -6488,7 +6483,7 @@ mod tests { repository::repo_path, status::{StatusCode, UnmergedStatus, UnmergedStatusCode}, }; - use gpui::{TestAppContext, UpdateGlobal, VisualTestContext}; + use gpui::{TestAppContext, UpdateGlobal, VisualTestContext, px}; use indoc::indoc; use project::FakeFs; use serde_json::json; @@ -7874,4 +7869,133 @@ mod tests { let message = panel.update(cx, |panel, cx| panel.suggest_commit_message(cx)); assert_eq!(message, Some("Update tracked".to_string())); } + + #[gpui::test] + async fn test_dispatch_context_with_focus_states(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.background_executor.clone()); + fs.insert_tree( + path!("/project"), + json!({ + ".git": {}, + "tracked": "tracked\n", + }), + ) + .await; + + fs.set_head_and_index_for_repo( + path!("/project/.git").as_ref(), + &[("tracked", "old tracked\n".into())], + ); + + let project = Project::test(fs.clone(), [Path::new(path!("/project"))], cx).await; + let window_handle = + cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + let workspace = window_handle + .read_with(cx, |mw, _| mw.workspace().clone()) + .unwrap(); + let cx = &mut VisualTestContext::from_window(window_handle.into(), cx); + let panel = workspace.update_in(cx, GitPanel::new); + + let handle = cx.update_window_entity(&panel, |panel, _, _| { + std::mem::replace(&mut panel.update_visible_entries_task, Task::ready(())) + }); + cx.executor().advance_clock(2 * UPDATE_DEBOUNCE); + handle.await; + + // Case 1: Focus the commit editor — should have "CommitEditor" but NOT "menu"/"ChangesList" + panel.update_in(cx, |panel, window, cx| { + panel.focus_editor(&FocusEditor, window, cx); + let editor_is_focused = panel.commit_editor.read(cx).is_focused(window); + assert!( + editor_is_focused, + "commit editor should be focused after focus_editor action" + ); + let context = panel.dispatch_context(window, cx); + assert!( + context.contains("GitPanel"), + "should always have GitPanel context" + ); + assert!( + context.contains("CommitEditor"), + "should have CommitEditor context when commit editor is focused" + ); + assert!( + !context.contains("menu"), + "should not have menu context when commit editor is focused" + ); + assert!( + !context.contains("ChangesList"), + "should not have ChangesList context when commit editor is focused" + ); + }); + + // Case 2: Focus the panel's focus handle directly — should have "menu" and "ChangesList". + // We force a draw via simulate_resize to ensure the dispatch tree is populated, + // since contains_focused() depends on the rendered dispatch tree. + panel.update_in(cx, |panel, window, cx| { + panel.focus_handle.focus(window, cx); + }); + cx.simulate_resize(gpui::size(px(800.), px(600.))); + + panel.update_in(cx, |panel, window, cx| { + let context = panel.dispatch_context(window, cx); + assert!( + context.contains("GitPanel"), + "should always have GitPanel context" + ); + assert!( + context.contains("menu"), + "should have menu context when changes list is focused" + ); + assert!( + context.contains("ChangesList"), + "should have ChangesList context when changes list is focused" + ); + assert!( + !context.contains("CommitEditor"), + "should not have CommitEditor context when changes list is focused" + ); + }); + + // Case 3: Switch back to commit editor and verify context switches correctly + panel.update_in(cx, |panel, window, cx| { + panel.focus_editor(&FocusEditor, window, cx); + }); + + panel.update_in(cx, |panel, window, cx| { + let context = panel.dispatch_context(window, cx); + assert!( + context.contains("CommitEditor"), + "should have CommitEditor after switching focus back to editor" + ); + assert!( + !context.contains("menu"), + "should not have menu after switching focus back to editor" + ); + }); + + // Case 4: Re-focus changes list and verify it transitions back correctly + panel.update_in(cx, |panel, window, cx| { + panel.focus_handle.focus(window, cx); + }); + cx.simulate_resize(gpui::size(px(800.), px(600.))); + + panel.update_in(cx, |panel, window, cx| { + assert!( + panel.focus_handle.contains_focused(window, cx), + "panel focus handle should report contains_focused when directly focused" + ); + let context = panel.dispatch_context(window, cx); + assert!( + context.contains("menu"), + "should have menu context after re-focusing changes list" + ); + assert!( + context.contains("ChangesList"), + "should have ChangesList context after re-focusing changes list" + ); + }); + } }