From 022d8cda2ad218ef85a98cce2e2f80a0af71e073 Mon Sep 17 00:00:00 2001 From: "sorokin.oleg.edu" Date: Sun, 13 Sep 2026 02:27:04 -0700 Subject: [PATCH] Preserve rename preview scroll through full details --- crates/oa-desktop/src/app/source_rename.rs | 41 +++++- .../app/source_rename/preview_scroll_tests.rs | 124 +++++++++++++++++- 2 files changed, 159 insertions(+), 6 deletions(-) diff --git a/crates/oa-desktop/src/app/source_rename.rs b/crates/oa-desktop/src/app/source_rename.rs index 7bcbaa0a..131976a9 100644 --- a/crates/oa-desktop/src/app/source_rename.rs +++ b/crates/oa-desktop/src/app/source_rename.rs @@ -49,6 +49,7 @@ pub enum Event { Line(Id, Id, bool), Segment(Id, Id, bool), Details(Id, bool), + PreviewScrolled(Id, Id, Id, f32), DetailSegment(Id, bool), ReadOnly(Id, text_editor::Action), Undo(Id), @@ -112,6 +113,7 @@ struct Form { line: usize, segment: usize, preview_destination: Id, + preview_offset: f32, details: bool, detail_segment: usize, detail_editor: text_editor::Content, @@ -170,6 +172,17 @@ impl App { return false; }; match event { + Event::PreviewScrolled(invocation, approval, destination, offset) => { + *invocation == form.invocation + && !form.details + && form + .approval + .as_ref() + .is_some_and(|ready| ready.id == *approval) + && *destination == form.preview_destination + && offset.is_finite() + && *offset >= 0. + } Event::Preview(invocation, generation) => { *invocation == form.invocation && *generation == form.generation } @@ -435,11 +448,23 @@ impl App { } form.details = value; // Leaving or returning to the preview ends the old visit. Keep - // its selected page and retained scroll; only retire delayed - // operations, without scheduling another return-to-top. + // its selected page and scroll; the Details widget replaces the + // preview subtree, so restore the reported offset on return. form.preview_destination = Id::new_v4(); form.detail_segment = 0; refresh_details(form); + if !value && let Some(ready) = &form.approval { + return iced::widget::operation::scroll_to( + preview_scroll_id(form, ready.id), + iced::widget::scrollable::AbsoluteOffset { + x: None, + y: Some(form.preview_offset), + }, + ); + } + } + Event::PreviewScrolled(_, _, _, offset) => { + self.source_rename.form.as_mut().unwrap().preview_offset = offset; } Event::DetailSegment(_, next) => { let form = self.source_rename.form.as_mut().unwrap(); @@ -516,6 +541,7 @@ impl App { line: 0, segment: 0, preview_destination: Id::new_v4(), + preview_offset: 0., details: false, detail_segment: 0, detail_editor: text_editor::Content::new(), @@ -953,6 +979,7 @@ fn input_id(invocation: Id) -> iced::widget::Id { // or changing a filter that leaves the page tuple unchanged (the ABA case). fn reset_preview_position(form: &mut Form) -> Task { form.preview_destination = Id::new_v4(); + form.preview_offset = 0.; if !form.details && let Some(ready) = &form.approval { @@ -1118,7 +1145,15 @@ where pager(format!("Row segments {}/{} ยท both sides", form.segment + 1, segments.max(1)), (form.segment > 0).then(|| message(Event::Segment(invocation, preview_id, false))), (form.segment + 1 < segments).then(|| message(Event::Segment(invocation, preview_id, true)))), - scrollable(displayed).id(preview_scroll_id(form, preview_id)).height(Fill).width(Fill).spacing(8), + scrollable(displayed) + .id(preview_scroll_id(form, preview_id)) + .on_scroll({ + let destination = form.preview_destination; + move |viewport| message(Event::PreviewScrolled( + invocation, preview_id, destination, viewport.absolute_offset().y, + )) + }) + .height(Fill).width(Fill).spacing(8), ].spacing(6).height(Fill).into() } else { scrollable(column![ diff --git a/crates/oa-desktop/src/app/source_rename/preview_scroll_tests.rs b/crates/oa-desktop/src/app/source_rename/preview_scroll_tests.rs index 4adb3bf4..0d03c317 100644 --- a/crates/oa-desktop/src/app/source_rename/preview_scroll_tests.rs +++ b/crates/oa-desktop/src/app/source_rename/preview_scroll_tests.rs @@ -50,6 +50,7 @@ fn scroll_id(app: &App) -> widget::Id { struct Geometry { id: widget::Id, offset: Option, + bounds: Option, } impl widget::Operation for Geometry { fn traverse(&mut self, visit: &mut dyn FnMut(&mut dyn widget::Operation)) { @@ -58,13 +59,14 @@ impl widget::Operation for Geometry { fn scrollable( &mut self, id: Option<&widget::Id>, - _: Rectangle, + bounds: Rectangle, _: Rectangle, offset: Vector, _: &mut dyn widget::operation::Scrollable, ) { if id == Some(&self.id) { self.offset = Some(offset); + self.bounds = Some(bounds); } } } @@ -95,6 +97,7 @@ impl Mounted { let mut geometry = Geometry { id: scroll_id(app), offset: None, + bounds: None, }; self.operate(app, &mut geometry); geometry.offset.expect("mounted preview scrollable").y @@ -114,6 +117,41 @@ impl Mounted { assert!(offset > 10., "fixture must really be scrolled"); offset } + fn wheel(&mut self, app: &App) -> Vec { + let mut geometry = Geometry { + id: scroll_id(app), + offset: None, + bounds: None, + }; + self.operate(app, &mut geometry); + let position = geometry.bounds.expect("visible preview bounds").center(); + let mut view = dialog_content(app, &self.editor); + self.tree.diff(view.as_widget()); + let node = view.as_widget_mut().layout( + &mut self.tree, + &FontLayout, + &layout::Limits::new(Size::ZERO, HOST), + ); + let mut messages = Vec::new(); + let mut shell = iced::advanced::Shell::new(&mut messages); + view.as_widget_mut().update( + &mut self.tree, + &iced::Event::Mouse(iced::mouse::Event::WheelScrolled { + delta: iced::mouse::ScrollDelta::Pixels { x: 0., y: -120. }, + }), + Layout::new(&node), + iced::mouse::Cursor::Available(position), + &FontLayout, + &mut iced::advanced::clipboard::Null, + &mut shell, + &Rectangle::with_size(HOST), + ); + assert!( + shell.is_event_captured(), + "preview must consume wheel input" + ); + messages + } async fn execute(&mut self, app: &App, task: Task) -> usize { let mut count = 0; if let Some(mut actions) = iced_runtime::task::into_stream(task) { @@ -337,6 +375,7 @@ async fn details_round_trip_retires_held_reset_without_changing_page_or_repeated let mut hidden = Geometry { id: old_preview.clone(), offset: None, + bounds: None, }; mounted.operate(&app, &mut hidden); assert!( @@ -355,8 +394,8 @@ async fn details_round_trip_retires_held_reset_without_changing_page_or_repeated let show_preview = app.update(Message::SourceRename(Event::Details(invocation, false))); assert_eq!( mounted.execute(&app, show_preview).await, - 0, - "returning does not force a scroll reset" + 1, + "returning restores the reported offset in the new preview subtree" ); let returned_visit = scroll_id(&app); assert_ne!(returned_visit, old_preview); @@ -383,3 +422,82 @@ async fn details_round_trip_retires_held_reset_without_changing_page_or_repeated assert_eq!(evidence(&app), before); } } + +#[tokio::test] +async fn details_remount_restores_wheel_offset_before_any_rescroll() { + for mount_details in [true, false] { + let mut app = paged_fixture(); + let before = evidence(&app); + let form = app.source_rename.form.as_ref().unwrap(); + let invocation = form.invocation; + let approval = form.approval.as_ref().unwrap().id; + let page = (form.file, form.hunk, form.line, form.segment); + let mut mounted = Mounted::new(&app); + let reports = mounted.wheel(&app); + assert_eq!(reports.len(), 1, "one actual preview scroll report"); + assert!(matches!( + reports[0], + Message::SourceRename(Event::PreviewScrolled(..)) + )); + let offset = mounted.offset(&app); + assert!( + offset > 10., + "real wheel input must move the overflowing fixture" + ); + for message in reports.iter().cloned() { + let task = app.update(message); + assert_eq!(mounted.execute(&app, task).await, 0); + } + assert_eq!( + app.source_rename.form.as_ref().unwrap().preview_offset, + offset + ); + + let details = app.update(Message::SourceRename(Event::Details(invocation, true))); + assert_eq!(mounted.execute(&app, details).await, 0); + if mount_details { + let mut hidden = Geometry { + id: scroll_id(&app), + offset: None, + bounds: None, + }; + mounted.operate(&app, &mut hidden); + assert!(hidden.offset.is_none(), "Details replaced the Scrollable"); + } + // A queued report from the hidden preview cannot overwrite its saved offset. + let Message::SourceRename(Event::PreviewScrolled( + old_invocation, + old_approval, + old_visit, + _, + )) = reports[0].clone() + else { + unreachable!() + }; + let stale = Event::PreviewScrolled(old_invocation, old_approval, old_visit, offset + 100.); + assert!(!app.source_rename_envelope(&stale)); + let task = app.update(Message::SourceRename(stale.clone())); + assert_eq!(mounted.execute(&app, task).await, 0); + + let returned = app.update(Message::SourceRename(Event::Details(invocation, false))); + assert_eq!(mounted.execute(&app, returned).await, 1); + // This assertion precedes any new scroll: the former test scrolled again here. + assert_eq!(mounted.offset(&app), offset); + let form = app.source_rename.form.as_ref().unwrap(); + assert_eq!((form.file, form.hunk, form.line, form.segment), page); + assert_eq!(form.approval.as_ref().unwrap().id, approval); + assert_eq!(form.preview_offset, offset); + assert!(!app.source_rename_envelope(&stale)); + let task = app.update(Message::SourceRename(stale)); + assert_eq!(mounted.execute(&app, task).await, 0); + assert_eq!(mounted.offset(&app), offset); + + let next = app.update(Message::SourceRename(Event::File( + invocation, approval, true, + ))); + assert_eq!(mounted.execute(&app, next).await, 1); + assert_eq!(mounted.offset(&app), 0.); + assert_eq!(app.source_rename.form.as_ref().unwrap().preview_offset, 0.); + assert_eq!(evidence(&app), before); + } +}