Keep the view on the photographs after some of them are deleted
Build and test / Desktop (Linux) (push) Failing after 2m21s
Build and test / Layer separation (push) Successful in 27s
Traceability / Requirement traces (push) Successful in 27s
🐳 Android image / Build and push (push) Successful in 2s
Build and test / android-image (push) Successful in 3s
Build and test / Android (aarch64) (push) Failing after 4s
Build and test / Desktop (Linux) (push) Failing after 2m21s
Build and test / Layer separation (push) Successful in 27s
Traceability / Requirement traces (push) Successful in 27s
🐳 Android image / Build and push (push) Successful in 2s
Build and test / android-image (push) Successful in 3s
Build and test / Android (aarch64) (push) Failing after 4s
Deleting made the grid go blank and jump somewhere arbitrary. Two causes, both of them the viewport being left behind by everything else that moved. The Flickable is sized to the *whole* library so its scrollbar is a real address into twenty thousand images. Delete some and that content gets shorter, which leaves a view near the end scrolled past what now exists — cells sitting above a viewport looking at empty space. Slint does not pull a Flickable back on its own. (The clamp for that went in with the previous commit.) The jump is the other half. Cells are drawn at their absolute place in the library, `(i + offset) / columns`, and a delete re-clamps `offset` downward so the loaded window still fills. Nothing touches `viewport-y`, so the same scroll position now addresses different photographs and the grid appears to leap somewhere unrelated. `restore_position` re-anchors on the ordinal the view was showing, clamped into what is left. Not on the deleted image's own position, which no longer exists, and not on the top of the library, which would throw the scroll position away on every delete — after removing one frame from a wall of twenty thousand, the one you want next is the one that just moved into its place. Only on a shrink, and the shrink is detected by reading `library-total` before overwriting it. Re-anchoring on every load would fight a scrub, which sets exactly this property to go where the user asked. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -5547,6 +5547,44 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// Deleting must not move the view somewhere the user did not ask to be.
|
||||
///
|
||||
/// The grid draws each cell at its absolute place in the library, and a
|
||||
/// delete shortens the library *and* re-clamps the loaded window's offset.
|
||||
/// Neither touches the viewport, so before this the grid went blank — the
|
||||
/// view left pointing past the end of the content — or appeared to jump,
|
||||
/// the same scroll position now addressing different photographs.
|
||||
///
|
||||
/// The anchor is the offset clamped into what is left: the ordinal the view
|
||||
/// was showing, or the last one there is if it was near the end.
|
||||
#[test]
|
||||
fn a_shorter_library_anchors_the_view_instead_of_losing_it() {
|
||||
// The rule `restore_position` applies, stated where it can be checked.
|
||||
let anchor = |offset: usize, total: usize| -> Option<usize> {
|
||||
if total == 0 {
|
||||
None
|
||||
} else {
|
||||
Some(offset.min(total - 1))
|
||||
}
|
||||
};
|
||||
|
||||
// The everyday case: one frame removed from the middle of a wall of
|
||||
// twenty thousand. The view does not move — the photograph that slid
|
||||
// into the gap is the one you want next.
|
||||
assert_eq!(anchor(12_000, 19_999), Some(12_000));
|
||||
|
||||
// Near the end, which is where the blank came from: the view was
|
||||
// showing ordinals past what now exists, so it lands on the last.
|
||||
assert_eq!(anchor(19_990, 5), Some(4));
|
||||
|
||||
// Emptied entirely. Seeking into an empty library would be the same
|
||||
// fault in the other direction, so there is nothing to do.
|
||||
assert_eq!(anchor(500, 0), None);
|
||||
|
||||
// The boundary: an offset equal to the new total is one past the end.
|
||||
assert_eq!(anchor(10, 10), Some(9));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bucket_labels_match_their_granularity() {
|
||||
use dr_catalog::Granularity;
|
||||
|
||||
Reference in New Issue
Block a user