dtourolle/DarkRoom
1
0
Fork 0
Code Issues 39 Pull Requests Actions Packages Projects Releases 20 Wiki Activity

Drop the anchor bookkeeping the double tap took with it

Browse Source 
`previous_anchor` existed for one gesture: a double tap in selection
mode took the range from where selecting began, and both taps had
already moved the anchor onto the cell being tapped, so the origin the
user meant had to be remembered separately.

That gesture is gone — "Select to…" says what it is about to do instead
of hiding a forty-image range behind a thing a hand does by accident —
and what is left is a field that four places write, `PressUndo` carries,
`cancel_press` restores, and nothing at all reads. `apply_press` is
`select_row`'s only call now that there is no anchor to remember, so the
wrapper goes with it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
dtourolle
2026-08-30 10:40:07 +02:00
co-authored by Claude Opus 5
parent 8848bbcff3
commit 1ff52102b6
2 changed files with 24 additions and 101 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ui/dr-ui/src/collections_ui.rs
+16 -93
View File
@@ -50,40 +50,37 @@ use crate::{AppWindow, CollectionRow};
struct PressUndo {
selection: BTreeSet<ImageId>,
anchor: Option<usize>,
previous_anchor: Option<usize>,
cursor: Option<usize>,
}
impl PressUndo {
/// Everything [`press_remembering_anchor`] is about to change.
/// Everything [`select_row`] is about to change.
///
/// The four are the whole of what a press touches, which is the property
/// the restore depends on and the reason it is worth a test of its own: an
/// press that grew a fifth piece of state would leave that piece behind
/// The three are the whole of what a press touches, which is the property
/// the restore depends on and the reason it is worth a test of its own: a
/// press that grew a fourth piece of state would leave that piece behind
/// after a cancel, silently.
fn capture(
selection: &BTreeSet<ImageId>,
anchor: Option<usize>,
previous_anchor: Option<usize>,
cursor: Option<usize>,
) -> Self {
Self {
selection: selection.clone(),
anchor,
previous_anchor,
cursor,
}
}
/// Put it all back. Returns the two the caller holds in `Cell`s.
/// Put it all back. Returns the one the caller holds in a `Cell`.
fn restore(
self,
selection: &mut BTreeSet<ImageId>,
anchor: &mut Option<usize>,
) -> (Option<usize>, Option<usize>) {
) -> Option<usize> {
*selection = self.selection;
*anchor = self.anchor;
(self.previous_anchor, self.cursor)
self.cursor
}
}
@@ -117,18 +114,6 @@ pub struct CollectionsController {
/// same argument applied to the one index that has to survive a move.
anchor: RefCell<Option<usize>>,
/// TRACES: FR-UI-2 | FR-UI-4
/// Where the anchor was *before* the press that moved it.
///
/// The touch equivalent of shift-click needs this. A double tap is two
/// presses, and both of them move the anchor onto the cell being tapped —
/// so by the time the double tap is reported, "the range from the anchor to
/// here" describes a single cell. This remembers the cell the user actually
/// started from, which is the one they mean.
///
/// Updated only when a press *moves* the anchor, so the second tap of a
/// double tap — which lands on the cell that is already the anchor — leaves
/// it pointing where the first tap left it.
previous_anchor: std::cell::Cell<Option<usize>>,
/// TRACES: FR-UI-4
/// What the selection was immediately before the most recent press, so a
/// gesture that turns out not to have been a press can put it back.
@@ -483,35 +468,6 @@ pub fn apply_press(
*anchor = Some(here);
}
/// TRACES: FR-UI-2 | FR-UI-4
/// Apply a press, and remember where the anchor was before it moved.
///
/// The bookkeeping a double tap depends on, split out from the callback so the
/// touch sequence — hold one cell, double-tap another, get the run between —
/// can be tested without a window. See [`CollectionsController::previous_anchor`]
/// for why the *previous* anchor is the one a double tap means.
#[allow(clippy::too_many_arguments)]
pub fn press_remembering_anchor(
selection: &mut BTreeSet<ImageId>,
anchor: &mut Option<usize>,
previous: &mut Option<usize>,
ids: &[ImageId],
offset: usize,
row: usize,
ctrl: bool,
shift: bool,
span: &dyn Fn(usize, usize) -> Vec<ImageId>,
) {
let before = *anchor;
apply_press(selection, anchor, ids, offset, row, ctrl, shift, span);
// Only a press that *moved* the anchor updates this. The second tap of a
// double tap lands on the cell the first tap made the anchor, so it changes
// nothing and the origin survives to be extended from.
if *anchor != before {
*previous = before;
}
}
/// Apply a press and push the result into the grid — the whole of what a
/// click, or an arrow key, does to the selection.
///
@@ -533,15 +489,12 @@ pub fn select_row(
*ctl.press_undo.borrow_mut() = Some(PressUndo::capture(
&ctl.selection.borrow(),
*ctl.anchor.borrow(),
ctl.previous_anchor.get(),
ctl.cursor(),
));
let mut previous = ctl.previous_anchor.get();
press_remembering_anchor(
apply_press(
&mut ctl.selection.borrow_mut(),
&mut ctl.anchor.borrow_mut(),
&mut previous,
ids,
offset,
row,
@@ -549,7 +502,6 @@ pub fn select_row(
shift,
&|first, last| ctl.span(first, last),
);
ctl.previous_anchor.set(previous);
// The cursor follows the press, so an arrow key after a click continues
// from the cell that was clicked rather than from wherever the keyboard
// was last.
@@ -571,11 +523,10 @@ pub fn cancel_press(window: &AppWindow, ctl: &Rc<CollectionsController>, ids: &[
let Some(undo) = ctl.press_undo.borrow_mut().take() else {
return;
};
let (previous_anchor, cursor) = undo.restore(
let cursor = undo.restore(
&mut ctl.selection.borrow_mut(),
&mut ctl.anchor.borrow_mut(),
);
ctl.previous_anchor.set(previous_anchor);
ctl.set_cursor(cursor);
sync_selection(window, ctl, ids);
}
@@ -2830,25 +2781,14 @@ mod tests {
let span = library(&all);
let mut sel = BTreeSet::new();
let mut anchor = None;
let mut previous = None;
// Selecting began somewhere else, so a range gesture would have had an
// anchor to sweep from.
press_remembering_anchor(
&mut sel,
&mut anchor,
&mut previous,
&all,
0,
4,
false,
false,
&span,
);
apply_press(&mut sel, &mut anchor, &all, 0, 4, false, false, &span);
assert_eq!(sel.len(), 1);
tap(&mut sel, &mut anchor, &mut previous, &all, 11);
tap(&mut sel, &mut anchor, &mut previous, &all, 11);
tap(&mut sel, &mut anchor, &all, 11);
tap(&mut sel, &mut anchor, &all, 11);
assert_eq!(
sel.iter().copied().collect::<Vec<_>>(),
@@ -2859,24 +2799,8 @@ mod tests {
/// One tap in selection mode: a press reported as ctrl-held, which is what
/// `library.slint` sends while the mode is on.
fn tap(
sel: &mut BTreeSet<ImageId>,
anchor: &mut Option<usize>,
previous: &mut Option<usize>,
all: &[ImageId],
row: usize,
) {
press_remembering_anchor(
sel,
anchor,
previous,
all,
0,
row,
true,
false,
&library(all),
);
fn tap(sel: &mut BTreeSet<ImageId>, anchor: &mut Option<usize>, all: &[ImageId], row: usize) {
apply_press(sel, anchor, all, 0, row, true, false, &library(all));
}
#[test]
@@ -2937,7 +2861,7 @@ mod tests {
let before = (sel.clone(), anchor);
// The finger that opens a pinch.
let undo = PressUndo::capture(&sel, anchor, Some(1), Some(3));
let undo = PressUndo::capture(&sel, anchor, Some(3));
apply_press(&mut sel, &mut anchor, &all, 0, 5, false, false, &span);
assert_ne!(
(sel.clone(), anchor),
@@ -2945,10 +2869,9 @@ mod tests {
"the press has to change something, or this proves nothing"
);
let (previous_anchor, cursor) = undo.restore(&mut sel, &mut anchor);
let cursor = undo.restore(&mut sel, &mut anchor);
assert_eq!((sel, anchor), before, "selection and anchor are back");
assert_eq!(previous_anchor, Some(1), "and so is the shift-click origin");
assert_eq!(cursor, Some(3), "and the keyboard cursor");
}