From 786756faf57e9ac58725bae0eb61ead1bf964352 Mon Sep 17 00:00:00 2001 From: "Youri T. K. K. Mattar" Date: Thu, 17 Sep 2026 16:18:03 -0300 Subject: [PATCH] feat(ui): the panes are divided where you drag them, not where I guessed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three numbers decided how the main window was divided: a 230px sidebar, a 6:4 graph against the detail pane, and a 280px file list inside the working directory. Each was a guess about a window nobody had seen, and no window could argue with it — a long commit message on a laptop screen had four tenths of the height and no way to ask for more. Every divider between those panes is draggable now, the file list included. ## Two units, and the reason for each The sidebar and the file list are pixels: what they need is set by the length of a branch name or a path, and that does not grow because the window did. The graph and the detail pane are a fraction of the height they share, because they are two halves of one view — a taller window should keep the graph the subject and the commit under it the footnote. Nothing can be dragged out of existence. The pixel splits stop at a minimum and at half the space they divide; the fraction keeps 15% for each side. A pane with no height has no divider left to drag it back with. ## How a drag arrives `mouse_area` reports that a press happened, not where, so the first pointer movement after a grab is the anchor and every size is a delta from it. That is what makes `layout.rs` testable without a window: nothing in it knows where anything is on screen. The pointer subscription exists only while a divider is held — following every mouse movement for a session to serve a drag nobody is doing is the most expensive subscription this could have had. `extent` — the size of the space being divided — is the one thing the arithmetic cannot derive, so the view measures it with `responsive` and the grab carries it. ## Where it is written `state.toml`, beside the window's own geometry, on release rather than per frame. It is state, not a setting: it changes by dragging, and nobody is going to open a file to choose a number of pixels. It is clamped again on the way in, so a hand-edited file cannot open hideGit with a sidebar wider than the window. A state file written before this exists loads unchanged. Double-clicking a divider restores it, which is why none of this needs a settings entry: the control that moved it is the control that undoes it. ## What it cost `staging::view` reached fourteen arguments — already flagged in #86 as wanting a struct, and still not this change. Its file list became its own function, because the row it sits in is built inside a `responsive` closure that may run more than once and therefore cannot consume anything. ## Tests Thirteen on the arithmetic, three on the divider widget — including that a press beside it grabs nothing — and five driving `update`, one of which holds that a divider dragged inside the working directory resizes the window's layout rather than the repository's. Two on the config file: a round trip, and a `state.toml` from before the dividers existed. --- crates/hidegit-ui/src/app.rs | 164 ++++++++- crates/hidegit-ui/src/layout.rs | 402 +++++++++++++++++++++ crates/hidegit-ui/src/lib.rs | 2 + crates/hidegit-ui/src/message.rs | 34 ++ crates/hidegit-ui/src/metrics.rs | 3 +- crates/hidegit-ui/src/screen/repository.rs | 45 ++- crates/hidegit-ui/src/state.rs | 7 + crates/hidegit-ui/src/widget/detail.rs | 2 + crates/hidegit-ui/src/widget/sidebar.rs | 4 +- crates/hidegit-ui/src/widget/split.rs | 174 +++++++++ crates/hidegit-ui/src/widget/staging.rs | 94 +++-- crates/hidegit/src/config.rs | 75 ++++ crates/hidegit/src/main.rs | 22 +- docs/ROADMAP.md | 7 + docs/UI_SPEC.md | 25 +- 15 files changed, 1011 insertions(+), 49 deletions(-) create mode 100644 crates/hidegit-ui/src/layout.rs create mode 100644 crates/hidegit-ui/src/widget/split.rs diff --git a/crates/hidegit-ui/src/app.rs b/crates/hidegit-ui/src/app.rs index 86674b3..cff771a 100644 --- a/crates/hidegit-ui/src/app.rs +++ b/crates/hidegit-ui/src/app.rs @@ -28,8 +28,8 @@ use iced::{Element as IcedElement, Subscription, Task, keyboard}; use hidegit_forge::{NewPullRequest, WebTarget}; use crate::message::{ - CommitLoad, Message, OpenedRepository, OperationOutcome, Page, PrsLoad, Refreshed, RepoMessage, - StatusLoad, UiError, + CommitLoad, LayoutMessage, Message, OpenedRepository, OperationOutcome, Page, PrsLoad, + Refreshed, RepoMessage, StatusLoad, UiError, }; use crate::state::{ ActionSheet, App, Confirmation, DetailPane, Draft, GraphView, OpenRepo, Operation, PAGE_SIZE, @@ -494,6 +494,8 @@ impl Hidegit { Task::none() } + Message::Layout(message) => self.update_layout(message), + Message::RememberGeometryToggled => { // Nothing in this crate acts on it; the shell reads it back out // and decides what to write. Kept here because the panel that @@ -1350,13 +1352,43 @@ impl Hidegit { Task::batch(indices.into_iter().map(|index| self.poll_task(index))) } + /// A drag on a pane divider, wherever it was raised from. + /// + /// The arithmetic is [`crate::layout::Layout`]'s; what happens here is the + /// mapping from four messages onto four of its methods. Nothing is loaded, + /// nothing is written, and nothing is redrawn that iced would not redraw + /// anyway — a divider drag is the cheapest message in the application, and + /// during a drag it arrives on every frame. + fn update_layout(&mut self, message: LayoutMessage) -> Task { + match message { + LayoutMessage::Grabbed(split, extent) => self.app.layout.grab(split, extent), + LayoutMessage::Dragged(x, y) => self.app.layout.drag_to(x, y), + LayoutMessage::Released => self.app.layout.release(), + LayoutMessage::Reset(split) => self.app.layout.reset(split), + } + + Task::none() + } + fn update_repo(&mut self, index: usize, message: RepoMessage) -> Task { + // Answered before the repository is even looked up: a divider inside + // the working directory addresses the window's layout, not this + // repository, and it only arrives here because the view it was dragged + // in speaks `RepoMessage`. + if let RepoMessage::Layout(message) = message { + return self.update_layout(message); + } + let Some(repo) = self.app.repos.get_mut(index) else { return Task::none(); }; let cache = self.caches.entry(index).or_default(); match message { + // Answered above, before `repo` was borrowed. The arm is here + // because the match has to be exhaustive, not because it can run. + RepoMessage::Layout(_) => Task::none(), + RepoMessage::Selected(selection) => { repo.selection = Some(selection.clone()); // A filter written against the last commit's files would hide @@ -3569,7 +3601,36 @@ impl Hidegit { _ => None, }); - Subscription::batch([keys, focus].into_iter().chain(watches).chain(polls)) + // Where the pointer is, but only while a divider is being dragged. + // + // A press on a divider says that it happened, not where: `mouse_area` + // reports the press and the window reports the position, and this is + // what joins them. It is switched off the rest of the time on purpose — + // a message per pointer movement for the whole session, to serve a drag + // nobody is doing, is the most expensive subscription in the + // application and the easiest one to avoid. + let pointer = if self.app.layout.is_dragging() { + iced::event::listen_with(|event, _, _| match event { + iced::Event::Mouse(iced::mouse::Event::CursorMoved { position }) => Some( + Message::Layout(LayoutMessage::Dragged(position.x, position.y)), + ), + // Released anywhere, including outside the window: a drag that + // only ended when the pointer came back would keep following it. + iced::Event::Mouse(iced::mouse::Event::ButtonReleased( + iced::mouse::Button::Left, + )) => Some(Message::Layout(LayoutMessage::Released)), + _ => None, + }) + } else { + Subscription::none() + }; + + Subscription::batch( + [keys, focus, pointer] + .into_iter() + .chain(watches) + .chain(polls), + ) } /// Which modal, if any, currently owns the keyboard. @@ -4721,6 +4782,103 @@ mod tests { .collect() } + // ---- dividers ---- + + /// Presses a divider, moves the pointer twice — the first movement is the + /// anchor — and lets go, the way the window delivers a drag. + fn drag_divider(app: &mut Hidegit, split: crate::layout::Split, extent: f32, by: (f32, f32)) { + let _ = app.update(Message::Layout(LayoutMessage::Grabbed(split, extent))); + let _ = app.update(Message::Layout(LayoutMessage::Dragged(500.0, 500.0))); + let _ = app.update(Message::Layout(LayoutMessage::Dragged( + 500.0 + by.0, + 500.0 + by.1, + ))); + let _ = app.update(Message::Layout(LayoutMessage::Released)); + } + + #[test] + fn dragging_the_sidebar_divider_resizes_the_sidebar() { + let mut app = app_with(3); + + drag_divider(&mut app, crate::layout::Split::Sidebar, 1440.0, (60.0, 0.0)); + + assert_eq!( + app.app.layout.sidebar(), + crate::layout::Layout::SIDEBAR + 60.0 + ); + } + + #[test] + fn a_divider_dragged_inside_the_working_directory_resizes_the_same_layout() { + // It arrives as a `RepoMessage` only because the staging view speaks + // one. If this ever stopped reaching the application's layout, the + // file list would resize and forget by the next render. + let mut app = app_with(3); + + let _ = app.update(Message::Repo( + 0, + RepoMessage::Layout(LayoutMessage::Grabbed(crate::layout::Split::Files, 1000.0)), + )); + let _ = app.update(Message::Repo( + 0, + RepoMessage::Layout(LayoutMessage::Dragged(400.0, 300.0)), + )); + let _ = app.update(Message::Repo( + 0, + RepoMessage::Layout(LayoutMessage::Dragged(440.0, 300.0)), + )); + + assert_eq!(app.app.layout.files(), crate::layout::Layout::FILES + 40.0); + } + + #[test] + fn a_divider_addresses_the_window_rather_than_the_repository_it_was_dragged_in() { + // Two tabs, a drag in the second: the sidebar is the window's, so + // switching back must not switch the width back with it. + let mut app = app_with_tabs(2); + + drag_divider(&mut app, crate::layout::Split::Sidebar, 1440.0, (40.0, 0.0)); + let dragged = app.app.layout.sidebar(); + let _ = app.update(Message::OpenRepository(PathBuf::from("/fake/repo-0"))); + + assert_eq!(app.app.layout.sidebar(), dragged); + } + + #[test] + fn the_pointer_is_only_followed_while_a_divider_is_held() { + // What decides whether the subscription that reports every pointer + // movement exists at all. + let mut app = app_with(3); + assert!(!app.app.layout.is_dragging()); + + let _ = app.update(Message::Layout(LayoutMessage::Grabbed( + crate::layout::Split::Detail, + 900.0, + ))); + assert!(app.app.layout.is_dragging()); + + let _ = app.update(Message::Layout(LayoutMessage::Released)); + assert!(!app.app.layout.is_dragging()); + } + + #[test] + fn double_clicking_a_divider_puts_it_back() { + let mut app = app_with(3); + drag_divider( + &mut app, + crate::layout::Split::Sidebar, + 1440.0, + (120.0, 0.0), + ); + assert_ne!(app.app.layout.sidebar(), crate::layout::Layout::SIDEBAR); + + let _ = app.update(Message::Layout(LayoutMessage::Reset( + crate::layout::Split::Sidebar, + ))); + + assert_eq!(app.app.layout.sidebar(), crate::layout::Layout::SIDEBAR); + } + #[test] fn dragging_a_row_across_the_list_moves_only_that_row() { // Remove-and-insert, not a swap. Swapping the ends would send the first diff --git a/crates/hidegit-ui/src/layout.rs b/crates/hidegit-ui/src/layout.rs new file mode 100644 index 0000000..91038dd --- /dev/null +++ b/crates/hidegit-ui/src/layout.rs @@ -0,0 +1,402 @@ +//! How the window divides itself, and what dragging a divider does to it. +//! +//! Until this module existed every division in the main window was a number +//! written into a `view`: the sidebar was `Fixed(230.0)`, the graph and the +//! detail pane below it were `FillPortion(6)` and `FillPortion(4)`, and the +//! file list inside the working directory was `Fixed(280.0)`. Each of those is +//! a reasonable guess about a window nobody had seen yet, and a guess is all it +//! can be — a 13" laptop reading a wide diff and a 32" monitor reading a branch +//! list want opposite things, and neither could ask for them. +//! +//! # Two units, on purpose +//! +//! The sidebar and the file list are held in **pixels**. What they need is set +//! by their contents — a branch name, a path — and that does not get longer +//! because the window did. A sidebar that grew with the window would take space +//! from the diff to show the same names in the same font. +//! +//! The graph and the detail pane are held as a **fraction** of the space they +//! share, because they are two halves of one view rather than a panel beside a +//! document: at 60/40 the graph is the subject and the commit under it is the +//! footnote, and that relationship is what a taller window should preserve. +//! +//! # Why a drag arrives as a delta +//! +//! A press on a divider says nothing about where the pointer is — `mouse_area` +//! reports that a press happened, not its position — so the first movement +//! after the grab is the anchor, and every size is computed from the pointer's +//! distance from it rather than from its absolute position. That is also what +//! makes the arithmetic here testable without a window: nothing in this module +//! knows where anything is on screen. +//! +//! `extent` is the one measurement the view has to supply, because the size of +//! the space being divided cannot be derived from a pointer delta. It is taken +//! at the moment of the grab, from the `responsive` wrapper in +//! [`screen::repository`](crate::screen::repository), and it is what turns a +//! pixel delta into a fraction and what bounds a pixel width. + +/// A divider that can be dragged, named for the pane that moves when it is. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum Split { + /// Between the sidebar and everything to its right. + Sidebar, + /// Between the graph and the detail pane under it. + Detail, + /// Between the file list and the diff, inside the working directory. + Files, +} + +/// A drag in progress. +#[derive(Debug, Clone, Copy, PartialEq)] +struct Drag { + split: Split, + /// The size of the space being divided, measured when the grab happened. + extent: f32, + /// What the split was when it was grabbed. Every position during the drag + /// is computed from this rather than from the previous position, so a drag + /// that leaves and re-enters the window does not accumulate error. + base: f32, + /// Where the pointer was when it first moved after the grab. + anchor: Option<(f32, f32)>, +} + +/// How the window's panes are divided, and the drag that is changing it. +#[derive(Debug, Clone, Copy, PartialEq)] +pub struct Layout { + sidebar: f32, + detail: f32, + files: f32, + drag: Option, +} + +impl Default for Layout { + fn default() -> Self { + Self { + sidebar: Self::SIDEBAR, + detail: Self::DETAIL, + files: Self::FILES, + drag: None, + } + } +} + +impl Layout { + /// The sidebar's width before anyone drags it. + pub const SIDEBAR: f32 = 230.0; + /// The detail pane's share of the height it splits with the graph. + pub const DETAIL: f32 = 0.4; + /// The file list's width inside the working directory. + pub const FILES: f32 = 280.0; + + /// Narrow enough to be worth doing, wide enough to still read a branch name. + const SIDEBAR_BOUNDS: (f32, f32) = (160.0, 520.0); + const FILES_BOUNDS: (f32, f32) = (180.0, 620.0); + /// Neither the graph nor the detail pane may be dragged out of existence: a + /// pane with no height is indistinguishable from a bug, and there is no + /// divider left to drag it back with. + const DETAIL_BOUNDS: (f32, f32) = (0.15, 0.85); + + /// The layout a config file asks for, made safe to use. + /// + /// A hand-edited file — or one written by a version that bounded these + /// differently — must not be able to produce a sidebar wider than the + /// window or a detail pane of zero height, so every value goes through the + /// same clamp a drag does. + pub fn restore(sidebar: f32, detail: f32, files: f32) -> Self { + Self { + sidebar: clamp(sidebar, Self::SIDEBAR_BOUNDS, Self::SIDEBAR), + detail: clamp(detail, Self::DETAIL_BOUNDS, Self::DETAIL), + files: clamp(files, Self::FILES_BOUNDS, Self::FILES), + drag: None, + } + } + + /// The sidebar's width, in pixels. + pub fn sidebar(&self) -> f32 { + self.sidebar + } + + /// The detail pane's share of the column it splits with the graph, as a + /// fraction between zero and one. + pub fn detail(&self) -> f32 { + self.detail + } + + /// The file list's width, in pixels. + pub fn files(&self) -> f32 { + self.files + } + + /// Is this divider the one being dragged right now? + /// + /// Only so the divider can say so: a hairline that does not change under + /// the pointer gives no sign that the drag was picked up at all. + pub fn dragging(&self, split: Split) -> bool { + self.drag.is_some_and(|drag| drag.split == split) + } + + /// Is any divider being dragged? + /// + /// The pointer subscription is only alive while this is true: listening to + /// every mouse movement in the session to serve a drag that is not + /// happening is a message per frame for nothing. + pub fn is_dragging(&self) -> bool { + self.drag.is_some() + } + + /// A divider was pressed. `extent` is the size of the space it divides. + pub fn grab(&mut self, split: Split, extent: f32) { + let base = match split { + Split::Sidebar => self.sidebar, + Split::Detail => self.detail, + Split::Files => self.files, + }; + + self.drag = Some(Drag { + split, + extent: if extent.is_finite() && extent > 0.0 { + extent + } else { + f32::INFINITY + }, + base, + anchor: None, + }); + } + + /// The pointer moved, in window coordinates. + pub fn drag_to(&mut self, x: f32, y: f32) { + if !x.is_finite() || !y.is_finite() { + return; + } + + let Some(drag) = &mut self.drag else { + return; + }; + + let Some((from_x, from_y)) = drag.anchor else { + drag.anchor = Some((x, y)); + return; + }; + + let drag = *drag; + match drag.split { + Split::Sidebar => { + self.sidebar = clamp( + drag.base + (x - from_x), + bounded_by(Self::SIDEBAR_BOUNDS, drag.extent), + Self::SIDEBAR, + ); + } + Split::Files => { + self.files = clamp( + drag.base + (x - from_x), + bounded_by(Self::FILES_BOUNDS, drag.extent), + Self::FILES, + ); + } + // The detail pane is *below* the divider, so dragging downwards + // makes it smaller. Getting this sign wrong produces a pane that + // runs away from the pointer, which is why it has its own test. + Split::Detail => { + self.detail = clamp( + drag.base - (y - from_y) / drag.extent, + Self::DETAIL_BOUNDS, + Self::DETAIL, + ); + } + } + } + + /// The pointer was released, wherever it was released. + pub fn release(&mut self) { + self.drag = None; + } + + /// Double-clicking a divider puts it back where it shipped. + /// + /// The way out of a layout somebody has dragged into uselessness, and the + /// reason this needs no settings entry: the control that broke it is the + /// control that fixes it. + pub fn reset(&mut self, split: Split) { + match split { + Split::Sidebar => self.sidebar = Self::SIDEBAR, + Split::Detail => self.detail = Self::DETAIL, + Split::Files => self.files = Self::FILES, + } + // A double click is a press as well, so a grab is in flight by the time + // this arrives. Leaving it would let the next pointer movement drag the + // divider away from the default it was just put back to. + self.drag = None; + } +} + +/// The bounds a pixel split gets inside a space of `extent` pixels. +/// +/// Half the space, at most: a sidebar that can eat the window is a sidebar +/// somebody has to drag back before they can read anything, and on a narrow +/// window the fixed maximum is more than the whole width. +fn bounded_by((low, high): (f32, f32), extent: f32) -> (f32, f32) { + (low, high.min(extent / 2.0).max(low)) +} + +/// Clamps, and refuses `NaN`. +/// +/// `f32::clamp` panics on a `NaN` bound and propagates a `NaN` value, and a +/// size of `NaN` reaches the layout engine as a pane that never draws. +fn clamp(value: f32, (low, high): (f32, f32), fallback: f32) -> f32 { + if value.is_finite() { + value.clamp(low, high) + } else { + fallback + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Grabs a divider and moves the pointer, which takes two movements: the + /// first is the anchor, the second is the drag. + fn dragged(split: Split, extent: f32, from: (f32, f32), to: (f32, f32)) -> Layout { + let mut layout = Layout::default(); + layout.grab(split, extent); + layout.drag_to(from.0, from.1); + layout.drag_to(to.0, to.1); + layout + } + + #[test] + fn dragging_the_sidebar_right_widens_it_by_what_the_pointer_travelled() { + let layout = dragged(Split::Sidebar, 1440.0, (230.0, 400.0), (310.0, 400.0)); + + assert_eq!(layout.sidebar(), Layout::SIDEBAR + 80.0); + } + + #[test] + fn the_first_movement_is_the_anchor_and_moves_nothing() { + // Otherwise the divider would jump to wherever the pointer happened to + // be when the press was reported, which is not where it was pressed. + let mut layout = Layout::default(); + layout.grab(Split::Sidebar, 1440.0); + layout.drag_to(900.0, 400.0); + + assert_eq!(layout.sidebar(), Layout::SIDEBAR); + } + + #[test] + fn the_sidebar_cannot_be_dragged_away_or_over_the_window() { + let shrunk = dragged(Split::Sidebar, 1440.0, (230.0, 400.0), (-900.0, 400.0)); + assert_eq!(shrunk.sidebar(), Layout::SIDEBAR_BOUNDS.0); + + let grown = dragged(Split::Sidebar, 1440.0, (230.0, 400.0), (9000.0, 400.0)); + assert_eq!(grown.sidebar(), Layout::SIDEBAR_BOUNDS.1); + } + + #[test] + fn a_narrow_window_bounds_the_sidebar_before_the_fixed_maximum_does() { + // 520 of a 700-pixel window leaves 180 for the graph, the diff and the + // commit message together. + let layout = dragged(Split::Sidebar, 700.0, (230.0, 400.0), (9000.0, 400.0)); + + assert_eq!(layout.sidebar(), 350.0, "half the window, not the maximum"); + } + + #[test] + fn dragging_the_detail_divider_down_makes_the_detail_pane_smaller() { + // 100 pixels down a 1000-pixel column is a tenth of it. + let layout = dragged(Split::Detail, 1000.0, (700.0, 500.0), (700.0, 600.0)); + + assert!( + (layout.detail() - (Layout::DETAIL - 0.1)).abs() < f32::EPSILON, + "the pane below the divider shrinks as the divider goes down, got {}", + layout.detail() + ); + } + + #[test] + fn neither_pane_can_be_dragged_out_of_existence() { + let flattened = dragged(Split::Detail, 1000.0, (700.0, 500.0), (700.0, 5000.0)); + assert_eq!(flattened.detail(), Layout::DETAIL_BOUNDS.0); + + let raised = dragged(Split::Detail, 1000.0, (700.0, 500.0), (700.0, -5000.0)); + assert_eq!(raised.detail(), Layout::DETAIL_BOUNDS.1); + } + + #[test] + fn a_release_ends_the_drag_and_later_movement_does_nothing() { + let mut layout = Layout::default(); + layout.grab(Split::Sidebar, 1440.0); + layout.drag_to(230.0, 400.0); + layout.drag_to(300.0, 400.0); + layout.release(); + layout.drag_to(600.0, 400.0); + + assert_eq!(layout.sidebar(), Layout::SIDEBAR + 70.0); + assert!(!layout.is_dragging()); + } + + #[test] + fn every_drag_is_measured_from_where_it_started() { + // Not from the previous position: a delta applied to the current size + // every time drifts, because the size is clamped and the pointer is not. + let mut layout = Layout::default(); + layout.grab(Split::Sidebar, 1440.0); + layout.drag_to(230.0, 400.0); + layout.drag_to(-900.0, 400.0); + layout.drag_to(280.0, 400.0); + + assert_eq!( + layout.sidebar(), + Layout::SIDEBAR + 50.0, + "the excursion into the clamp did not move the origin" + ); + } + + #[test] + fn resetting_puts_a_divider_back_and_drops_the_drag_that_did_it() { + let mut layout = dragged(Split::Files, 1200.0, (280.0, 400.0), (500.0, 400.0)); + assert_ne!(layout.files(), Layout::FILES); + + layout.grab(Split::Files, 1200.0); + layout.reset(Split::Files); + + assert_eq!(layout.files(), Layout::FILES); + assert!( + !layout.is_dragging(), + "a double click is a press too, and the drag it armed would undo the reset" + ); + } + + #[test] + fn only_the_divider_being_dragged_says_it_is() { + let mut layout = Layout::default(); + layout.grab(Split::Detail, 1000.0); + + assert!(layout.dragging(Split::Detail)); + assert!(!layout.dragging(Split::Sidebar)); + } + + #[test] + fn a_pointer_that_reports_nonsense_moves_nothing() { + let layout = dragged(Split::Sidebar, 1440.0, (230.0, 400.0), (f32::NAN, 400.0)); + + assert_eq!(layout.sidebar(), Layout::SIDEBAR); + } + + #[test] + fn a_config_file_cannot_ask_for_a_pane_that_does_not_fit() { + let restored = Layout::restore(9000.0, 2.0, -5.0); + + assert_eq!(restored.sidebar(), Layout::SIDEBAR_BOUNDS.1); + assert_eq!(restored.detail(), Layout::DETAIL_BOUNDS.1); + assert_eq!(restored.files(), Layout::FILES_BOUNDS.0); + } + + #[test] + fn a_config_file_full_of_nonsense_falls_back_to_the_defaults() { + let restored = Layout::restore(f32::NAN, f32::INFINITY, f32::NEG_INFINITY); + + assert_eq!(restored, Layout::default()); + } +} diff --git a/crates/hidegit-ui/src/lib.rs b/crates/hidegit-ui/src/lib.rs index 4b0a27f..76f2585 100644 --- a/crates/hidegit-ui/src/lib.rs +++ b/crates/hidegit-ui/src/lib.rs @@ -12,6 +12,7 @@ pub mod format; pub mod highlight; pub mod i18n; pub mod keymap; +pub mod layout; pub mod message; pub mod metrics; pub mod state; @@ -38,6 +39,7 @@ pub mod widget { pub mod settings; pub mod shortcuts; pub mod sidebar; + pub mod split; pub mod staging; pub mod tabs; } diff --git a/crates/hidegit-ui/src/message.rs b/crates/hidegit-ui/src/message.rs index 3b96c36..c378205 100644 --- a/crates/hidegit-ui/src/message.rs +++ b/crates/hidegit-ui/src/message.rs @@ -224,6 +224,25 @@ pub struct Page { pub more: bool, } +/// What a drag on a pane divider does, from press to release. +/// +/// Its own enum rather than four variants on [`Message`], because the same four +/// are needed on [`RepoMessage`] too and writing them twice is how the two +/// copies end up meaning different things. +#[derive(Debug, Clone, Copy, PartialEq)] +pub enum LayoutMessage { + /// A divider was pressed, carrying the size of the space it divides — which + /// only the view that laid it out can know. + Grabbed(crate::layout::Split, f32), + /// The pointer moved, in window coordinates. Delivered by a subscription + /// that is only alive while a drag is in progress. + Dragged(f32, f32), + /// The pointer was released, wherever it happened to be. + Released, + /// A divider was double-clicked: put it back where it shipped. + Reset(crate::layout::Split), +} + #[derive(Debug, Clone)] pub enum Message { /// `Cmd+O`: ask for a repository with the platform's own picker. @@ -327,6 +346,15 @@ pub enum Message { RememberGeometryToggled, RepositoryMuteToggled(String), + /// A divider between two panes was grabbed, dragged or released. + /// + /// Reached from two places, because the dividers are in two places: the + /// main window emits this, and the one inside the working directory arrives + /// as [`RepoMessage::Layout`] and is forwarded here. The layout belongs to + /// the application rather than to a repository — the sidebar is the same + /// width whichever tab is in front — so both end up in the same handler. + Layout(LayoutMessage), + // ---- the forge ---- /// The client exists, and a stored session was restored if there was one. /// @@ -371,6 +399,12 @@ pub enum Message { #[derive(Debug, Clone)] pub enum RepoMessage { // ---- user intent ---- + /// A divider inside the working directory was dragged. + /// + /// Forwarded to [`Message::Layout`] unchanged. The staging view speaks + /// `RepoMessage` all the way down, so a divider placed in it cannot emit a + /// top-level message without every view above it changing its type. + Layout(LayoutMessage), Selected(Selection), /// A wheel or trackpad scroll, in pixels. Positive scrolls toward older /// history. diff --git a/crates/hidegit-ui/src/metrics.rs b/crates/hidegit-ui/src/metrics.rs index ba6608e..386094a 100644 --- a/crates/hidegit-ui/src/metrics.rs +++ b/crates/hidegit-ui/src/metrics.rs @@ -126,7 +126,7 @@ pub mod text { /// pretending otherwise would mean converting everything about a file at once /// or nothing at all. #[cfg(test)] -const ON_THE_LAYOUT_SCALE: &[&str] = &["widget/tabs.rs"]; +const ON_THE_LAYOUT_SCALE: &[&str] = &["widget/split.rs", "widget/tabs.rs"]; /// Files whose every type size comes from [`text`]. #[cfg(test)] @@ -139,6 +139,7 @@ const ON_THE_TYPE_SCALE: &[&str] = &[ "widget/resolver.rs", "widget/search.rs", "widget/sidebar.rs", + "widget/split.rs", "widget/staging.rs", "widget/tabs.rs", ]; diff --git a/crates/hidegit-ui/src/screen/repository.rs b/crates/hidegit-ui/src/screen/repository.rs index 570cff7..d0330ad 100644 --- a/crates/hidegit-ui/src/screen/repository.rs +++ b/crates/hidegit-ui/src/screen/repository.rs @@ -6,12 +6,13 @@ use iced::widget::{Space, button, column, container, responsive, row, text, tool use iced::{Center, Fill, Font, Length, Padding}; use crate::Element; +use crate::layout::Split; use crate::message::{Message, RepoMessage}; use crate::metrics; use crate::state::{App, OpenRepo, Pane, ROW_HEIGHT}; use crate::theme::Palette; use crate::widget::common; -use crate::widget::{detail, graph, sidebar}; +use crate::widget::{detail, graph, sidebar, split}; /// The main window. /// @@ -49,7 +50,7 @@ pub fn view<'a>( // Which pane the keyboard is pointed at, said out loud. `Tab` cycles all // three; before this only the graph showed it, through the tint on its // selected row, so two of the three positions looked identical. - let ring = |pane: Pane, element: Element<'a, Message>| -> Element<'a, Message> { + let ring = move |pane: Pane, element: Element<'a, Message>| -> Element<'a, Message> { let border = crate::theme::focus_ring(repo.focus == pane, palette); container(element) .style(move |_| container::Style { @@ -59,10 +60,27 @@ pub fn view<'a>( .into() }; - stack = stack.push( + // `responsive` because both dividers need a measurement no `view` is + // otherwise given: the height of the column decides what a pixel of + // pointer travel is worth as a fraction, and the width of the window + // decides how wide the sidebar is allowed to get. The layout pass knows + // both; nothing else does. + stack = stack.push(responsive(move |size| { + let layout = &app.layout; + // Portions rather than a fixed height, so the split survives a window + // resize as the proportion it was dragged to. A thousandth is finer + // than a pixel on any monitor this will run on. + let detail = (layout.detail() * 1000.0) as u16; + row![ ring(Pane::Sidebar, sidebar::view(app, repo, index, palette)), - common::vertical_rule(palette).map(repo_message), + split::vertical( + Split::Sidebar, + size.width, + layout.dragging(Split::Sidebar), + palette + ) + .map(Message::Layout), column![ // The portion goes on this container rather than inside // `graph_pane`, because `ring` wraps its child in a container @@ -75,21 +93,28 @@ pub fn view<'a>( Pane::Graph, graph_pane(repo, palette, cache).map(repo_message) )) - .height(Length::FillPortion(6)) + .height(Length::FillPortion(1000 - detail)) .width(Fill), - common::divider(palette).map(repo_message), + split::horizontal( + Split::Detail, + size.height, + layout.dragging(Split::Detail), + palette + ) + .map(Message::Layout), container(ring( Pane::Detail, - detail::view(repo, palette, &app.text).map(repo_message) + detail::view(repo, palette, &app.text, layout).map(repo_message) )) - .height(Length::FillPortion(4)) + .height(Length::FillPortion(detail)) .width(Fill), ] .height(Fill) .width(Fill), ] - .height(Fill), - ); + .height(Fill) + .into() + })); stack.into() } diff --git a/crates/hidegit-ui/src/state.rs b/crates/hidegit-ui/src/state.rs index a76a3f8..8b450bf 100644 --- a/crates/hidegit-ui/src/state.rs +++ b/crates/hidegit-ui/src/state.rs @@ -1332,6 +1332,12 @@ pub struct App { pub focused: bool, /// Which alerts to send, and when not to. pub alerts: AlertPrefs, + /// How the window's panes are divided, and the drag that is changing it. + /// + /// On `App` rather than on `OpenRepo` because it is a property of the + /// window: dragging the sidebar wider with one repository in front and + /// finding it narrow again in the next tab would read as a bug. + pub layout: crate::layout::Layout, /// Write a report to disk when hideGit panics. /// /// Held here for the same reason as the geometry switch: the panel shows it, @@ -1377,6 +1383,7 @@ impl Default for App { // than on demand. focused: true, alerts: AlertPrefs::default(), + layout: crate::layout::Layout::default(), remember_geometry: true, panic_reports: false, check_for_updates: true, diff --git a/crates/hidegit-ui/src/widget/detail.rs b/crates/hidegit-ui/src/widget/detail.rs index 11d5a32..37c0ce3 100644 --- a/crates/hidegit-ui/src/widget/detail.rs +++ b/crates/hidegit-ui/src/widget/detail.rs @@ -16,6 +16,7 @@ pub fn view<'a>( repo: &'a OpenRepo, palette: &'a Palette, text_catalogue: &'a crate::i18n::Catalogue, + layout: &'a crate::layout::Layout, ) -> Element<'a, RepoMessage> { let body: Element<'a, RepoMessage> = match &repo.detail { DetailPane::Empty => common::empty("Select a commit to read its message and diff", palette), @@ -60,6 +61,7 @@ pub fn view<'a>( palette, text_catalogue, repo.head.target(), + layout, ), // The one pane whose contents did not come from the repository. DetailPane::PullRequest(detail) => crate::widget::pr::detail(detail, palette), diff --git a/crates/hidegit-ui/src/widget/sidebar.rs b/crates/hidegit-ui/src/widget/sidebar.rs index 238b866..197aaf6 100644 --- a/crates/hidegit-ui/src/widget/sidebar.rs +++ b/crates/hidegit-ui/src/widget/sidebar.rs @@ -156,7 +156,9 @@ pub fn view<'a>( let palette = *palette; container(scrollable(sections).height(Fill)) - .width(Length::Fixed(230.0)) + // Whatever the divider beside it was last dragged to, rather than the + // 230 pixels this was born with. + .width(Length::Fixed(app.layout.sidebar())) .height(Fill) .style(move |_| container::Style { background: Some(palette.surface.into()), diff --git a/crates/hidegit-ui/src/widget/split.rs b/crates/hidegit-ui/src/widget/split.rs new file mode 100644 index 0000000..563356b --- /dev/null +++ b/crates/hidegit-ui/src/widget/split.rs @@ -0,0 +1,174 @@ +//! The dividers you can drag. +//! +//! A hairline is one pixel wide, and one pixel is not something a pointer can +//! be expected to hit. So a draggable divider is a strip +//! [`GRAB`](self::GRAB) pixels across with the hairline drawn down the middle +//! of it: what you see is the same rule [`common::divider`](crate::widget::common::divider) +//! draws everywhere else, and what you can grab is the part either side of it. +//! +//! It emits [`LayoutMessage`], and nothing else — the screen that places it +//! decides how that reaches `update`. That is what lets the same divider sit in +//! the main window, which speaks `Message`, and inside the working directory, +//! which speaks `RepoMessage`: both map it. + +use iced::widget::{Space, container, mouse_area}; +use iced::{Fill, Length, mouse}; + +use crate::Element; +use crate::layout::Split; +use crate::message::LayoutMessage; +use crate::metrics; +use crate::theme::Palette; + +/// How much of the divider is pointer target rather than hairline. +/// +/// Seven pixels: three either side of the rule. Small enough that the panes +/// keep the space they had to within a few pixels, large enough to hit without +/// aiming — the macOS window-resize border is about this wide, and it is the +/// closest thing to a platform answer. +pub const GRAB: f32 = 7.0; + +/// A divider between two columns: drag it left and right. +pub fn vertical<'a>( + split: Split, + extent: f32, + dragging: bool, + palette: &Palette, +) -> Element<'a, LayoutMessage> { + handle( + split, + extent, + dragging, + palette, + (Length::Fixed(GRAB), Fill), + (Length::Fixed(metrics::HAIR), Fill), + mouse::Interaction::ResizingColumn, + ) +} + +/// A divider between two rows: drag it up and down. +pub fn horizontal<'a>( + split: Split, + extent: f32, + dragging: bool, + palette: &Palette, +) -> Element<'a, LayoutMessage> { + handle( + split, + extent, + dragging, + palette, + (Fill, Length::Fixed(GRAB)), + (Fill, Length::Fixed(metrics::HAIR)), + mouse::Interaction::ResizingRow, + ) +} + +#[allow(clippy::too_many_arguments)] +fn handle<'a>( + split: Split, + extent: f32, + dragging: bool, + palette: &Palette, + (width, height): (Length, Length), + (rule_width, rule_height): (Length, Length), + interaction: mouse::Interaction, +) -> Element<'a, LayoutMessage> { + // The rule brightens while it is being dragged, and only then. Without it + // there is no way to tell a drag that was picked up from a press that + // missed — the panes either side move, but not until the pointer does. + let colour = if dragging { + palette.accent + } else { + palette.border + }; + + let rule = container(Space::new()) + .width(rule_width) + .height(rule_height) + .style(move |_| container::Style { + background: Some(colour.into()), + ..container::Style::default() + }); + + mouse_area( + container(rule) + .width(width) + .height(height) + .center_x(width) + .center_y(height), + ) + .interaction(interaction) + .on_press(LayoutMessage::Grabbed(split, extent)) + // The pointer usually leaves the divider during a drag, so the release that + // ends one arrives through the subscription rather than here. This is for + // the press that never became a drag at all. + .on_release(LayoutMessage::Released) + .on_double_click(LayoutMessage::Reset(split)) + .into() +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::theme::Theme; + + /// A divider carries no text, so it cannot be found the way the rest of the + /// render tests find things. What can be asserted is what pressing it says, + /// which is the whole of its job. + fn pressed(element: Element<'_, LayoutMessage>, at: (f32, f32)) -> Vec { + let mut ui = iced_test::simulator(element); + ui.point_at(at); + let _ = ui.simulate(iced_test::simulator::click()); + + ui.into_messages().collect() + } + + #[test] + fn pressing_a_divider_grabs_it_and_carries_the_space_it_divides() { + let palette = Theme::default().palette; + + // Three pixels in: the hairline is one pixel of a seven-pixel strip, so + // this is a press that hits the target and misses the rule. + let messages = pressed( + vertical(Split::Sidebar, 1440.0, false, &palette), + (3.0, 100.0), + ); + + assert_eq!( + messages.first(), + Some(&LayoutMessage::Grabbed(Split::Sidebar, 1440.0)), + "the extent is measured by the view, so the grab has to carry it" + ); + } + + #[test] + fn a_press_beside_the_divider_grabs_nothing() { + let palette = Theme::default().palette; + + // Deliberately outside the strip: the grab area is generous, not + // unbounded, and a divider that swallowed presses meant for the pane + // beside it would be worse than a thin one. + let messages = pressed( + vertical(Split::Sidebar, 1440.0, false, &palette), + (40.0, 100.0), + ); + + assert!(messages.is_empty(), "got {messages:?}"); + } + + #[test] + fn a_horizontal_divider_is_grabbed_across_its_thickness_rather_than_its_length() { + let palette = Theme::default().palette; + + let messages = pressed( + horizontal(Split::Detail, 900.0, true, &palette), + (100.0, 3.0), + ); + + assert_eq!( + messages.first(), + Some(&LayoutMessage::Grabbed(Split::Detail, 900.0)) + ); + } +} diff --git a/crates/hidegit-ui/src/widget/staging.rs b/crates/hidegit-ui/src/widget/staging.rs index b961afe..e793e0a 100644 --- a/crates/hidegit-ui/src/widget/staging.rs +++ b/crates/hidegit-ui/src/widget/staging.rs @@ -10,7 +10,8 @@ //! behind it — the change it destroys was never committed anywhere. use iced::widget::{ - Space, button, column, container, mouse_area, row, scrollable, text, text_input, tooltip, + Space, button, column, container, mouse_area, responsive, row, scrollable, text, text_input, + tooltip, }; use iced::{Center, Fill, Font, Length, Padding}; @@ -25,8 +26,6 @@ use crate::widget::common; use crate::widget::diff; use crate::widget::sidebar::{heading, item_style}; -const LIST_WIDTH: f32 = 280.0; - #[allow(clippy::too_many_arguments)] pub fn view<'a>( status: &'a WorktreeStatus, @@ -42,6 +41,7 @@ pub fn view<'a>( palette: &'a Palette, text_catalogue: &'a crate::i18n::Catalogue, head: Option, + layout: &'a crate::layout::Layout, ) -> Element<'a, RepoMessage> { // Even a clean tree gets the composer: amending the last commit is a real // thing to want, and it needs nothing staged. @@ -49,11 +49,68 @@ pub fn view<'a>( return column![ container(clean(palette, text_catalogue, head)).height(Fill), common::divider(palette), - container(composer(status, draft, state, palette)).width(Length::Fixed(LIST_WIDTH)), + container(composer(status, draft, state, palette)).width(Length::Fixed(layout.files())), ] .into(); } + // Measured, because the divider between the list and the diff has to know + // how much space it is dividing before it can refuse to give away all of + // it. `responsive` is the only thing that knows; the cost is that the two + // sides are built inside the closure rather than before it, which is why + // the list is its own function. + responsive(move |size| { + row![ + column![ + mouse_area( + container(scrollable(files(status, selected, palette)).height(Fill)) + .width(Length::Fixed(layout.files())) + ) + .on_press(RepoMessage::EditingChanged(false)), + common::divider(palette), + composer(status, draft, state, palette), + ] + .width(Length::Fixed(layout.files())), + crate::widget::split::vertical( + crate::layout::Split::Files, + size.width, + layout.dragging(crate::layout::Split::Files), + palette + ) + .map(RepoMessage::Layout), + mouse_area( + container(pane( + staged, + unstaged, + selected, + lines, + focused_hunk, + mode, + state, + resolver, + status.conflicted.len(), + palette, + )) + .width(Fill), + ) + .on_press(RepoMessage::EditingChanged(false)), + ] + .height(Fill) + .into() + }) + .into() +} + +/// The four lists, in the order the working directory is read in. +/// +/// Its own function rather than a local, because the row it sits in is built +/// inside a `responsive` closure — which may run more than once per frame, and +/// therefore cannot consume anything. +fn files<'a>( + status: &'a WorktreeStatus, + selected: Option, + palette: &'a Palette, +) -> Element<'a, RepoMessage> { let mut list = column![].spacing(0); // Conflicts first: nothing else in the working directory matters until @@ -97,34 +154,7 @@ pub fn view<'a>( } } - row![ - column![ - mouse_area(container(scrollable(list).height(Fill)).width(Length::Fixed(LIST_WIDTH))) - .on_press(RepoMessage::EditingChanged(false)), - common::divider(palette), - composer(status, draft, state, palette), - ] - .width(Length::Fixed(LIST_WIDTH)), - common::vertical_rule(palette), - mouse_area( - container(pane( - staged, - unstaged, - selected, - lines, - focused_hunk, - mode, - state, - resolver, - status.conflicted.len(), - palette, - )) - .width(Fill), - ) - .on_press(RepoMessage::EditingChanged(false)), - ] - .height(Fill) - .into() + list.into() } /// The diff for whichever row is open. diff --git a/crates/hidegit/src/config.rs b/crates/hidegit/src/config.rs index eaba314..448172c 100644 --- a/crates/hidegit/src/config.rs +++ b/crates/hidegit/src/config.rs @@ -156,6 +156,56 @@ pub struct State { /// filename is enough: they are named by the moment they happened, so a /// newer one is a different one. pub announced_panic: Option, + /// Where the window's dividers were left. + pub panes: Panes, +} + +/// Where the dividers between the panes were left. +/// +/// State rather than settings: it is changed by dragging a divider, not by the +/// settings panel, and nobody is expected to open this file to choose a number +/// of pixels. It is written for the same reason the window's size is — having +/// to redo the same three drags on every start is the kind of small friction +/// that makes an application feel unfinished. +/// +/// The interface owns what these mean and what range each may take, so the +/// defaults are its defaults, and a value read from here goes back through its +/// clamp before it is used. +#[derive(Debug, Clone, Copy, Serialize, Deserialize)] +#[serde(default, deny_unknown_fields)] +pub struct Panes { + /// The sidebar's width, in pixels. + pub sidebar: f32, + /// The detail pane's share of the height it splits with the graph. + pub detail: f32, + /// The file list's width inside the working directory, in pixels. + pub files: f32, +} + +impl Default for Panes { + fn default() -> Self { + Self { + sidebar: hidegit_ui::layout::Layout::SIDEBAR, + detail: hidegit_ui::layout::Layout::DETAIL, + files: hidegit_ui::layout::Layout::FILES, + } + } +} + +impl From for Panes { + fn from(layout: hidegit_ui::layout::Layout) -> Self { + Self { + sidebar: layout.sidebar(), + detail: layout.detail(), + files: layout.files(), + } + } +} + +impl From for hidegit_ui::layout::Layout { + fn from(panes: Panes) -> Self { + Self::restore(panes.sidebar, panes.detail, panes.files) + } } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -406,6 +456,11 @@ mod tests { announced_panic: None, announced_update: None, last_update_check: None, + panes: Panes { + sidebar: 310.0, + detail: 0.55, + files: 240.0, + }, }; save(&path, &state); @@ -413,6 +468,26 @@ mod tests { assert_eq!(read.window.width, 1200.0); assert_eq!(read.recents.len(), 1); assert_eq!(read.recents[0].path, PathBuf::from("/src/hideGit")); + assert_eq!( + read.panes.sidebar, 310.0, + "a dragged divider survives a restart" + ); + assert_eq!(read.panes.detail, 0.55); + } + + #[test] + fn a_state_file_from_before_the_dividers_existed_still_loads() { + // `deny_unknown_fields` cuts both ways: a *missing* table has to be + // fine, or the first run after an upgrade loses the whole session — + // recents, window position and all — over a section nobody wrote yet. + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("state.toml"); + std::fs::write(&path, "[window]\nwidth = 1200.0\nheight = 800.0\n").unwrap(); + + let read: State = load(&path); + + assert_eq!(read.window.width, 1200.0); + assert_eq!(read.panes.sidebar, hidegit_ui::layout::Layout::SIDEBAR); } #[test] diff --git a/crates/hidegit/src/main.rs b/crates/hidegit/src/main.rs index fc543a4..aece97a 100644 --- a/crates/hidegit/src/main.rs +++ b/crates/hidegit/src/main.rs @@ -10,6 +10,7 @@ mod crash; use std::path::PathBuf; use hidegit_core::{GitError, MINIMUM_GIT_VERSION, git_preflight}; +use hidegit_ui::message::{LayoutMessage, RepoMessage}; use hidegit_ui::{Hidegit, Message}; use iced::{Size, Subscription, Task, window}; @@ -153,6 +154,7 @@ fn main() -> iced::Result { unannounced_panic: unannounced, announced_update: state.announced_update, last_update_check: state.last_update_check, + panes: state.panes, }; // `mut` is only used on Linux; see the cfg block below. @@ -255,6 +257,8 @@ struct Session { unannounced_panic: Option, announced_update: Option, last_update_check: Option, + /// Where the dividers between the panes were left. + panes: config::Panes, } fn boot( @@ -270,6 +274,7 @@ fn boot( unannounced_panic, announced_update, last_update_check, + panes, } = session; // Off disk once, here, rather than every time the panel is opened: a theme @@ -320,6 +325,9 @@ fn boot( // Set here rather than passed through `Hidegit::new`: the interface only // shows this one, and the shell is what acts on it. ui.app.remember_geometry = config.window.remember_geometry; + // Clamped on the way in by `Layout::restore`, so a hand-edited file cannot + // open hideGit with a sidebar wider than the window. + ui.app.layout = panes.into(); ui.app.panic_reports = config.diagnostics.panic_reports; ui.app.check_for_updates = config.diagnostics.check_for_updates; @@ -407,6 +415,17 @@ fn update(shell: &mut Shell, message: ShellMessage) -> Task { // handler — Cmd+Q on macOS closes the window without ever emitting // `CloseRequested`, and a kill or a panic reaches nothing at all. let touched_recents = matches!(message, Message::RepositoryOpened(_)); + // Written when a drag ends rather than while it runs: the same + // reason the window's own geometry is not written per frame, and + // here it is one file write instead of one per pointer movement. + let touched_panes = matches!( + message, + Message::Layout(LayoutMessage::Released | LayoutMessage::Reset(_)) + | Message::Repo( + _, + RepoMessage::Layout(LayoutMessage::Released | LayoutMessage::Reset(_)) + ) + ); let task = shell.ui.update(message).map(ShellMessage::Ui); if touched_settings { shell.config.theme.name = shell.ui.app.theme.name.clone(); @@ -440,7 +459,7 @@ fn update(shell: &mut Shell, message: ShellMessage) -> Task { }; } - if touched_recents { + if touched_recents || touched_panes { persist(shell); } task @@ -531,6 +550,7 @@ fn persist(shell: &mut Shell) { .map(|path| RecentRepository { path: path.clone() }) .collect(), announced_panic: shell.announced_panic.clone(), + panes: shell.ui.app.layout.into(), announced_update: shell.announced_update.clone(), last_update_check: shell.last_update_check, }; diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 20e3655..b4d84cc 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -382,6 +382,13 @@ Everything between "works" and "someone who does not write Rust can install it". can already see. Opening a repository that is already open switches to its tab rather than opening a second copy — two tabs on one repository would each hold their own idea of its state. Closing lands on the neighbour rather than the last tab +- **Resizable panes.** **Landed.** Every divider in the main window is draggable: the sidebar, the + graph against the detail pane, and the file list against the diff. Two of the three are pixels and + one is a fraction, because a list of names does not need more room on a wider window while the + graph and the commit under it are two halves of one view and should keep their ratio. Where they + are left is written to `state.toml` with the window's own geometry, and clamped on the way back in. + Double-clicking a divider restores it, which is why this needs no settings entry — before it, three + numbers chosen against a window nobody had seen decided how much of a commit message you could read - Settings UI covering everything currently in TOML. **Partly landed**: theme and every alert switch are on a `Cmd+,` panel, applied as they are changed and written back to `config.toml` **in place** — the file keeps its comments, its key order and any key hideGit does not own, because it is diff --git a/docs/UI_SPEC.md b/docs/UI_SPEC.md index 2863bc9..d40f510 100644 --- a/docs/UI_SPEC.md +++ b/docs/UI_SPEC.md @@ -194,7 +194,8 @@ worktree cannot be checked out in another, so the row is the answer to a checkou refused. The right-hand column carries that branch — or Git's own `detached at ` — except when the worktree is locked or its directory is gone, which are states to act on and take the column instead. Both reasons are in the tooltip along with the full path; the row shows the directory name, -because a path does not fit in 230px and its useful end is the last component. +because a path does not fit in a sidebar that is 230px by default and its useful end is +the last component. A worktree whose directory is gone is **listed, not hidden**. It still holds its branch until somebody runs `git worktree prune`, so hiding it would leave the refusal it causes with no visible @@ -279,6 +280,28 @@ submodule it left exactly as it found it, so "the operation succeeded" is not th submodule is now current". The second one is what the user is owed, and a toast says so by name when it is not true. +**Every divider between the panes is draggable.** Three of them: the sidebar against everything +right of it, the graph against the detail pane below it, and — inside the working directory — the +file list against the diff. A divider is a one-pixel rule with a seven-pixel grab area around it, it +shows the platform's resize cursor, and it turns the accent colour while it is being held. +Double-clicking one puts it back where it shipped, which is why resizing needs no settings entry: +the control that moved it is the control that undoes it. + +**Two of them are pixels and one is a fraction, on purpose.** The sidebar and the file list hold +names, and a name does not get longer because the window did, so they keep the width they were +given. The graph and the detail pane are two halves of one view, so they hold a *ratio* — a taller +window keeps the graph the subject and the commit under it the footnote, at whatever proportion the +last drag chose. Defaults are 230px, 60/40, and 280px. + +Nothing can be dragged out of existence: the sidebar and the file list stop at a minimum and at half +the space they divide, and the graph and detail pane each keep at least 15% of the column. A pane +with no height has no divider left to drag it back with. + +**Where they end up is written to `state.toml`, not to `config.toml`.** It is state, like the +window's own size — changed by dragging rather than by the settings panel, written when the drag +ends rather than during it, and clamped again on the way back in so a hand-edited file cannot open +hideGit with a sidebar wider than the window. + **Graph** — the centre. Virtualised: only visible rows are laid out and drawn. Refs are rendered as badges on their commits. Selecting a row updates the detail pane. Full rendering rules in [COMMIT_GRAPH.md](./COMMIT_GRAPH.md).