From 9cd9e2961489d27b14b9ebe5ebd671b35a218028 Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Sat, 10 Oct 2026 12:51:55 -0400 Subject: [PATCH] LR-BEHAV-SIDEBAR-STICKY: rows beneath the pinned header are not drop targets An album or photos released on the pinned Albums header went to the row scrolled under it. Folding from the pinned header keeps the sideways scroll, its hand-off to the next frame can't go stale, and a section picked up by the pinned header doesn't start scrolling at once. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/panels/left.rs | 60 ++++++++++++++++++++----- crates/ui-egui/src/tests_panels.rs | 71 +++++++++++++++++++++++++++++- docs/parity.md | 2 +- 3 files changed, 120 insertions(+), 13 deletions(-) diff --git a/crates/ui-egui/src/panels/left.rs b/crates/ui-egui/src/panels/left.rs index 1ef894b32..a35e44289 100644 --- a/crates/ui-egui/src/panels/left.rs +++ b/crates/ui-egui/src/panels/left.rs @@ -206,9 +206,17 @@ fn section( let y = header.top() - SECTION_GAP / 2.0; ui.painter().hline(header.left() + 18.0..=(edge - 18.0).max(header.left() + 18.0), y, egui::Stroke::new(1.0, t.divider)); let show = egui::Id::new("left-show-section"); - if ui.data(|d| d.get_temp::(show)) == Some(section) { - ui.data_mut(|d| d.remove::(show)); - ui.scroll_to_rect_animation(header, Some(egui::Align::TOP), egui::style::ScrollAnimation::none()); + if let Some((asked, frame)) = ui.data(|d| d.get_temp::<(SidebarSection, u64)>(show)) + && asked == section + { + ui.data_mut(|d| d.remove::<(SidebarSection, u64)>(show)); + // asked for by the frame before (a request the section was not drawn for is forgotten), + // and up or down only: the sidebar stays where it was scrolled sideways + if ui.ctx().cumulative_frame_nr().saturating_sub(frame) <= 2 { + let view_top: f32 = ui.data(|d| d.get_temp(egui::Id::new("left-view-top"))).unwrap_or(header.top()); + let delta = vec2(0.0, view_top - header.top()); + ui.scroll_with_delta_animation(delta, egui::style::ScrollAnimation::none()); + } } let open = section_header(app, ui, section, header, rows, false).open; if open { @@ -232,9 +240,14 @@ fn visible_right(ui: &egui::Ui) -> f32 { /// belong to, and the section can be folded, moved or hidden from anywhere in a long list. The /// next section's header pushes it out as it arrives. `view_top`: where the visible part starts. fn pinned_header(app: &mut LightcraftApp, ui: &mut egui::Ui, placed: &[PlacedSection], view_top: f32) { - let Some(p) = placed.iter().find(|p| p.open && p.top < view_top && p.bottom > view_top) else { return }; - let top = view_top.min(p.bottom - SECTION_HEADER); - let r = Rect::from_x_y_ranges(ui.max_rect().x_range(), top..=top + SECTION_HEADER); + let pinned = placed.iter().find(|p| p.open && p.top < view_top && p.bottom > view_top); + let rect = pinned.map(|p| { + let top = view_top.min(p.bottom - SECTION_HEADER); + Rect::from_x_y_ranges(ui.max_rect().x_range(), top..=top + SECTION_HEADER) + }); + // rows ask where it is (next frame: they are laid out before it) + ui.data_mut(|d| d.insert_temp(egui::Id::new("left-pinned-rect"), rect)); + let (Some(p), Some(r)) = (pinned, rect) else { return }; let t = Tokens::get(ui.ctx()); let view = ui.clip_rect(); // over the rows scrolling beneath it @@ -243,7 +256,8 @@ fn pinned_header(app: &mut LightcraftApp, ui: &mut egui::Ui, placed: &[PlacedSec if section_header(app, ui, p.section, r, p.rows, true).toggled { // folded from here: its own header comes into view, not whatever was below the section // (next frame, when the section is laid out folded: see `section`) - ui.data_mut(|d| d.insert_temp(egui::Id::new("left-show-section"), p.section)); + let frame = ui.ctx().cumulative_frame_nr(); + ui.data_mut(|d| d.insert_temp(egui::Id::new("left-show-section"), (p.section, frame))); ui.ctx().request_repaint(); } } @@ -383,7 +397,9 @@ fn section_header(app: &mut LightcraftApp, ui: &mut egui::Ui, section: SidebarSe ui.painter().add(egui::Shape::convex_polygon(triangle_points(c, open), col, egui::Stroke::NONE)); ui.painter().text(pos2(r.left() + 18.0, r.center().y), Align2::LEFT_CENTER, title, t.semibold(13.5), t.text_label); let has_actions = section == SidebarSection::Albums; - if !open && rows > 0 && app.ui.show_counts { + // (an album being dragged puts "Top Level" there) + let room = !(has_actions && app.ui.dragging_album.is_some()); + if !open && rows > 0 && app.ui.show_counts && room { // folded: how much is in there (left of the header's own buttons) let right = edge - 18.0 - if has_actions { 30.0 } else { 0.0 }; let count = ui.painter().text(pos2(right, r.center().y), Align2::RIGHT_CENTER, rows.to_string(), t.font(12.5), t.text_dim); @@ -492,6 +508,7 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { drag_auto_scroll(app, ui); // where the visible part of the content starts (screen y), for the pinned header let view_top = ui.cursor().top() + viewport.min.y; + ui.data_mut(|d| d.insert_temp(egui::Id::new("left-view-top"), view_top)); // rows are as wide as the widest one needs (last frame), at least the panel let wide = viewport.width().max(content_width(ui.ctx())); ui.set_min_width(wide); @@ -1084,6 +1101,22 @@ fn pointer_over(ui: &egui::Ui, rect: Rect) -> bool { ui.input(|i| i.pointer.latest_pos()).is_some_and(|p| rect.contains(p) && ui.clip_rect().contains(p)) } +/// [`pointer_over`] for a row: a row scrolled beneath the pinned header is covered by it, so the +/// pointer there is over the header, not over the row. +fn pointer_over_row(ui: &egui::Ui, rect: Rect) -> bool { + pointer_over(ui, rect) && !pointer_on_pinned_header(ui) +} + +/// Where the pinned header was drawn last frame, if one was (rows are laid out before it). +fn pinned_rect(ui: &egui::Ui) -> Option { + ui.data(|d| d.get_temp::>(egui::Id::new("left-pinned-rect"))).flatten() +} + +fn pointer_on_pinned_header(ui: &egui::Ui) -> bool { + let pos = ui.input(|i| i.pointer.latest_pos()); + pinned_rect(ui).zip(pos).is_some_and(|(r, p)| r.contains(p)) +} + /// How close to the top or bottom edge of the sidebar (points) a dragged album starts scrolling it. const SCROLL_EDGE: f32 = 36.0; /// The fastest the sidebar scrolls for a drag (points per second). @@ -1106,6 +1139,11 @@ fn drag_auto_scroll(app: &LightcraftApp, ui: &egui::Ui) { } let view = ui.clip_rect(); let Some(p) = ui.input(|i| i.pointer.latest_pos()).filter(|p| p.x >= view.left() && p.x <= view.right()) else { return }; + // a section picked up by its pinned header starts inside the top edge: it scrolls once it has + // left the header (albums and photos scroll there as at any edge, to reach the rows above) + if app.ui.dragging_section.is_some() && pointer_on_pinned_header(ui) { + return; + } let speed = auto_scroll_speed(p.y, view.top(), view.bottom()); if speed != 0.0 { // a positive delta moves the content down: the view goes up @@ -1218,7 +1256,7 @@ fn album_drop_at(app: &LightcraftApp, dragged: AlbumId, target: &Album, frac: f3 fn album_drag_over(app: &mut LightcraftApp, ui: &mut egui::Ui, resp: &egui::Response, row: &Album, open: Option<&mut bool>, indent: f32) -> bool { let hover_id = egui::Id::new(("album-hover", row.id.0)); let Some(dragged) = app.ui.dragging_album.map(AlbumId) else { return false }; - let pos = ui.input(|i| i.pointer.latest_pos()).filter(|_| pointer_over(ui, resp.rect)); + let pos = ui.input(|i| i.pointer.latest_pos()).filter(|_| pointer_over_row(ui, resp.rect)); let Some(pos) = pos else { if ui.data(|d| d.get_temp::(hover_id)).is_some() { ui.data_mut(|d| d.remove_temp::(hover_id)); @@ -1316,8 +1354,8 @@ pub fn album_drag_feedback(app: &mut LightcraftApp, ctx: &egui::Context) { /// release there adds them. fn drop_target(app: &mut LightcraftApp, ui: &mut egui::Ui, resp: &egui::Response, a: &Album) { let Some(ids) = app.ui.dragging_photos.clone() else { return }; - let over = ui.input(|i| i.pointer.latest_pos()).is_some_and(|p| resp.rect.contains(p)); - if !over { + // only where the row can be seen: not scrolled out of the sidebar or beneath the pinned header + if !pointer_over_row(ui, resp.rect) { return; } let t = Tokens::get(ui.ctx()); diff --git a/crates/ui-egui/src/tests_panels.rs b/crates/ui-egui/src/tests_panels.rs index ddb09401c..362725a65 100644 --- a/crates/ui-egui/src/tests_panels.rs +++ b/crates/ui-egui/src/tests_panels.rs @@ -711,7 +711,7 @@ fn the_header_of_a_long_section_stays_in_sight() { pinned.top() > header(&h, "albums").bottom() && first_row < pinned.bottom() + 29.0, "at the top of the rows in view: {pinned:?} {first_row}" ); - assert!(has(&h, "icon:albumNew"), "with its buttons"); + assert!(h.app.widgets.iter().any(|(w, r)| w == "icon:albumNew" && pinned.contains_rect(*r)), "with its buttons"); // folded from the pinned header: Albums' own header is what the sidebar shows now click(&mut h, "sidebarSectionPinned:albums"); h.settle(SETTLE); @@ -722,6 +722,75 @@ fn the_header_of_a_long_section_stays_in_sight() { assert!(own.top() >= panel.top() && own.bottom() <= panel.bottom(), "its own header is in view: {own:?}"); } +/// Scroll the sidebar (the pointer over it) by `dy`. +fn scroll_sidebar(h: &mut Headless, dy: f32) { + h.request("ui.move", json!({"x": 120.0, "y": 400.0}), T); + let r = h.request("ui.scroll", json!({"dy": dy}), T); + assert_eq!(r["ok"], true, "{r}"); + for _ in 0..60 { + h.step(); + } +} + +/// Given a pinned Albums header with album folders scrolled beneath it, when an album is dragged +/// onto the pinned header and released, then it goes to the top level — what the header shows — +/// and not into the folder hidden under the header. +#[test] +fn a_drop_on_the_pinned_header_is_not_a_drop_on_the_row_beneath_it() { + let mut h = demo([1400.0, 700.0], json!({"view": "photoGrid", "leftPanel": true})); + let make = |h: &mut Headless, params: serde_json::Value| h.app.session.execute("album.create", ¶ms).unwrap()["id"].as_u64().unwrap(); + for i in 0..30 { + make(&mut h, json!({"name": format!("Folder {i:02}"), "folder": true})); + } + let last = make(&mut h, json!({"name": "Zz last", "folder": true})); + let best = make(&mut h, json!({"name": "Best", "parent": last})); + h.step(); + h.step(); + let parent = |h: &Headless| h.app.session.catalog.album(lightcraft_catalog::AlbumId(best)).unwrap().parent.map(|p| p.0); + let row = |h: &Headless| widget(h, &format!("source:album:{best}")); + // far enough that Best is in view and Albums' own header is not + for _ in 0..8 { + if row(&h).bottom() < 600.0 { + break; + } + scroll_sidebar(&mut h, -200.0); + } + let pinned = widget(&h, "sidebarSectionPinned:albums"); + assert!(row(&h).bottom() < 600.0 && row(&h).top() > pinned.bottom(), "Best is in view below the pinned header: {:?}", row(&h)); + let beneath = h.app.widgets.iter().filter(|(w, r)| w.starts_with("source:folder:") && r.contains(pinned.center())).count(); + assert_eq!(beneath, 1, "a folder row lies under the middle of the pinned header"); + let from = row(&h).center(); + drag_between(&mut h, from, pinned.center(), Vec::new()); + assert_eq!(parent(&h), None, "to the top level, as the header said"); +} + +/// The pinned header belongs to the section whose rows are at the top of the sidebar: scrolling on +/// into the next section pushes it out and pins that section's header instead. +#[test] +fn the_next_section_takes_over_the_pinned_header() { + let mut h = demo([1400.0, 500.0], json!({"view": "photoGrid", "leftPanel": true, "sidebar": ["albums", "keywords"]})); + exec(&mut h, "view.sidebarSection", json!({"section": "local", "show": false})); + exec(&mut h, "view.sidebarSection", json!({"section": "byDate", "show": false})); + exec(&mut h, "view.sidebarSection", json!({"section": "folders", "show": false})); + for i in 0..12 { + h.app.session.execute("album.create", &json!({"name": format!("Album {i:02}")})).unwrap(); + } + h.step(); + let pinned = |h: &Headless| -> Vec { + h.app.widgets.iter().filter_map(|(w, _)| w.strip_prefix("sidebarSectionPinned:").map(str::to_string)).collect() + }; + let mut seen = Vec::new(); + for _ in 0..30 { + scroll_sidebar(&mut h, -60.0); + let now = pinned(&h); + assert!(now.len() <= 1, "one header at a time: {now:?}"); + if let Some(id) = now.first().filter(|id| seen.last() != Some(*id)) { + seen.push(id.clone()); + } + } + assert_eq!(seen, ["albums", "keywords"]); +} + /// A folded section says how many rows it holds (unless photo counts are off); an open one /// doesn't need to. #[test] diff --git a/docs/parity.md b/docs/parity.md index aca0c0561..48ec8761c 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -529,7 +529,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins. | LR-BEHAV-SIDEBAR-SECTIONS | Left-sidebar sections share one frame and one arrangement | P2 | ✅ | `crates/ui-egui/src/sidebar.rs` (`SidebarSection`, `SidebarLayout`), `crates/ui-egui/src/panels/left.rs` (`section`, `section_header`, `tree_toggle`), `crates/ui-egui/src/tests_panels.rs` | Albums, Local, By Date, Folders and Keywords are drawn by one `section` frame (the same gap above, header, fold and accessibility name) from one arrangement, `sidebar` in the UI state: a list of `{id, hidden, collapsed}` top to bottom, also settable with `ui.set`. Whatever is read, every section is in it exactly once (unknown and repeated entries are dropped, the rest follow in the usual order), so a damaged or newer settings file never loses the other settings. Rows that open (album folders, Local folders, dates, library folders, keywords) share `tree_toggle`: the same triangle, the same remembered open state, announced as Expand / Collapse. A section with nothing to list (Folders before anything is imported, Keywords before any is added) stays, open, with a one-line note in place of rows (`sidebarEmpty:`) instead of disappearing. `ui.set` reads `sidebar` strictly (`SidebarLayout::parse`): an unknown or repeated section, or a flag that is not true / false, is refused and changes nothing; sections left out follow in the usual order, shown and open; the older `collapsedSidebar` list still folds and unfolds. My Photos (All Photos, Recently Added, Picks, Missing Photos, Recently Deleted) is not a section: it stays on top | | LR-BEHAV-SIDEBAR-HIDE | Show and hide left-sidebar sections | P2 | ✅ | `cmd:view.sidebarSection`, `cmd:view.sidebarReset`, `crates/ui-egui/src/sidebar.rs`, `crates/ui-egui/src/panels/left.rs` (`sections_menu`), `crates/ui-egui/src/menubar.rs`, `crates/ui-egui/src/tests_panels.rs` | View ▸ Sidebar Sections lists Albums, Local, By Date, Folders and Keywords in the sidebar's order, each checked while shown; unchecking one takes its header and rows off the sidebar, checking it puts it back where and how it was (still folded if it was). The same list opens with a right-click on any section header (with Hide This Section first; widgets `sidebarMenu:hide`, `sidebarMenu:section:`, `sidebarMenu:reset`) and on the My Photos title (`sidebarTitle`), which is always there, so hidden sections can always come back. Reset Sidebar Sections (enabled once something changed) shows and opens every section in the usual order. `view.sidebarSection` `{section, show?, collapsed?}` flips shown / hidden when neither flag is given and refuses an unknown section or a flag that is not true / false. Hiding the section of what the grid shows (an album, a keyword filter) leaves the grid as it is; an album made while Albums is hidden or folded brings Albums back, open. Kept with the UI state (`sidebar`). My Photos itself can't be hidden | | LR-BEHAV-SIDEBAR-REORDER | Reorder left-sidebar sections | P2 | ✅ | `cmd:view.sidebarMoveSection`, `crates/ui-egui/src/sidebar.rs` (`move_before`, `move_by`), `crates/ui-egui/src/panels/left.rs` (`section_drag`, `section_drop_before`, `section_drag_feedback`), `crates/ui-egui/src/tests_panels.rs` | drag a section by its header: its name follows the pointer, a line (`sidebarSectionDrop`) shows the gap it would go to (above the section whose upper half the pointer is over, or below the last), releasing over the sidebar moves it there, Esc or releasing elsewhere leaves it; the sidebar scrolls when the drag nears its top or bottom edge. A click on the header still folds the section, and an album dragged onto the Albums header is still an album drag. The header's menu has Move Up / Move Down (`sidebarMenu:moveUp` / `moveDown`), one shown section at a time, stepping over hidden ones, disabled at the ends. `view.sidebarMoveSection` `{section, direction: up\|down}` or `{section, before:
\|null}` (null: last) answers `{moved, order}`. The order is kept with the UI state (`sidebar`). Recently Deleted is listed with My Photos (below Picks / Missing Photos), not after the sections. Not here: dragging My Photos, or rows from one section into another | -| LR-BEHAV-SIDEBAR-STICKY | Pinned section header and section look | P2 | ✅ | `crates/ui-egui/src/panels/left.rs` (`pinned_header`, `section_header`, `empty_hint`), `crates/ui-egui/src/tests_panels.rs` | while the rows of an open section are in view but its header has scrolled away, a copy of the header (`sidebarSectionPinned:`, with the section's buttons and menu) stays at the top of the sidebar; the next section's header pushes it out; folding from it brings the section's own header into view. Sections are told apart by a hairline in the gap above each; the fold chevron sits in the gutter, in the column of the rows' triangles (`sidebarSectionChevron:`); a header highlights under the pointer; a folded section shows how many top-level rows it holds (`sidebarSectionCount:`, with Show Photo Counts); a count of 0 is fainter. Not here: a height of its own per section (one scroll area serves the whole sidebar), a count for Recently Added, Local's Browse Folder… as a header button | +| LR-BEHAV-SIDEBAR-STICKY | Pinned section header and section look | P2 | ✅ | `crates/ui-egui/src/panels/left.rs` (`pinned_header`, `section_header`, `empty_hint`), `crates/ui-egui/src/tests_panels.rs` | while the rows of an open section are in view but its header has scrolled away, a copy of the header (`sidebarSectionPinned:`, with the section's buttons and menu) stays at the top of the sidebar; the next section's header pushes it out; folding from it brings the section's own header into view. Rows scrolled beneath the pinned header are covered by it: an album or photos dropped on the pinned Albums header go to the top level / nowhere, never to the row under it; folding from it keeps the sideways scroll. Sections are told apart by a hairline in the gap above each; the fold chevron sits in the gutter, in the column of the rows' triangles (`sidebarSectionChevron:`); a header highlights under the pointer; a folded section shows how many top-level rows it holds (`sidebarSectionCount:`, with Show Photo Counts); a count of 0 is fainter. Not here: a height of its own per section (one scroll area serves the whole sidebar), a count for Recently Added, Local's Browse Folder… as a header button | | LR-BEHAV-PANEL-RESIZE | Resizable side panels | P1 | ✅ | `crates/ui-egui/src/panels/mod.rs` (`resizable_side`), `crates/ui-egui/src/state.rs` (`LEFT_WIDTH`, `RIGHT_WIDTH`), `crates/ui-egui/src/tests_panels.rs` | drag the left sidebar's right edge (200–480 pt) or the right panel's left edge (250–520 pt); the photo area keeps ≥ 360 pt; widths are kept across panels, views and restarts (`leftWidth` / `rightWidth` in the UI state); the Presets column stays fixed | | LR-BEHAV-EMPTY-STATES | Empty states | P1 | ✅ | `crates/ui-egui/src/panels/mod.rs` (`empty_message`) | | | LR-BEHAV-TOOLTIPS | Tooltips with shortcuts | P0 | ✅ | `crates/ui-egui/src/panels/bottombar.rs` | |