Skip to content

Commit

Permalink
terminal: Fix unresponsive buttons on load until center pane is click…
Browse files Browse the repository at this point in the history
…ed + 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.

<img
src="https://github.com/user-attachments/assets/af8848aa-ccb5-4a3b-b2c6-486e8d588f09"
alt="image" height="280px" />

<img
src="https://github.com/user-attachments/assets/8f76ca2e-de29-4cc0-979b-749b50a00bbd"
alt="image" height="280px" />

2. When other items are visible along with the dock terminal on load.
Editor is focused.

<img
src="https://github.com/user-attachments/assets/d3248272-a75d-4763-9e99-defb8a369b68"
alt="image" height="280px" />

<img
src="https://github.com/user-attachments/assets/fba5184e-1ab2-406c-9669-b141aaf1c32f"
alt="image" height="280px" />

3. Multiple tabs along with split panes. Last terminal is focused.

<img
src="https://github.com/user-attachments/assets/7a10c3cf-8bb3-4b88-aacc-732b678bee19"
alt="image" height="270px" />

<img
src="https://github.com/user-attachments/assets/4d16e98f-9d7a-45f6-8701-d6652e411d3b"
alt="image" height="270px" />

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.
  • Loading branch information
0xtimsb authored Jan 13, 2025
1 parent 13405ed commit 7ed834b
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 6 deletions.
11 changes: 7 additions & 4 deletions crates/terminal_view/src/persistence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,13 +146,16 @@ fn populate_pane_items(
cx: &mut ViewContext<Pane>,
) {
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);
}
}

Expand Down
22 changes: 20 additions & 2 deletions crates/terminal_view/src/terminal_panel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
}

Expand Down
13 changes: 13 additions & 0 deletions crates/workspace/src/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2298,6 +2298,19 @@ impl Workspace {
}
}

pub fn is_dock_at_position_open(
&self,
position: DockPosition,
cx: &mut ViewContext<Self>,
) -> 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<Self>) {
let dock = match dock_side {
DockPosition::Left => &self.left_dock,
Expand Down

0 comments on commit 7ed834b

Please sign in to comment.