Skip to content

Commit

Permalink
fix(screen): focus pane on screen edge when moving pane focus offtab (z…
Browse files Browse the repository at this point in the history
…ellij-org#2293)

* fix(screen): focus pane on proper edge when switching tabs through pane switch

* style(fmt): rustfmt
  • Loading branch information
imsnif authored Mar 15, 2023
1 parent 96b3ce2 commit c0dd1ba
Show file tree
Hide file tree
Showing 7 changed files with 201 additions and 23 deletions.
46 changes: 45 additions & 1 deletion zellij-server/src/panes/floating_panes/floating_pane_grid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::tab::{MIN_TERMINAL_HEIGHT, MIN_TERMINAL_WIDTH};
use crate::{panes::PaneId, tab::Pane};
use std::cmp::Ordering;
use std::collections::HashMap;
use zellij_utils::data::ResizeStrategy;
use zellij_utils::data::{Direction, ResizeStrategy};
use zellij_utils::errors::prelude::*;
use zellij_utils::pane_size::{Dimension, PaneGeom, Size, Viewport};

Expand Down Expand Up @@ -625,6 +625,50 @@ impl<'a> FloatingPaneGrid<'a> {
.copied();
next_index
}
pub fn pane_id_on_edge(&self, direction: Direction) -> Option<PaneId> {
let panes = self.panes.borrow();
let panes: Vec<(PaneId, &&mut Box<dyn Pane>)> = panes
.iter()
.filter(|(_, p)| p.selectable())
.map(|(p_id, p)| (*p_id, p))
.collect();
let next_index = panes
.iter()
.enumerate()
.max_by(|(_, (_, a)), (_, (_, b))| match direction {
Direction::Left => {
let x_comparison = a.x().cmp(&b.x());
match x_comparison {
Ordering::Equal => a.y().cmp(&b.y()),
_ => x_comparison,
}
},
Direction::Right => {
let x_comparison = b.x().cmp(&a.x());
match x_comparison {
Ordering::Equal => a.y().cmp(&b.y()),
_ => x_comparison,
}
},
Direction::Up => {
let y_comparison = a.y().cmp(&b.y());
match y_comparison {
Ordering::Equal => a.x().cmp(&b.x()),
_ => y_comparison,
}
},
Direction::Down => {
let y_comparison = b.y().cmp(&a.y());
match y_comparison {
Ordering::Equal => b.x().cmp(&a.x()),
_ => y_comparison,
}
},
})
.map(|(_, (pid, _))| pid)
.copied();
next_index
}
pub fn next_selectable_pane_id_below(&self, current_pane_id: &PaneId) -> Option<PaneId> {
let panes = self.panes.borrow();
let current_pane = panes.get(current_pane_id)?;
Expand Down
13 changes: 13 additions & 0 deletions zellij-server/src/panes/floating_panes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,19 @@ impl FloatingPanes {
}
Ok(false)
}
pub fn focus_pane_on_edge(&mut self, direction: Direction, client_id: ClientId) {
let display_area = *self.display_area.borrow();
let viewport = *self.viewport.borrow();
let mut floating_pane_grid = FloatingPaneGrid::new(
&mut self.panes,
&mut self.desired_pane_positions,
display_area,
viewport,
);
let pane_id = floating_pane_grid.pane_id_on_edge(direction).unwrap();
self.focus_pane(pane_id, client_id);
self.set_force_render();
}

pub fn move_active_pane_down(&mut self, client_id: ClientId) {
let display_area = *self.display_area.borrow();
Expand Down
31 changes: 30 additions & 1 deletion zellij-server/src/panes/tiled_panes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::{
};
use stacked_panes::StackedPanes;
use zellij_utils::{
data::{ModeInfo, ResizeStrategy, Style},
data::{Direction, ModeInfo, ResizeStrategy, Style},
errors::prelude::*,
input::{command::RunCommand, layout::SplitDirection},
pane_size::{Offset, PaneGeom, Size, SizeInPixels, Viewport},
Expand Down Expand Up @@ -862,6 +862,35 @@ impl TiledPanes {
(size_in_pixels.height as f64 / size_in_pixels.width as f64).round() as usize
})
}
pub fn focus_pane_on_edge(&mut self, direction: Direction, client_id: ClientId) {
let pane_grid = TiledPaneGrid::new(
&mut self.panes,
&self.panes_to_hide,
*self.display_area.borrow(),
*self.viewport.borrow(),
);
let next_index = pane_grid.pane_id_on_edge(direction).unwrap();
// render previously active pane so that its frame does not remain actively
// colored
let previously_active_pane = self
.panes
.get_mut(self.active_panes.get(&client_id).unwrap())
.unwrap();

previously_active_pane.set_should_render(true);
// we render the full viewport to remove any ui elements that might have been
// there before (eg. another user's cursor)
previously_active_pane.render_full_viewport();

let next_active_pane = self.panes.get_mut(&next_index).unwrap();
next_active_pane.set_should_render(true);
// we render the full viewport to remove any ui elements that might have been
// there before (eg. another user's cursor)
next_active_pane.render_full_viewport();

self.focus_pane(next_index, client_id);
self.set_pane_active_at(next_index);
}
pub fn move_focus_left(&mut self, client_id: ClientId) -> bool {
match self.get_active_pane_id(client_id) {
Some(active_pane_id) => {
Expand Down
46 changes: 45 additions & 1 deletion zellij-server/src/panes/tiled_panes/tiled_pane_grid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use super::pane_resizer::PaneResizer;
use super::stacked_panes::StackedPanes;
use crate::tab::{MIN_TERMINAL_HEIGHT, MIN_TERMINAL_WIDTH};
use crate::{panes::PaneId, tab::Pane};
use std::cmp::Reverse;
use std::cmp::{Ordering, Reverse};
use std::collections::{HashMap, HashSet};
use zellij_utils::data::{Direction, Resize, ResizeStrategy};
use zellij_utils::{
Expand Down Expand Up @@ -1025,6 +1025,50 @@ impl<'a> TiledPaneGrid<'a> {
.copied();
next_index
}
pub fn pane_id_on_edge(&self, direction: Direction) -> Option<PaneId> {
let panes = self.panes.borrow();
let panes: Vec<(PaneId, &&mut Box<dyn Pane>)> = panes
.iter()
.filter(|(_, p)| p.selectable())
.map(|(p_id, p)| (*p_id, p))
.collect();
let next_index = panes
.iter()
.enumerate()
.max_by(|(_, (_, a)), (_, (_, b))| match direction {
Direction::Left => {
let x_comparison = a.x().cmp(&b.x());
match x_comparison {
Ordering::Equal => b.y().cmp(&a.y()),
_ => x_comparison,
}
},
Direction::Right => {
let x_comparison = b.x().cmp(&a.x());
match x_comparison {
Ordering::Equal => b.y().cmp(&a.y()),
_ => x_comparison,
}
},
Direction::Up => {
let y_comparison = a.y().cmp(&b.y());
match y_comparison {
Ordering::Equal => a.x().cmp(&b.x()),
_ => y_comparison,
}
},
Direction::Down => {
let y_comparison = b.y().cmp(&a.y());
match y_comparison {
Ordering::Equal => b.x().cmp(&a.x()),
_ => y_comparison,
}
},
})
.map(|(_, (pid, _))| pid)
.copied();
next_index
}
pub fn next_selectable_pane_id_above(&self, current_pane_id: &PaneId) -> Option<PaneId> {
let panes = self.panes.borrow();
let current_pane = panes.get(current_pane_id)?;
Expand Down
61 changes: 50 additions & 11 deletions zellij-server/src/screen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,12 @@ impl Screen {
}

/// A helper function to switch to a new tab at specified position.
fn switch_active_tab(&mut self, new_tab_pos: usize, client_id: ClientId) -> Result<()> {
fn switch_active_tab(
&mut self,
new_tab_pos: usize,
should_change_pane_focus: Option<Direction>,
client_id: ClientId,
) -> Result<()> {
let err_context = || {
format!(
"Failed to switch to active tab at position {new_tab_pos} for client id: {client_id:?}"
Expand All @@ -621,6 +626,15 @@ impl Screen {
self.connected_clients.borrow().iter().copied().collect();
for client_id in all_connected_clients {
self.update_client_tab_focus(client_id, new_tab_index);
match (
should_change_pane_focus,
self.get_indexed_tab_mut(new_tab_index),
) {
(Some(direction), Some(new_tab)) => {
new_tab.focus_pane_on_edge(direction, client_id);
},
_ => {},
}
}
} else {
self.move_clients_between_tabs(
Expand All @@ -629,6 +643,15 @@ impl Screen {
Some(vec![client_id]),
)
.with_context(err_context)?;
match (
should_change_pane_focus,
self.get_indexed_tab_mut(new_tab_index),
) {
(Some(direction), Some(new_tab)) => {
new_tab.focus_pane_on_edge(direction, client_id);
},
_ => {},
}
self.update_client_tab_focus(client_id, new_tab_index);
}

Expand Down Expand Up @@ -656,15 +679,19 @@ impl Screen {
fn switch_active_tab_name(&mut self, name: String, client_id: ClientId) -> Result<bool> {
match self.tabs.values().find(|t| t.name == name) {
Some(new_tab) => {
self.switch_active_tab(new_tab.position, client_id)?;
self.switch_active_tab(new_tab.position, None, client_id)?;
Ok(true)
},
None => Ok(false),
}
}

/// Sets this [`Screen`]'s active [`Tab`] to the next tab.
pub fn switch_tab_next(&mut self, client_id: ClientId) -> Result<()> {
pub fn switch_tab_next(
&mut self,
should_change_pane_focus: Option<Direction>,
client_id: ClientId,
) -> Result<()> {
let err_context = || format!("failed to switch to next tab for client {client_id}");

let client_id = if self.get_active_tab(client_id).is_ok() {
Expand All @@ -678,7 +705,11 @@ impl Screen {
Ok(active_tab) => {
let active_tab_pos = active_tab.position;
let new_tab_pos = (active_tab_pos + 1) % self.tabs.len();
return self.switch_active_tab(new_tab_pos, client_id);
return self.switch_active_tab(
new_tab_pos,
should_change_pane_focus,
client_id,
);
},
Err(err) => Err::<(), _>(err).with_context(err_context).non_fatal(),
}
Expand All @@ -687,7 +718,11 @@ impl Screen {
}

/// Sets this [`Screen`]'s active [`Tab`] to the previous tab.
pub fn switch_tab_prev(&mut self, client_id: ClientId) -> Result<()> {
pub fn switch_tab_prev(
&mut self,
should_change_pane_focus: Option<Direction>,
client_id: ClientId,
) -> Result<()> {
let err_context = || format!("failed to switch to previous tab for client {client_id}");

let client_id = if self.get_active_tab(client_id).is_ok() {
Expand All @@ -706,7 +741,11 @@ impl Screen {
active_tab_pos - 1
};

return self.switch_active_tab(new_tab_pos, client_id);
return self.switch_active_tab(
new_tab_pos,
should_change_pane_focus,
client_id,
);
},
Err(err) => Err::<(), _>(err).with_context(err_context).non_fatal(),
}
Expand All @@ -715,7 +754,7 @@ impl Screen {
}

pub fn go_to_tab(&mut self, tab_index: usize, client_id: ClientId) -> Result<()> {
self.switch_active_tab(tab_index.saturating_sub(1), client_id)
self.switch_active_tab(tab_index.saturating_sub(1), None, client_id)
}

pub fn go_to_tab_name(&mut self, name: String, client_id: ClientId) -> Result<bool> {
Expand Down Expand Up @@ -1320,7 +1359,7 @@ impl Screen {
.move_focus_left(client_id)
.and_then(|success| {
if !success {
self.switch_tab_prev(client_id)
self.switch_tab_prev(Some(Direction::Left), client_id)
.context("failed to move focus to previous tab")
} else {
Ok(())
Expand Down Expand Up @@ -1354,7 +1393,7 @@ impl Screen {
.move_focus_right(client_id)
.and_then(|success| {
if !success {
self.switch_tab_next(client_id)
self.switch_tab_next(Some(Direction::Right), client_id)
.context("failed to move focus to next tab")
} else {
Ok(())
Expand Down Expand Up @@ -2004,12 +2043,12 @@ pub(crate) fn screen_thread_main(
screen.unblock_input()?;
},
ScreenInstruction::SwitchTabNext(client_id) => {
screen.switch_tab_next(client_id)?;
screen.switch_tab_next(None, client_id)?;
screen.unblock_input()?;
screen.render()?;
},
ScreenInstruction::SwitchTabPrev(client_id) => {
screen.switch_tab_prev(client_id)?;
screen.switch_tab_prev(None, client_id)?;
screen.unblock_input()?;
screen.render()?;
},
Expand Down
9 changes: 9 additions & 0 deletions zellij-server/src/tab/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1915,6 +1915,15 @@ impl Tab {
}
self.tiled_panes.focus_previous_pane(client_id);
}
pub fn focus_pane_on_edge(&mut self, direction: Direction, client_id: ClientId) {
let err_context = || format!("failed to move focus left for client {}", client_id);

if self.floating_panes.panes_are_visible() {
self.floating_panes.focus_pane_on_edge(direction, client_id);
} else if self.has_selectable_panes() && !self.tiled_panes.fullscreen_is_active() {
self.tiled_panes.focus_pane_on_edge(direction, client_id);
}
}
// returns a boolean that indicates whether the focus moved
pub fn move_focus_left(&mut self, client_id: ClientId) -> Result<bool> {
let err_context = || format!("failed to move focus left for client {}", client_id);
Expand Down
Loading

0 comments on commit c0dd1ba

Please sign in to comment.