diff options
| author | user <user@clank> | 2026-08-11 16:19:19 +0200 |
|---|---|---|
| committer | user <user@clank> | 2026-08-11 16:19:19 +0200 |
| commit | c51e5caa6d5c9368db0ddbd6b406047dfc451e20 (patch) | |
| tree | 6747c34b75272888520315b17341f521b0d9409c /zellij-server/src/tab | |
| parent | sl0p: fix clicking a scrolled stack entry selecting the wrong pane (diff) | |
| download | sl0ppij-c51e5caa6d5c9368db0ddbd6b406047dfc451e20.tar.gz sl0ppij-c51e5caa6d5c9368db0ddbd6b406047dfc451e20.tar.xz sl0ppij-c51e5caa6d5c9368db0ddbd6b406047dfc451e20.zip | |
sl0p: click the picker's indicator row to expand a capped stack
The cap made the picker a scrolling viewport -- everything stays reachable by
moving focus -- but there was no way to see the whole stack at once.
Clicking "+N more" expands it to every member; the indicator becomes "- fewer"
and collapses it again. Collapse gets its own label because an invisible way
back out is not an affordance.
Hangs off the indicator row because that row is already reserved, already
rendered, and resolves to no member, so it is free real estate. It is also why
this needs no new keybinding: mouse handling is entirely server-side, while
Action is a protobuf enum we cannot extend without protoc.
A click rather than hover, as expanding changes the reserved row count and so
resizes the visible pane's PTY -- acceptable once per deliberate click, not on
every mouse move.
All three consumers of the row -> member mapping (renderer, reserved-row
budget, mouse hit-test) now go through stack_list_window_for, so expansion
cannot desync them the way the cap desynced the hit-test in 844617b4.
Diffstat (limited to 'zellij-server/src/tab')
| -rw-r--r-- | zellij-server/src/tab/mod.rs | 102 | ||||
| -rw-r--r-- | zellij-server/src/tab/mouse_handler.rs | 28 | ||||
| -rw-r--r-- | zellij-server/src/tab/unit/tab_integration_tests.rs | 86 |
3 files changed, 208 insertions, 8 deletions
diff --git a/zellij-server/src/tab/mod.rs b/zellij-server/src/tab/mod.rs index 7f132465..f1f5e630 100644 --- a/zellij-server/src/tab/mod.rs +++ b/zellij-server/src/tab/mod.rs @@ -282,6 +282,8 @@ pub(crate) struct Tab { responsive_layout_name: String, /// Cap on picker rows; see stack_list_window. 0 = uncapped. stack_list_max_rows: usize, + /// Stacks the user has expanded past the cap by clicking "+N more". + expanded_stack_lists: HashSet<StackListId>, currently_marking_pane_group: Rc<RefCell<HashMap<ClientId, bool>>>, connected_clients_in_app: Rc<RefCell<HashMap<ClientId, bool>>>, // bool -> is_web_client // the below are the configured values - the ones that will be set if and when the web server @@ -1098,6 +1100,7 @@ impl Tab { responsive_min_cols: 0, responsive_layout_name: String::from("stacked"), stack_list_max_rows: 0, + expanded_stack_lists: HashSet::new(), connected_clients_in_app, web_server_ip, web_server_port, @@ -1176,7 +1179,8 @@ impl Tab { // the wrong pane for every capped, scrolled stack. let row = (point.line() - geom.y as isize) as usize; let visible_rank = list.rank_of(&list.visible).unwrap_or(0); - let (window_start, shown_count) = self.stack_list_window(list.members.len(), visible_rank); + let (window_start, shown_count) = + self.stack_list_window_for(stack_list_id, list.members.len(), visible_rank); if row >= shown_count { // the "+N more" indicator row, which is not a member return pane_id; @@ -1284,6 +1288,82 @@ impl Tab { /// Both the reserved-row arithmetic and the renderer go through this. If /// they ever disagree the list draws into rows the geometry did not reserve, /// i.e. over the pane below it. + /// The window for a specific stack, honouring a user expansion. + /// + /// Every consumer of the row -> member mapping must go through this rather + /// than the raw arithmetic below, or the renderer, the reserved-row budget + /// and the mouse hit-test can disagree about which member a row shows. + fn stack_list_window_for( + &self, + stack_list_id: StackListId, + member_count: usize, + visible_rank: usize, + ) -> (usize, usize) { + if self.expanded_stack_lists.contains(&stack_list_id) { + return (0, member_count); + } + self.stack_list_window(member_count, visible_rank) + } + /// Text for the picker's indicator row, if it has one. + /// + /// Rendered in the padding row the list already reserves, so it never costs + /// a row or displaces a member. Also the click target that toggles the + /// expansion, which is why collapse gets a label too: an invisible way back + /// out is not an affordance. + fn stack_list_indicator( + &self, + stack_list_id: StackListId, + member_count: usize, + shown: usize, + ) -> Option<String> { + let hidden = member_count.saturating_sub(shown); + if hidden > 0 { + Some(format!("+{} more", hidden)) + } else if self.expanded_stack_lists.contains(&stack_list_id) { + Some(String::from("- fewer")) + } else { + None + } + } + /// Toggle a stack's expansion if `point` is on its indicator row. + /// + /// Returns true if the click was consumed. Expanding changes the reserved + /// row count, hence the refresh: that is a one-off pane resize, which is + /// why this hangs off a deliberate click and not off hover. + fn toggle_stack_list_expansion_at_point(&mut self, pane_id: PaneId, point: &Position) -> bool { + let Some(stack_list_id) = self.stack_list_id_of_member(&pane_id) else { + return false; + }; + let Some(list) = self.stack_lists.get(&stack_list_id) else { + return false; + }; + let (member_count, visible) = (list.members.len(), list.visible); + let visible_rank = list.rank_of(&visible).unwrap_or(0); + let Some(pane) = self.tiled_panes.get_pane(visible) else { + return false; + }; + let geom = pane.current_geom(); + if point.line() < geom.y as isize { + return false; + } + let row = (point.line() - geom.y as isize) as usize; + let (_start, shown) = + self.stack_list_window_for(stack_list_id, member_count, visible_rank); + if row != shown + || self + .stack_list_indicator(stack_list_id, member_count, shown) + .is_none() + { + return false; + } + if !self.expanded_stack_lists.remove(&stack_list_id) { + self.expanded_stack_lists.insert(stack_list_id); + } + self.refresh_or_dissolve_stack_list(stack_list_id); + self.set_force_render(); + self.set_should_clear_display_before_rendering(); + true + } fn stack_list_window(&self, member_count: usize, visible_rank: usize) -> (usize, usize) { let cap = self.stack_list_max_rows; if cap == 0 || member_count <= cap { @@ -1306,7 +1386,8 @@ impl Tab { if member_count <= 1 { self.dissolve_stack_list(stack_list_id); } else { - let (_start, shown) = self.stack_list_window(member_count, visible_rank); + let (_start, shown) = + self.stack_list_window_for(stack_list_id, member_count, visible_rank); self.reserved_top_rows .borrow_mut() .insert(visible, shown + 1); @@ -1522,6 +1603,7 @@ impl Tab { } fn drop_stack_list_ledger(&mut self, stack_list_id: StackListId, members: &[PaneId]) { self.stack_lists.remove(&stack_list_id); + self.expanded_stack_lists.remove(&stack_list_id); let mut reserved = self.reserved_top_rows.borrow_mut(); for member in members { self.stack_list_of_member.remove(member); @@ -1730,8 +1812,12 @@ impl Tab { }) .collect() }; - let lists: Vec<StackList> = self.stack_lists.values().cloned().collect(); - for list in lists { + let lists: Vec<(StackListId, StackList)> = self + .stack_lists + .iter() + .map(|(id, list)| (*id, list.clone())) + .collect(); + for (stack_list_id, list) in lists { let (rect, visible_content_offset) = match self.tiled_panes.get_pane(list.visible) { Some(p) => (p.position_and_size(), p.get_content_offset()), None => continue, @@ -1763,8 +1849,9 @@ impl Tab { let stack_is_focused = active_panes.values().any(|p_id| *p_id == list.visible); let visible_rank = list.rank_of(&list.visible).unwrap_or(0); let (window_start, shown_count) = - self.stack_list_window(list.members.len(), visible_rank); - let hidden_count = list.members.len().saturating_sub(shown_count); + self.stack_list_window_for(stack_list_id, list.members.len(), visible_rank); + let indicator = + self.stack_list_indicator(stack_list_id, list.members.len(), shown_count); for (row, (rank, member)) in list .members .iter() @@ -1829,8 +1916,7 @@ impl Tab { // so the "+N hidden" indicator costs nothing extra and never steals a // row from a real entry. let mut padding_row = Vec::with_capacity(entry_cols); - if hidden_count > 0 { - let indicator = format!("+{} more", hidden_count); + if let Some(indicator) = indicator { let indicator_start = entry_cols.saturating_sub(indicator.width()) / 2; for _ in 0..indicator_start { padding_row.push(EMPTY_TERMINAL_CHARACTER); diff --git a/zellij-server/src/tab/mouse_handler.rs b/zellij-server/src/tab/mouse_handler.rs index eb3ebe9e..5e8c38e5 100644 --- a/zellij-server/src/tab/mouse_handler.rs +++ b/zellij-server/src/tab/mouse_handler.rs @@ -404,11 +404,39 @@ impl MouseHandler { if let Some(effect) = Self::intercept_guest_modal_mouse_event(tab, event, client_id)? { return Ok(effect); } + if let Some(effect) = Self::intercept_stack_list_indicator_click(tab, event)? { + return Ok(effect); + } let context = Self::gather_mouse_event_context(tab, event, client_id)?; let action = Self::determine_mouse_action(event, &context)?; Self::execute_mouse_action(tab, action, event, client_id) } + /// Clicking the picker's "+N more" / "- fewer" row toggles the stack's + /// expansion. + /// + /// Intercepted before normal handling because that row is not a member: it + /// resolves to no pane, so letting it fall through would either do nothing + /// or act on the pane underneath. Deliberately a click rather than hover, + /// since expanding changes the reserved row count and therefore resizes the + /// visible pane's PTY. + fn intercept_stack_list_indicator_click( + tab: &mut Tab, + event: &MouseEvent, + ) -> Result<Option<MouseEffect>> { + if !(event.left && event.event_type == MouseEventType::Press) { + return Ok(None); + } + let Some(pane_id) = Self::get_pane_at(tab, &event.position, false)?.map(|p| p.pid()) else { + return Ok(None); + }; + if tab.toggle_stack_list_expansion_at_point(pane_id, &event.position) { + Ok(Some(MouseEffect::state_changed())) + } else { + Ok(None) + } + } + fn intercept_guest_modal_mouse_event( tab: &mut Tab, event: &MouseEvent, diff --git a/zellij-server/src/tab/unit/tab_integration_tests.rs b/zellij-server/src/tab/unit/tab_integration_tests.rs index b22b38f9..c0034708 100644 --- a/zellij-server/src/tab/unit/tab_integration_tests.rs +++ b/zellij-server/src/tab/unit/tab_integration_tests.rs @@ -16136,3 +16136,89 @@ fn sl0p_clicking_a_scrolled_stack_entry_selects_the_pane_it_shows() { ); } } + +#[test] +fn sl0p_clicking_the_indicator_row_expands_and_collapses_the_stack_list() { + let size = Size { + cols: 121, + rows: 40, + }; + let client_id = 1; + let mut tab = create_new_tab(size, ModeInfo::default()); + let mut members = vec![PaneId::Terminal(1)]; + for pid in 2..=8 { + tab.new_pane( + PaneId::Terminal(pid), + None, + None, + false, + true, + NewPanePlacement::default(), + Some(client_id), + None, + ) + .unwrap(); + members.push(PaneId::Terminal(pid)); + } + let visible = members[7]; + tab.stack_lists.insert( + 0, + super::StackList { + members: members.clone(), + visible, + }, + ); + for member in &members { + tab.stack_list_of_member.insert(*member, 0); + } + tab.update_stack_list_max_rows(3); + + let geom = tab.tiled_panes.get_pane(visible).unwrap().current_geom(); + let row_of = |row: usize| Position::new((geom.y + row) as i32, 0); + + // collapsed: 3 of 8 shown, indicator on the row after them + assert_eq!(tab.stack_list_window_for(0, 8, 7), (5, 3)); + assert_eq!( + tab.stack_list_indicator(0, 8, 3), + Some(String::from("+5 more")) + ); + + // clicking a member row must NOT toggle + assert!( + !tab.toggle_stack_list_expansion_at_point(visible, &row_of(1)), + "a click on a member row must not toggle the expansion" + ); + assert_eq!(tab.stack_list_window_for(0, 8, 7), (5, 3)); + + // clicking the indicator row expands to every member + assert!( + tab.toggle_stack_list_expansion_at_point(visible, &row_of(3)), + "a click on the indicator row should be consumed" + ); + assert_eq!( + tab.stack_list_window_for(0, 8, 7), + (0, 8), + "expanded should show every member" + ); + assert_eq!( + tab.stack_list_indicator(0, 8, 8), + Some(String::from("- fewer")), + "expanded needs a visible way back, or the toggle is one-way" + ); + + // the reserved rows must follow, or the list draws over the pane below it + assert_eq!( + tab.reserved_top_rows.borrow().get(&visible).copied(), + Some(9), + "expanded reserves one row per member plus the indicator" + ); + + // and clicking it again collapses (indicator is now one row lower) + assert!(tab.toggle_stack_list_expansion_at_point(visible, &row_of(8))); + assert_eq!(tab.stack_list_window_for(0, 8, 7), (5, 3)); + assert_eq!( + tab.reserved_top_rows.borrow().get(&visible).copied(), + Some(4), + "collapsed reserves the cap plus the indicator" + ); +} |
