Integration: History follows the history, not every revision, and never moves a visible row (#5940)

The panel scrolled with `scroll_to_rect(current, Some(Align::BOTTOM))` on every revision
change. In egui an explicit alignment always realigns, and the revision also changes when
nothing in the history does (selecting a layer, a channel eye). So clicking a visible older
state slid it to the bottom under the pointer, Undo hid the greyed redo states, a layer click
threw a user who had scrolled up back to the bottom, and selecting a snapshot scrolled it out
of view.

Now the panel follows the history position (document, past/redo counts, current name, and the
newest past state, which also changes at the state limit), skips it while a snapshot is
selected, and scrolls without alignment, so only a hidden current row moves into view. New
tests: a clicked visible state stays put, Undo keeps the redo state in view, a layer click keeps
the scroll, a selected snapshot stays in view, a new state at the limit is revealed; the first
three fail on the PR head, the others without the snapshot guard / the newest-state key.

Co-authored-by: nrq6993-a11y <342501791+nrq6993-a11y@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

crates/ui-egui/src/history_panel_select_tests.rs

@@ -83,3 +83,145 @@ fn new_history_steps_scroll_the_current_state_into_view() {
     let view = h.ctx.input(|i| i.viewport_rect());
     assert!(view.contains_rect(row), "the current history row must scroll into view: row {row:?} view {view:?}",);
 }
+
+/// Open plus 18 New Layer states in a dock that shows only a few rows.
+fn long_harness() -> Harness<'static, PhotocraftApp> {
+    let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), Default::default());
+    app.run("file.new", json!({"width": 64, "height": 48})).unwrap();
+    for n in 0..18 {
+        app.run("layer.new.layer", json!({"name": format!("Layer {n}")})).unwrap();
+    }
+    let mut ready = false;
+    let mut h = Harness::builder().with_size(vec2(280.0, 220.0)).build_ui_state(
+        move |ui, app: &mut PhotocraftApp| {
+            if !ready {
+                PhotocraftApp::setup_context(ui.ctx(), theme::ThemeKind::ProMedium);
+                ready = true;
+                return;
+            }
+            history(app, ui);
+        },
+        app,
+    );
+    h.run_steps(4);
+    h
+}
+
+/// Top of the `n`th New Layer row from the end (0 = the last).
+fn row_top(h: &Harness<'_, PhotocraftApp>, n: usize) -> f32 {
+    let rows: Vec<_> = h.get_all_by_label("New Layer").map(|r| r.rect()).collect();
+    rows[rows.len() - 1 - n].top()
+}
+
+/// The rows' visible band: below the window top, above the footer.
+fn shown(h: &Harness<'_, PhotocraftApp>, n: usize) -> bool {
+    let footer = h.get_by_label("Delete current state").rect().top();
+    let top = row_top(h, n);
+    top >= h.ctx.input(|i| i.viewport_rect()).top() && top + 24.0 <= footer + 0.5
+}
+
+#[test]
+fn clicking_a_visible_state_leaves_the_list_still() {
+    let mut h = long_harness();
+    assert!(shown(&h, 0) && shown(&h, 1), "the newest states are in view");
+    let before = row_top(&h, 1);
+    // A click on the state above the current one goes back one step; the row stays under the pointer.
+    let rows: Vec<_> = h.get_all_by_label("New Layer").collect();
+    rows[rows.len() - 2].click();
+    h.run_steps(4);
+    assert_eq!(h.state().session.active().unwrap().history.redo_labels().count(), 1);
+    assert_eq!(row_top(&h, 1), before, "the clicked row must not move");
+}
+
+#[test]
+fn undo_and_a_layer_click_keep_the_redo_state_in_view() {
+    let mut h = long_harness();
+    h.state_mut().run("edit.undo", json!({})).unwrap();
+    h.run_steps(4);
+    assert!(shown(&h, 0), "the greyed redo state stays in view after Undo");
+    let before = row_top(&h, 0);
+    // Selecting a layer bumps the revision but adds no state: the list stays where it is.
+    let id = h.state().session.active().unwrap().doc.layers[0].id;
+    h.state_mut().session.select_layer(id).unwrap();
+    h.run_steps(4);
+    assert_eq!(row_top(&h, 0), before, "a layer click must not scroll History");
+}
+
+#[test]
+fn scrolling_away_survives_a_change_that_adds_no_state() {
+    let mut h = long_harness();
+    let view = h.ctx.input(|i| i.viewport_rect());
+    h.event(egui::Event::PointerMoved(view.center()));
+    for _ in 0..3 {
+        h.event(egui::Event::MouseWheel {
+            unit: egui::MouseWheelUnit::Line,
+            delta: vec2(0.0, 3.0),
+            phase: egui::TouchPhase::Move,
+            modifiers: Default::default(),
+        });
+        h.run_steps(1);
+    }
+    h.run_steps(4);
+    let scrolled = row_top(&h, 0);
+    assert!(!shown(&h, 0), "the wheel scrolled the current state out of view");
+    let id = h.state().session.active().unwrap().doc.layers[0].id;
+    h.state_mut().session.select_layer(id).unwrap();
+    h.run_steps(4);
+    assert_eq!(row_top(&h, 0), scrolled, "the user's scroll position is kept");
+    // A new state brings the current row back into view.
+    h.state_mut().run("layer.new.layer", json!({})).unwrap();
+    h.run_steps(4);
+    assert!(shown(&h, 0), "a new state scrolls into view");
+}
+
+#[test]
+fn clicking_a_snapshot_keeps_it_in_view() {
+    let mut h = long_harness();
+    h.state_mut().run("history.newSnapshot", json!({})).unwrap();
+    h.run_steps(2);
+    // Scroll to the top, where the snapshots are.
+    let view = h.ctx.input(|i| i.viewport_rect());
+    h.event(egui::Event::PointerMoved(view.center()));
+    for _ in 0..10 {
+        h.event(egui::Event::MouseWheel {
+            unit: egui::MouseWheelUnit::Line,
+            delta: vec2(0.0, 5.0),
+            phase: egui::TouchPhase::Move,
+            modifiers: Default::default(),
+        });
+        h.run_steps(1);
+    }
+    h.run_steps(4);
+    let before = h.get_all_by_label("Snapshot 1").next().expect("the snapshot row").rect().top();
+    // Selecting it adds a "Snapshot 1" state at the bottom; the selected snapshot row must not scroll away.
+    h.get_all_by_label("Snapshot 1").next().expect("the snapshot row").click();
+    h.run_steps(4);
+    assert_eq!(h.state().session.active().unwrap().history.entries().last().map(String::as_str), Some("Snapshot 1"));
+    assert_eq!(h.get_all_by_label("Snapshot 1").next().expect("the snapshot row").rect().top(), before, "the clicked snapshot stays put");
+}
+
+#[test]
+fn a_new_state_at_the_history_limit_still_scrolls_into_view() {
+    let mut h = long_harness();
+    // At the limit a new step drops the oldest: the count and the (same) name stay as they were.
+    let st = h.state_mut().session.active_mut().unwrap();
+    st.history.max_states = st.history.past_len();
+    let view = h.ctx.input(|i| i.viewport_rect());
+    h.event(egui::Event::PointerMoved(view.center()));
+    for _ in 0..3 {
+        h.event(egui::Event::MouseWheel {
+            unit: egui::MouseWheelUnit::Line,
+            delta: vec2(0.0, 3.0),
+            phase: egui::TouchPhase::Move,
+            modifiers: Default::default(),
+        });
+        h.run_steps(1);
+    }
+    h.run_steps(4);
+    assert!(!shown(&h, 0), "the wheel scrolled the current state out of view");
+    let past = h.state().session.active().unwrap().history.past_len();
+    h.state_mut().run("layer.new.layer", json!({})).unwrap();
+    assert_eq!(h.state().session.active().unwrap().history.past_len(), past, "the oldest state was dropped");
+    h.run_steps(4);
+    assert!(shown(&h, 0), "a new state at the limit scrolls into view");
+}

