diff options
| author | user <user@clank> | 2026-08-08 23:32:35 +0200 |
|---|---|---|
| committer | user <user@clank> | 2026-08-08 23:32:35 +0200 |
| commit | b612c5342d0fb30530be6500314e70d85a4452d0 (patch) | |
| tree | a06a49ed1510bb1b5acdd065c8811d746776e6c7 | |
| parent | sl0p: highlight the drop target while dragging a pane (diff) | |
| download | sl0ppij-b612c5342d0fb30530be6500314e70d85a4452d0.tar.gz sl0ppij-b612c5342d0fb30530be6500314e70d85a4452d0.tar.xz sl0ppij-b612c5342d0fb30530be6500314e70d85a4452d0.zip | |
sl0p: preview pane reordering live while dragging
Dragging showed only a green border on the target. The swap is now applied as
you drag, so the real layout IS the preview and releasing just keeps it.
Dragging elsewhere undoes it and previews the new arrangement; dragging off all
panes restores the original.
Because swap_panes is symmetric, applying it twice is the identity, so 'undo
the preview' is just the same call again. Drop needs no work at all now - the
swap already happened - which also removes the old drop-time swap.
THE TRAP, which a naive version hits immediately: once the swap is applied the
dragged pane sits under the cursor, so the next motion computes the dragged
pane as its own target, undoes the swap, which puts the original pane back
under the cursor, which re-applies it - oscillating on every motion event and
spamming both PTYs with resizes.
Avoided by treating two cases as 'the layout already shows what you would get'
and doing nothing:
- the cursor is over the dragged pane, wherever the preview has moved it;
- the cursor is over the pane it is currently swapped with, which after a
preview sits at the dragged pane's original position.
So a swap is only re-applied when the cursor reaches a THIRD pane, which also
keeps PTY resizes down to one pair per target change rather than per event.
The dragged pane is kept highlighted rather than the target, so it stays
identifiable while the layout moves around it.
Note: the state machine lives in the executor and needs a real Tab, so it is
reasoned through in the commit and README rather than unit-tested; the
decision-level tests cover only which action is produced.
Baselines held: zellij-server 1441/30.
| -rw-r--r-- | README.md | 12 | ||||
| -rw-r--r-- | zellij-server/src/tab/mod.rs | 5 | ||||
| -rw-r--r-- | zellij-server/src/tab/mouse_handler.rs | 108 |
3 files changed, 71 insertions, 54 deletions
@@ -71,13 +71,13 @@ handles, so two-axis resize from the top still works. Plain top-edge resize is g which costs nothing in practice: every boundary is shared, so the boundary above a pane is the bottom edge of the pane above it and can be dragged from there. -While dragging, the pane the drop would land on is highlighted with the theme's -`frame_highlight` colour. Hovering back over the pane being dragged shows NO -highlight, because dropping onto itself does nothing - the absence of a highlight -is the signal that releasing there changes nothing. +The reordering is previewed LIVE: the swap is actually applied as you drag, so the +real layout shows what you would get, and releasing simply keeps it. Dragging away +undoes the preview and restores the original arrangement. The pane being dragged +stays highlighted so it remains identifiable as the layout moves around it. -Dropping a pane on itself is a no-op, so a plain click still just focuses, and -double-click-to-rename still works. Stacked panes are excluded — in a stack the +A drag that never leaves its own pane previews nothing, so a plain click still just +focuses and double-click-to-rename still works. Stacked panes are excluded — in a stack the title row is how you focus a member. ### Double-click a pane title to rename it diff --git a/zellij-server/src/tab/mod.rs b/zellij-server/src/tab/mod.rs index 340d3c5b..bc8337e1 100644 --- a/zellij-server/src/tab/mod.rs +++ b/zellij-server/src/tab/mod.rs @@ -267,6 +267,10 @@ pub(crate) struct Tab { frame_clicks: HashMap<ClientId, crate::panes::grid::Click>, /// Tiled pane currently being dragged by its title bar, per client. tiled_pane_being_dragged: HashMap<ClientId, PaneId>, + /// While dragging, the pane the dragged one is currently swapped WITH, so + /// the live layout acts as the preview. Undone when the drag moves to a + /// different target. + drag_preview_swapped_with: HashMap<ClientId, PaneId>, 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 @@ -1078,6 +1082,7 @@ impl Tab { mouse_click_through, frame_clicks: HashMap::new(), tiled_pane_being_dragged: HashMap::new(), + drag_preview_swapped_with: HashMap::new(), connected_clients_in_app, web_server_ip, web_server_port, diff --git a/zellij-server/src/tab/mouse_handler.rs b/zellij-server/src/tab/mouse_handler.rs index b4aef7d3..eb3ebe9e 100644 --- a/zellij-server/src/tab/mouse_handler.rs +++ b/zellij-server/src/tab/mouse_handler.rs @@ -139,9 +139,10 @@ enum MouseAction { DropDraggedTiledPane { position: Position, }, - /// while dragging -> highlight the pane the drop would land on - UpdateDragTarget { - pane_id: Option<PaneId>, + /// while dragging -> show what the reordering would look like by actually + /// performing the swap; undone if the drag moves elsewhere + PreviewDragSwap { + target: Option<PaneId>, }, GroupToggle(PaneId), GroupAdd(PaneId), @@ -847,46 +848,54 @@ impl MouseHandler { tab.tiled_pane_being_dragged.insert(client_id, pane_id); Ok(MouseEffect::state_changed()) }, - MouseAction::UpdateDragTarget { pane_id } => { - // Reuses the hover highlight so the drop target is drawn with - // the theme's frame_highlight colour. Set directly rather than - // going through execute_update_hover, which is gated on - // mouse_hover_effects: during a drag this is essential feedback - // rather than decoration, so it must not be switchable off. - let previous = tab.mouse_hover_pane_id.get(&client_id).copied(); - match pane_id { - Some(pane_id) => { - tab.mouse_hover_pane_id.insert(client_id, pane_id); - }, - None => { - tab.mouse_hover_pane_id.remove(&client_id); - }, + MouseAction::PreviewDragSwap { target } => { + let Some(dragged) = tab.tiled_pane_being_dragged.get(&client_id).copied() else { + return Ok(MouseEffect::default()); + }; + let current_preview = tab.drag_preview_swapped_with.get(&client_id).copied(); + + // Nothing to do when the cursor is over the dragged pane itself + // (wherever the preview has moved it to) or over the pane it is + // currently swapped with. Both mean "the layout on screen is + // already what dropping here would give you". Re-applying would + // undo and redo the same swap on every motion event. + let hovering_current_preview = match (target, current_preview) { + (Some(t), Some(p)) => t == p, + _ => false, + }; + if target == Some(dragged) || hovering_current_preview { + return Ok(MouseEffect::default()); } - if previous != pane_id { - Ok(MouseEffect::state_changed()) - } else { - Ok(MouseEffect::default()) + if target == current_preview { + return Ok(MouseEffect::default()); + } + + // undo whatever is currently previewed, then preview the new one + if let Some(previous) = current_preview { + tab.tiled_panes.swap_panes(dragged, previous); + tab.drag_preview_swapped_with.remove(&client_id); + } + if let Some(new_target) = target { + tab.tiled_panes.swap_panes(dragged, new_target); + tab.drag_preview_swapped_with.insert(client_id, new_target); } + // keep the pane being dragged highlighted so it stays + // identifiable as the layout moves around it + tab.mouse_hover_pane_id.insert(client_id, dragged); + Ok(MouseEffect::state_changed()) }, - MouseAction::DropDraggedTiledPane { position } => { + MouseAction::DropDraggedTiledPane { position: _ } => { + // Nothing to apply: the preview WAS the swap, so releasing just + // keeps what is already on screen. A drag that never left the + // source pane has no preview, so a plain click and a + // double-click still do nothing here. clear_hover_for_client(tab, client_id); - let dragged = tab.tiled_pane_being_dragged.remove(&client_id); - let Some(dragged_pane_id) = dragged else { - return Ok(MouseEffect::default()); - }; - let target = Self::get_pane_at(tab, &position, false) - .ok() - .flatten() - .map(|pane| pane.pid()); - match target { - // dropping a pane on itself is a no-op, which is what keeps - // click and double-click (press/release with no movement) - // working on the title bar - Some(target_id) if target_id != dragged_pane_id => { - tab.tiled_panes.swap_panes(dragged_pane_id, target_id); - Ok(MouseEffect::state_changed()) - }, - _ => Ok(MouseEffect::default()), + let was_dragging = tab.tiled_pane_being_dragged.remove(&client_id).is_some(); + let had_preview = tab.drag_preview_swapped_with.remove(&client_id).is_some(); + if was_dragging && had_preview { + Ok(MouseEffect::state_changed()) + } else { + Ok(MouseEffect::default()) } }, MouseAction::GroupAdd(pane_id) => Ok(MouseEffect::group_add(pane_id)), @@ -1571,10 +1580,10 @@ impl MouseHandler { // dragged is deliberately NOT highlighted: dropping onto // itself is a no-op, so showing no highlight is the honest // signal that releasing here changes nothing. - let target = ctx - .pane_id_at_position - .filter(|pane_id| *pane_id != dragged_pane_id); - return Ok(MouseAction::UpdateDragTarget { pane_id: target }); + let _ = dragged_pane_id; + return Ok(MouseAction::PreviewDragSwap { + target: ctx.pane_id_at_position, + }); }, _ => return Ok(MouseAction::NoAction), } @@ -2161,15 +2170,18 @@ mod tests { } #[test] - fn dragging_over_the_source_pane_shows_no_drop_target() { - // dropping onto itself is a no-op, so it must not look like a target + fn dragging_over_the_source_pane_previews_nothing_new() { + // the executor treats a target equal to the dragged pane as "the layout + // already shows what you would get", so the decision just forwards it let mut ctx = mouse_event_context(false); ctx.tiled_pane_being_dragged = Some(PaneId::Terminal(1)); ctx.pane_id_at_position = Some(PaneId::Terminal(1)); let motion = MouseEvent::new_left_motion_event(Position::new(5, 5)); assert_eq!( MouseHandler::determine_mouse_action(&motion, &ctx).unwrap(), - MouseAction::UpdateDragTarget { pane_id: None } + MouseAction::PreviewDragSwap { + target: Some(PaneId::Terminal(1)) + } ); } @@ -2222,8 +2234,8 @@ mod tests { let motion = MouseEvent::new_left_motion_event(Position::new(5, 5)); assert_eq!( MouseHandler::determine_mouse_action(&motion, &ctx).unwrap(), - MouseAction::UpdateDragTarget { - pane_id: Some(PaneId::Terminal(2)) + MouseAction::PreviewDragSwap { + target: Some(PaneId::Terminal(2)) } ); |
