From d9a30905db12945626699e081d029e7c6b9fae17 Mon Sep 17 00:00:00 2001 From: "gcp-cherry-pick-bot[bot]" <98988430+gcp-cherry-pick-bot[bot]@users.noreply.github.com> Date: Mon, 13 Jan 2025 23:28:57 +0200 Subject: [PATCH] terminal: Fix unresponsive buttons on load until center pane is clicked + Auto-focus docked terminal on load if no other item is focused (cherry-pick #23039) (#23093) Cherry-picked terminal: Fix unresponsive buttons on load until center pane is clicked + Auto-focus docked terminal on load if no other item is focused (#23039) Closes #23006 This PR should have been split into two, but since the changes are related, I merged them into one. 1. On load, the title bar actions and bottom bar toggles are unresponsive until the center pane is clicked. This happens because the terminal captures focus (even if it's closed) long after the workspace sets focus to itself during loading. The issue was in the `focus_view` call used in the `new` method of `TerminalPanel`. Since new terminal views can be created behind the scenes (i.e., without the terminal being visible to the user), we shouldn't handle focus for the terminal in this case. Removing `focus_view` from the `new` method has no impact on the existing terminal focusing logic. I've tested scenarios such as creating new terminals, splitting terminals, zooming, etc., and everything works as expected. 2. Currently, on load, docked terminals do not automatically focus when they are only visible item to the user. This PR implements it. Before/After: 1. When only the dock terminal is visible on load. Terminal is focused. image image 2. When other items are visible along with the dock terminal on load. Editor is focused. image image 3. Multiple tabs along with split panes. Last terminal is focused. image image Future: When a docked terminal is in a zoomed state and Zed is loaded, we should prioritize focusing on the terminal over the active item (e.g., an editor) behind it. This hasn't been implemented in this PR because the zoomed state during the load function is stale. The correct state is received later via the workspace. I'm still investigating where exactly this should be handled, so this will be a separate PR. cc: @SomeoneToIgnore Release Notes: - Fixed unresponsive buttons on load until the center pane is clicked. - Added auto-focus for the docked terminal on load when no other item is focused. Co-authored-by: tims <0xtimsb@gmail.com> --- crates/terminal_view/src/persistence.rs | 11 +++++++---- crates/terminal_view/src/terminal_panel.rs | 22 ++++++++++++++++++++-- crates/workspace/src/workspace.rs | 13 +++++++++++++ 3 files changed, 40 insertions(+), 6 deletions(-) diff --git a/crates/terminal_view/src/persistence.rs b/crates/terminal_view/src/persistence.rs index a3bb2cc522b0441fd986f1e1fffad0682ecc20c2..36b07bdd6dda9ce6adc25b0a776346bd7e858371 100644 --- a/crates/terminal_view/src/persistence.rs +++ b/crates/terminal_view/src/persistence.rs @@ -146,13 +146,16 @@ fn populate_pane_items( cx: &mut ViewContext, ) { let mut item_index = pane.items_len(); + let mut active_item_index = None; for item in items { - let activate_item = Some(item.item_id().as_u64()) == active_item; + if Some(item.item_id().as_u64()) == active_item { + active_item_index = Some(item_index); + } pane.add_item(Box::new(item), false, false, None, cx); item_index += 1; - if activate_item { - pane.activate_item(item_index, false, false, cx); - } + } + if let Some(index) = active_item_index { + pane.activate_item(index, false, false, cx); } } diff --git a/crates/terminal_view/src/terminal_panel.rs b/crates/terminal_view/src/terminal_panel.rs index 1d6298a581a4aa4d3c2cebd175325256b872215d..2faecf163b94971b1d15c6387b96650644e927a1 100644 --- a/crates/terminal_view/src/terminal_panel.rs +++ b/crates/terminal_view/src/terminal_panel.rs @@ -31,7 +31,7 @@ use ui::{ }; use util::{ResultExt, TryFutureExt}; use workspace::{ - dock::{DockPosition, Panel, PanelEvent}, + dock::{DockPosition, Panel, PanelEvent, PanelHandle}, item::SerializableItem, move_active_item, move_item, pane, ui::IconName, @@ -83,7 +83,6 @@ impl TerminalPanel { let project = workspace.project(); let pane = new_terminal_pane(workspace.weak_handle(), project.clone(), false, cx); let center = PaneGroup::new(pane.clone()); - cx.focus_view(&pane); let terminal_panel = Self { center, active_pane: pane, @@ -283,6 +282,25 @@ impl TerminalPanel { } } + if let Some(workspace) = workspace.upgrade() { + let should_focus = workspace + .update(&mut cx, |workspace, cx| { + workspace.active_item(cx).is_none() + && workspace.is_dock_at_position_open(terminal_panel.position(cx), cx) + }) + .unwrap_or(false); + + if should_focus { + terminal_panel + .update(&mut cx, |panel, cx| { + panel.active_pane.update(cx, |pane, cx| { + pane.focus_active_item(cx); + }); + }) + .ok(); + } + } + Ok(terminal_panel) } diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 97ef07a9179504512f5b5c6ea5e611903fedc81b..a2c102aa7076d9812bacb3a6204524daaee6b1e7 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -2274,6 +2274,19 @@ impl Workspace { } } + pub fn is_dock_at_position_open( + &self, + position: DockPosition, + cx: &mut ViewContext, + ) -> bool { + let dock = match position { + DockPosition::Left => &self.left_dock, + DockPosition::Bottom => &self.bottom_dock, + DockPosition::Right => &self.right_dock, + }; + dock.read(cx).is_open() + } + pub fn toggle_dock(&mut self, dock_side: DockPosition, cx: &mut ViewContext) { let dock = match dock_side { DockPosition::Left => &self.left_dock,