crates/ui-egui/src/panels.rs

@@ -3605,12 +3605,15 @@ fn history(app: &mut PhotocraftApp, ui: &mut egui::Ui) {
         return;
     };
     let doc = st.doc.clone();
-    let revision = st.revision;
     let entries = st.history.entries();
     let current = entries.len() - 1;
     let redo: Vec<String> = st.history.redo_labels().map(str::to_string).collect();
     let source = photocraft_engine::history_source_cmds::source(st);
     let selected_snap = selected_snapshot(ui.ctx(), st);
+    // The newest past state is a new document on every step, Undo and Redo, also at the state
+    // limit, where the counts and a repeated name stay the same.
+    let newest = st.history.past_len().checked_sub(1).and_then(|i| st.history.state(i)).map(|d| std::sync::Arc::as_ptr(&d) as usize);
+    let spot = (doc.id, history_spot(st), newest);
     let ctx = ui.ctx().clone();
     // A row dragged onto the trash (#5324); a drag that never saw its release must not outlive it.
     if !ctx.input(|i| i.pointer.primary_down() || i.pointer.any_released()) {
@@ -3638,8 +3641,16 @@ fn history(app: &mut PhotocraftApp, ui: &mut egui::Ui) {
     // The dock gives History a fixed height: the rows scroll above the footer.
     let footer = if t.pro { widgets::footer_height(ui) + ui.spacing().item_spacing.y } else { 0.0 };
     let max_h = (ui.available_height() - footer).max(40.0);
-    let follow_id = ui.id().with(("history-follow-rev", doc.id));
-    let follow = ui.ctx().data(|d| d.get_temp::<u64>(follow_id)) != Some(revision);
+    // Follow the current state when the history moves (a new step, Undo, a state row) or another
+    // document comes up (`spot`), scrolling only if its row is hidden (#5909). Other changes (a
+    // layer click, a channel eye) and a selected snapshot leave the list where the user put it.
+    let follow_id = egui::Id::new("history-follow");
+    let moved = ui.ctx().data_mut(|d| {
+        let moved = d.get_temp::<(photocraft_doc::DocId, HistorySpot, Option<usize>)>(follow_id).as_ref() != Some(&spot);
+        d.insert_temp(follow_id, spot);
+        moved
+    });
+    let follow = moved && selected_snap.is_none();
     egui::ScrollArea::vertical().id_salt("history-rows").max_height(max_h).auto_shrink([false, false]).show(ui, |ui| {
         // Snapshots head the list and scroll with the states, so many of them never push the
         // footer out of a short dock.
@@ -3693,9 +3704,9 @@ fn history(app: &mut PhotocraftApp, ui: &mut egui::Ui) {
             }
         }
         if follow && let Some(rect) = current_rect {
-            ui.scroll_to_rect(rect, Some(egui::Align::BOTTOM));
+            // No alignment: a row already in view stays put. No animation, as the Layers panel reveal.
+            ui.scroll_to_rect_animation(rect, None, egui::style::ScrollAnimation::none());
         }
-        ui.ctx().data_mut(|d| d.insert_temp(follow_id, revision));
     });
     if let Some(i) = set_source
         && let Err(e) = app.run("history.setSource", json!({"state": i}))