Let a collection be picked up, rearranged, and emptied after the fact

Collections could be made and filled and never reorganised. Nesting had
a drag; un-nesting had nothing, in either direction — "All photographs"
refused every drop, which is right for a photograph and wrong for a
collection, which has a top level to be returned to. So a collection put
inside another was in there permanently. Right-click deleted an *empty*
collection outright and refused otherwise, which is wrong in both
directions at once: destructive with no confirmation, and no way at all
to delete a collection that held anything without emptying it by hand,
child by child. And a photograph could only leave the collection the
grid was scoped to, since that is the only one a button in the header
can name — the cell's badge says a photograph is in three collections
and never which three.

Three ways in, one vocabulary:

**Hold a row.** The tree is inside a Flickable, which claims any drag
beginning inside it, so with a finger a drag on a row is a scroll until
something says otherwise. The hold is that something. It lifts the row —
drawn before anything moves, so the gesture says it has been understood
— and then what the user does decides which of two things they meant:
move, and it is a rearrangement; let go, and it is the row menu. The
same fork the grid already uses to tell hold-to-select from drag-to-file.
`decide_release` is that fork, and it is tested, because getting it
wrong one way puts a sheet over every tidied tree and the other way
makes the menu unreachable by touch.

**The row menu.** Rename, new collection inside, move to top level,
keep offline, delete. Deleting asks once when there is anything to lose
and says what survives: the photographs stay in the library, and nested
collections move up rather than going with it — which is what the
catalog does, and what a user would never assume. An empty collection
goes on the first press, because a dialogue about losing nothing is how
people learn to dismiss dialogues.

**"Collections…" on a selection.** Every collection the selection is
filed in, each with a count — "3 of 40", so nobody takes forty
photographs out of a collection thirty-seven were never in — and a way
out of any of them without navigating there first.

The long press used to open the offline question by itself. That
question is one item in this menu now: there is one hold per row, and
while it was spent on a single action nothing else the tree can do had
a touch route at all. Nothing is lost — the tray on the row keeps its
tap, and the question gains a full-width control in place of a 30px
icon in a row shorter than the touch minimum.

The row-press handler moves to `collections_ui` with the rest of what a
collection row does; it lived in `library_ui` only because it opened
that prompt.
This commit is contained in:
2026-09-07 19:59:44 +02:00
parent 577bbd82b0
commit 1e171c6d31
8 changed files with 1525 additions and 163 deletions
+794 -44
View File
@@ -206,6 +206,43 @@ pub struct CollectionsController {
/// works — the drop callback carries only the target's id, so what is being
/// dropped has to be remembered rather than inspected.
dragging_collection: RefCell<Option<CollectionId>>,
/// TRACES: FR-UI-3 | FR-UI-4
/// The collection a hold has picked up, and the timer that picks it up.
///
/// The tree is inside a Flickable, which claims any drag beginning inside
/// it — so with a finger, a drag on a row is a scroll unless something says
/// otherwise first. The hold is that something. It arms the drag *and*
/// opens the row menu, and which of the two the user gets is decided by
/// whether they then moved: the same fork the grid already uses to tell a
/// hold-to-select from a drag-to-file.
lifted: RefCell<Option<CollectionId>>,
/// Whether the lifted row's drag actually began. Reset by the press that
/// arms the next one, so a release can tell a rearrangement from a menu.
drag_began: std::cell::Cell<bool>,
/// The hold timer for a sidebar row. One, replaced per press, so a press
/// that became a scroll leaves nothing queued to fire over the list the
/// user is now scrolling.
row_hold_timer: RefCell<Option<slint::Timer>>,
/// Whether each visible row has a parent, in `row_ids` order — what decides
/// whether "All photographs" lights up as a drop target. Kept beside the
/// rows it indexes rather than queried per drag: `refresh_tree` already has
/// the parent in hand, and a query would answer for a tree that may have
/// been rebuilt since.
row_nested: RefCell<Vec<bool>>,
/// The collection the row menu is open on.
///
/// Held here as well as in the window's title property because the two say
/// different things: the property is what the sheet *draws*, and this is
/// what every action *acts on*. A rename committed from the menu changes
/// the first and must not change the second.
menu_for: RefCell<Option<CollectionId>>,
/// Whether the menu's delete has been asked once and is waiting to be
/// confirmed.
///
/// A `Cell` rather than a window property alone so the decision is made in
/// Rust: the sheet must not be able to reach the destructive branch by
/// flipping a bit of its own.
menu_confirming: std::cell::Cell<bool>,
/// The collection a drag is currently over, by id.
///
/// Only the spring needs this — the *drop* is hit-tested by Slint and
@@ -289,6 +326,118 @@ pub fn decide_drop(
}
}
/// TRACES: FR-CAT-7
/// What the row menu's Delete should do on this press.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum DeleteStep {
/// Delete it now. Nothing is lost that the user cannot see is nothing.
Now,
/// Say what goes, and wait to be asked again.
Confirm,
}
/// TRACES: FR-CAT-7
/// Whether deleting this collection needs to be confirmed first.
///
/// The gesture this replaced refused outright whenever the collection held
/// anything, which made a full collection undeletable without emptying it by
/// hand — child by child, since a parent counts its descendants. Refusing is
/// not a safety property; it is the absence of one, because the user goes and
/// does the same thing the long way round.
///
/// So: an empty collection goes on the first press, because there is nothing
/// to warn about and a dialogue asking "delete this empty thing?" is the kind
/// that teaches people to dismiss dialogues. Anything else is asked once.
///
/// Split from the handler because this is the whole of the safety rule, and a
/// handler needs a window to run.
pub fn decide_delete(holds: usize, children: usize, confirmed: bool) -> DeleteStep {
if confirmed || (holds == 0 && children == 0) {
DeleteStep::Now
} else {
DeleteStep::Confirm
}
}
/// TRACES: FR-CAT-7
/// The line under the menu's title: what this collection directly holds.
///
/// Direct members and direct children, not the sidebar's deep count, because
/// this line sits above a Delete — and delete drops *this* collection's member
/// rows while promoting its children rather than taking them. A number here
/// that counted descendants' photographs would be describing something the
/// button below it does not do.
pub fn menu_detail(holds: usize, children: usize) -> String {
let photos = match holds {
0 => "no photographs".to_string(),
1 => "1 photograph".to_string(),
n => format!("{n} photographs"),
};
match children {
0 => photos,
1 => format!("{photos} · 1 collection inside"),
n => format!("{photos} · {n} collections inside"),
}
}
/// TRACES: FR-CAT-7
/// What deleting this collection actually does, in full.
///
/// Every clause here is one a user has got wrong about a collections feature
/// before: that deleting a collection deletes the photographs (it does not —
/// membership is a join table), and that deleting a parent takes the
/// collections nested in it (it does not — [`dr_catalog::collections::delete`]
/// promotes them, because losing a subtree because its container was tidied
/// away is not recoverable).
pub fn delete_warning(name: &str, holds: usize, children: usize) -> String {
let mut out = format!("Deleting “{name}” ");
match (holds, children) {
(0, _) => out.push_str("removes it from the sidebar."),
(1, _) => out.push_str("takes 1 photograph out of it."),
(n, _) => out.push_str(&format!("takes {n} photographs out of it.")),
}
if holds > 0 {
out.push_str(" They stay in your library and in every other collection they are in.");
}
match children {
0 => {}
1 => {
out.push_str(" The collection inside it moves up one level rather than going with it.")
}
n => out.push_str(&format!(
" The {n} collections inside it move up one level rather than going with it."
)),
}
out
}
/// TRACES: FR-UI-3 | FR-UI-4
/// What letting go of a held collection row means.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum Release {
/// The hold fired and nothing moved: the user is asking what can be done
/// to this collection.
OpenMenu(CollectionId),
/// Either the hold never fired — an ordinary tap, which selects — or it
/// did and the row was then dragged, in which case the drop has already
/// done the work.
Nothing,
}
/// TRACES: FR-UI-3 | FR-UI-4
/// Decide what a release does, from what the press turned into.
///
/// The whole of the fork that lets one gesture mean two things, and the reason
/// it is worth a test: get it wrong in the direction of `OpenMenu` and every
/// rearrangement ends with a sheet over the tree the user just tidied; get it
/// wrong the other way and the menu is unreachable with a finger.
pub fn decide_release(held: Option<CollectionId>, dragged: bool) -> Release {
match held {
Some(id) if !dragged => Release::OpenMenu(id),
_ => Release::Nothing,
}
}
impl CollectionsController {
pub fn new(activity: Rc<crate::activity::ActivityLog>) -> Rc<Self> {
Rc::new(Self {
@@ -645,6 +794,7 @@ pub fn refresh_tree(window: &AppWindow, ctl: &Rc<CollectionsController>, catalog
let mut ids = Vec::new();
let mut smart = Vec::new();
let mut has_kids = Vec::new();
let mut nested = Vec::new();
let mut out = Vec::new();
for row in &rows {
@@ -671,6 +821,7 @@ pub fn refresh_tree(window: &AppWindow, ctl: &Rc<CollectionsController>, catalog
ids.push(id);
smart.push(row.collection.kind == CollectionKind::Smart);
has_kids.push(row.has_children);
nested.push(row.collection.parent.is_some());
out.push(CollectionRow {
id: id.0 as i32,
name: row.collection.name.as_str().into(),
@@ -692,6 +843,7 @@ pub fn refresh_tree(window: &AppWindow, ctl: &Rc<CollectionsController>, catalog
*ctl.row_ids.borrow_mut() = ids;
*ctl.row_smart.borrow_mut() = smart;
*ctl.row_has_children.borrow_mut() = has_kids;
*ctl.row_nested.borrow_mut() = nested;
window.set_collection_rows(slint::ModelRc::new(slint::VecModel::from(out)));
// The rows are what `sync_reorderable` reads, so it has to be told they
@@ -2417,36 +2569,9 @@ pub fn wire<S, R, P, C>(
};
// Created inside whatever is selected, which is how a hierarchy
// gets built without a separate "new child" command: select the
// parent, press +.
let parent = *ctl.scope.borrow();
let name = unique_name(cat.connection(), parent);
match coll::create(cat.connection(), &name, parent, CollectionKind::Manual) {
Ok(id) => {
w.set_collection_error(slint::SharedString::new());
// A new child is useless if its parent is collapsed.
if let Some(p) = parent {
ctl.collapsed.borrow_mut().remove(&p);
}
// Straight into the name field, with "New collection"
// selected. The name is a placeholder nobody wants to
// keep, so making the user find the rename gesture
// afterwards is asking them to finish a job we started.
//
// Opened *after* the rebuild, and the order is load-bearing:
// `refresh_tree` replaces the row model, which destroys and
// recreates every row. A field opened before it would be
// torn down along with the `init` that focuses it, leaving
// an edit box nothing had typed into. Setting the property
// afterwards puts the field on a row that already exists.
refresh_tree(&w, &ctl, cat);
*ctl.renaming.borrow_mut() = Some(id);
w.set_collection_renaming(id.0 as i32);
log::info!("created collection {} ({name})", id.0);
}
Err(e) => w.set_collection_error(format!("creating collection: {e}").into()),
}
// gets built without leaving the header: select the parent, press
// +. The row menu aims the same act at a row instead.
create_child(&w, &ctl, cat, *ctl.scope.borrow());
});
}
@@ -2573,44 +2698,547 @@ pub fn wire<S, R, P, C>(
});
}
// Right-click. A real context menu needs a popup with keyboard handling and
// a rename field; until that exists the gesture deletes an *empty*
// collection, which is the one destructive action safe without a
// confirmation dialog, and says why when it declines.
// TRACES: FR-CAT-7 | FR-UI-2 | FR-UI-3 | FR-UI-4
// The hold on a sidebar row: it arms the drag that rearranges the tree,
// and it opens the row menu. Which one the user gets is decided on release
// by whether they moved.
//
// Both from one gesture because there is only one to spend. A finger has
// no right button, and this tree lives in a Flickable that claims any drag
// beginning inside it — so without a hold, a touch drag on a row is a
// scroll, and arranging the tree is a pointer-only feature. The row lifts
// the moment the timer fires, which is what tells the user the next
// movement will carry the collection rather than scroll past it.
{
let weak = window.as_weak();
let ctl = ctl.clone();
window.on_collection_row_press(move |id, down| {
let Some(w) = weak.upgrade() else { return };
let id = CollectionId(id as u64);
if !down {
// Let go. A drag that began has already been dealt with by the
// drop; a hold that did not move is a request for the menu.
*ctl.row_hold_timer.borrow_mut() = None;
let held = ctl.lifted.borrow_mut().take();
let dragged = ctl.drag_began.replace(false);
w.set_collection_lifted(0);
if let Release::OpenMenu(held) = decide_release(held, dragged) {
w.invoke_collection_menu(held.0 as i32);
}
return;
}
// A drag of this row, if one follows, carries this collection.
ctl.note_row_press(id);
ctl.drag_began.set(false);
let timer = slint::Timer::default();
let weak = w.as_weak();
let ctl_cb = ctl.clone();
timer.start(
slint::TimerMode::SingleShot,
std::time::Duration::from_millis(HOLD_DELAY_MS),
move || {
let Some(w) = weak.upgrade() else { return };
// Picked up, not yet acted on. The menu waits for the
// release, because opening it here would put a sheet over
// the tree the user may be about to drag this row across.
*ctl_cb.lifted.borrow_mut() = Some(id);
w.set_collection_lifted(id.0 as i32);
},
);
*ctl.row_hold_timer.borrow_mut() = Some(timer);
});
}
// TRACES: FR-CAT-7 | FR-UI-4
// A row's drag crossed the threshold, or ended.
//
// This is what turns a hold into a rearrangement: once it has fired, the
// release that follows opens no menu. It also decides whether
// "All photographs" will accept the drop, which only makes sense for a
// collection that has a parent to be taken out of.
{
let weak = window.as_weak();
let ctl = ctl.clone();
window.on_collection_drag_active(move |id, active| {
let Some(w) = weak.upgrade() else { return };
let id = CollectionId(id as u64);
if !active {
w.set_collection_root_drop_allowed(false);
w.set_collection_lifted(0);
*ctl.lifted.borrow_mut() = None;
return;
}
// The hold may not have fired — a pointer drag needs no hold — so
// the pick-up is recorded here as well as there, and the row lifts
// either way.
ctl.drag_began.set(true);
ctl.note_row_press(id);
*ctl.lifted.borrow_mut() = Some(id);
w.set_collection_lifted(id.0 as i32);
// The hold has been spent on a drag; nothing may fire behind it.
*ctl.row_hold_timer.borrow_mut() = None;
// Only a nested collection has a top level to be returned to, and
// only a collection drag — never photographs — means anything
// there at all.
let carrying_images = !ctl.dragging.borrow().is_empty();
let nested = ctl
.row_ids
.borrow()
.iter()
.position(|&c| c == id)
.and_then(|row| ctl.row_nested.borrow().get(row).copied())
.unwrap_or(false);
w.set_collection_root_drop_allowed(nested && !carrying_images);
});
}
// TRACES: FR-CAT-7
// Dropped on "All photographs": out to the top level.
//
// The gestural inverse of dropping one row onto another. `can-drop` has
// already refused everything but a nested collection, so reaching here
// means the drop is one — but the payload is taken rather than read, for
// the reason the row's own drop gives: a remembered press must be spent by
// the drop it belongs to, or a later empty drop moves a collection nobody
// touched.
{
let weak = window.as_weak();
let ctl = ctl.clone();
let catalog = catalog.clone();
window.on_collection_dropped_on_root(move || {
let Some(w) = weak.upgrade() else { return };
let source = ctl.dragging_collection.borrow_mut().take();
let Some(source) = source else { return };
// Photographs cannot land here. Guarded as well as refused in
// `can-drop`, so a payload that somehow arrives does nothing
// rather than reparenting whatever row was last pressed.
if !ctl.dragging.borrow().is_empty() {
return;
}
let borrow = catalog.borrow();
let Some(cat) = borrow.as_ref() else { return };
let moved = coll::set_parent(cat.connection(), source, None);
match moved {
Ok(()) => {
w.set_collection_error(slint::SharedString::new());
w.set_library_status("Moved to the top level".into());
refresh_tree(&w, &ctl, cat);
log::info!("promoted collection {} by drag", source.0);
}
Err(e) => w.set_collection_error(format!("moving collection: {e}").into()),
}
});
}
// --- the row menu ------------------------------------------------------
//
// Right-click, or a long press on touch. This gesture used to *delete an
// empty collection outright* and refuse with an error message otherwise,
// which was wrong in both directions at once: the destructive half fired
// with no confirmation and nothing on screen said it would, and the
// refusing half meant a collection holding anything could not be deleted
// at all — the user emptied it by hand and then did the same thing.
{
let weak = window.as_weak();
let ctl = ctl.clone();
let catalog = catalog.clone();
window.on_collection_menu(move |id| {
let Some(w) = weak.upgrade() else { return };
let borrow = catalog.borrow();
let Some(cat) = borrow.as_ref() else { return };
open_row_menu(&w, &ctl, cat, CollectionId(id as u64));
});
}
// Rename, from the menu into the row's own inline field.
//
// The menu closes first and the field opens second, and the order is
// load-bearing for the same reason it is in `collection-new`: the field is
// focused by an `init` on a row, and a row torn down by a model rebuild
// takes the focus with it. Here the tear-down is the sheet's, and a field
// opened behind a sheet that is still up is a field the user cannot see
// they are typing into.
{
let weak = window.as_weak();
let ctl = ctl.clone();
window.on_collection_menu_rename(move || {
let Some(w) = weak.upgrade() else { return };
let Some(id) = *ctl.menu_for.borrow() else {
return;
};
close_row_menu(&w, &ctl);
*ctl.renaming.borrow_mut() = Some(id);
w.set_collection_renaming(id.0 as i32);
});
}
// A new collection nested inside this one.
//
// The `+` in the header already creates inside whatever the grid is scoped
// to, which builds a hierarchy only if you first go and look at the parent.
// This is the same act aimed at a row, so a tree can be built from the tree.
{
let weak = window.as_weak();
let ctl = ctl.clone();
let catalog = catalog.clone();
window.on_collection_menu_new_child(move || {
let Some(w) = weak.upgrade() else { return };
let Some(parent) = *ctl.menu_for.borrow() else {
return;
};
let borrow = catalog.borrow();
let Some(cat) = borrow.as_ref() else { return };
close_row_menu(&w, &ctl);
create_child(&w, &ctl, cat, Some(parent));
});
}
// TRACES: FR-CAT-7
// Un-nesting: back out to the top level.
//
// Nesting has had a gesture since collections landed — drag a row onto
// another row — and its inverse had none, in either direction: "All
// photographs" ignores drops on purpose (an image is already in the
// library), and no menu existed to ask. So a collection dragged into
// another was in there permanently, and the only way out was to delete it
// and build it again.
{
let weak = window.as_weak();
let ctl = ctl.clone();
let catalog = catalog.clone();
window.on_collection_menu_promote(move || {
let Some(w) = weak.upgrade() else { return };
let Some(id) = *ctl.menu_for.borrow() else {
return;
};
let borrow = catalog.borrow();
let Some(cat) = borrow.as_ref() else { return };
match coll::set_parent(cat.connection(), id, None) {
Ok(()) => {
w.set_collection_error(slint::SharedString::new());
w.set_library_status("Moved to the top level".into());
close_row_menu(&w, &ctl);
refresh_tree(&w, &ctl, cat);
log::info!("promoted collection {} to the top level", id.0);
}
// Left open on failure, showing the collection it failed on:
// closing would leave a red line referring to a row the user
// can no longer tell was the one they aimed at.
Err(e) => w.set_collection_error(format!("moving collection: {e}").into()),
}
});
}
// TRACES: FR-NC-6a
// The offline question, handed to the handler the tray on the row already
// uses. Routed through the window's own callback rather than reaching into
// `library_ui`'s prompt directly: that module owns the transfer state the
// prompt reads, and a second entry point into it is a second place for the
// two to disagree about what is downloading.
{
let weak = window.as_weak();
let ctl = ctl.clone();
window.on_collection_menu_offline(move || {
let Some(w) = weak.upgrade() else { return };
let Some(id) = *ctl.menu_for.borrow() else {
return;
};
close_row_menu(&w, &ctl);
w.invoke_collection_offline_menu(id.0 as i32);
});
}
// TRACES: FR-CAT-7
// Delete, in one press or two — see [`decide_delete`].
{
let weak = window.as_weak();
let ctl = ctl.clone();
let catalog = catalog.clone();
let reload = on_scope_changed.clone();
window.on_collection_menu(move |id| {
let visible = visible_ids.clone();
window.on_collection_menu_delete(move || {
let Some(w) = weak.upgrade() else { return };
let Some(id) = *ctl.menu_for.borrow() else {
return;
};
let borrow = catalog.borrow();
let Some(cat) = borrow.as_ref() else { return };
let id = CollectionId(id as u64);
let holds = coll::deep_count(cat.connection(), id).unwrap_or(1);
if holds > 0 {
w.set_collection_error(
format!("{holds} photograph(s) in there — empty it first.").into(),
);
let (holds, children) = direct_holdings(cat.connection(), id).unwrap_or((0, 0));
if decide_delete(holds, children, ctl.menu_confirming.get()) == DeleteStep::Confirm {
// Asked once. The sheet swaps its other actions for the
// warning, so the second press cannot be a mis-aimed first one.
ctl.menu_confirming.set(true);
w.set_collection_menu_confirming(true);
w.set_collection_menu_delete_label("Delete anyway".into());
return;
}
match coll::delete(cat.connection(), id) {
// Hoisted out of the `match` rather than called in the scrutinee:
// the borrow `cat` comes from lives as long as the match does, and
// the arm below has to release it before rereading the grid.
let deleted = coll::delete(cat.connection(), id);
match deleted {
Ok(()) => {
w.set_collection_error(slint::SharedString::new());
if *ctl.scope.borrow() == Some(id) {
close_row_menu(&w, &ctl);
// The grid was showing what no longer exists.
let was_scope = *ctl.scope.borrow() == Some(id);
if was_scope {
*ctl.scope.borrow_mut() = None;
w.set_collection_selected(0);
w.set_collection_scope_label(slint::SharedString::new());
reload();
}
refresh_tree(&w, &ctl, cat);
// Named as a membership change, as the removal button is:
// a user who reads this as a delete of their photographs
// will not trust the feature again.
w.set_library_status(if holds > 0 {
format!(
"Deleted the collection; its {holds} photograph(s) stay in the library"
)
.into()
} else {
slint::SharedString::from("Deleted the collection")
});
log::info!("deleted collection {}", id.0);
// The badge on every visible cell just lost a collection,
// and a scope that has gone needs the grid rereading.
let cells = visible();
sync_badges(&w, cat, &cells);
drop(borrow);
if was_scope {
reload();
}
}
Err(e) => w.set_collection_error(format!("deleting: {e}").into()),
}
});
}
// Dismiss. A pending confirmation is backed out of one step rather than
// closing the whole menu: Escape from "are you sure" means "no", and
// taking the sheet away with it would leave the user unsure whether the
// key had cancelled the delete or performed it.
{
let weak = window.as_weak();
let ctl = ctl.clone();
window.on_collection_menu_dismiss(move || {
let Some(w) = weak.upgrade() else { return };
if ctl.menu_confirming.get() {
ctl.menu_confirming.set(false);
w.set_collection_menu_confirming(false);
w.set_collection_menu_delete_label("Delete".into());
return;
}
close_row_menu(&w, &ctl);
});
}
// --- where the selection is filed --------------------------------------
{
let weak = window.as_weak();
let ctl = ctl.clone();
let catalog = catalog.clone();
window.on_library_open_membership(move || {
let Some(w) = weak.upgrade() else { return };
let borrow = catalog.borrow();
let Some(cat) = borrow.as_ref() else { return };
refresh_membership(&w, &ctl, cat);
w.set_membership_open(true);
});
}
{
let weak = window.as_weak();
let ctl = ctl.clone();
let catalog = catalog.clone();
let visible = visible_ids.clone();
let reload = on_scope_changed.clone();
window.on_membership_remove(move |id| {
let Some(w) = weak.upgrade() else { return };
let target = CollectionId(id as u64);
let chosen = ctl.selected();
if chosen.is_empty() {
return;
}
let borrow = catalog.borrow();
let Some(cat) = borrow.as_ref() else { return };
// Hoisted for the same reason the delete above is: the arm
// releases the catalog borrow before it rereads the grid.
let removed = coll::remove_images(cat.connection(), target, &chosen);
match removed {
Ok(n) => {
w.set_collection_error(slint::SharedString::new());
// The same wording the scoped button uses, and for the same
// reason: this is a membership change, not a delete.
w.set_library_status(
format!("Removed {n} from that collection; still in the library").into(),
);
// The sheet stays open. Taking photographs out of several
// collections is one job, and a sheet that closed after
// each would have to be reopened — with the selection
// still live — to finish it.
refresh_membership(&w, &ctl, cat);
refresh_tree(&w, &ctl, cat);
let cells = visible();
sync_badges(&w, cat, &cells);
// Only the collection on screen changes what the grid
// holds. Removing from another one leaves the grid right,
// and rereading it would scroll the user's place away.
let showing = *ctl.scope.borrow() == Some(target);
drop(borrow);
if showing {
reload();
}
}
Err(e) => w.set_collection_error(format!("removing: {e}").into()),
}
});
}
{
let weak = window.as_weak();
window.on_membership_dismiss(move || {
let Some(w) = weak.upgrade() else { return };
w.set_membership_open(false);
});
}
}
/// TRACES: FR-CAT-7
/// What a collection directly holds: its own member rows, and its own children.
///
/// One query each rather than reusing `deep_count`, which counts descendants'
/// photographs — see [`menu_detail`] for why the menu needs the direct numbers.
fn direct_holdings(
conn: &rusqlite::Connection,
id: CollectionId,
) -> Result<(usize, usize), dr_catalog::CatalogError> {
let holds: i64 = conn.query_row(
"SELECT count(*) FROM collection_members WHERE collection_id = ?1",
[id.0 as i64],
|r| r.get(0),
)?;
let children: i64 = conn.query_row(
"SELECT count(*) FROM collections WHERE parent_id = ?1 AND deleted = 0",
[id.0 as i64],
|r| r.get(0),
)?;
Ok((holds as usize, children as usize))
}
/// TRACES: FR-CAT-7
/// Open the row menu on `id`, or leave it shut if the collection is gone.
///
/// Everything the sheet draws is computed here, in one place, so the menu
/// cannot offer "Move to top level" on a collection already at the top, or a
/// delete warning about children it no longer has. Reopened from scratch after
/// every action for the same reason.
fn open_row_menu(
window: &AppWindow,
ctl: &Rc<CollectionsController>,
catalog: &Catalog,
id: CollectionId,
) {
let conn = catalog.connection();
let row: Option<(String, Option<i64>, i64)> = conn
.query_row(
"SELECT name, parent_id, kind FROM collections WHERE id = ?1 AND deleted = 0",
[id.0 as i64],
|r| Ok((r.get(0)?, r.get(1)?, r.get(2)?)),
)
.optional()
.unwrap_or(None);
let Some((name, parent, kind)) = row else {
// Deleted underneath us — by a merge, or by the action that just ran.
close_row_menu(window, ctl);
return;
};
let (holds, children) = direct_holdings(conn, id).unwrap_or((0, 0));
*ctl.menu_for.borrow_mut() = Some(id);
ctl.menu_confirming.set(false);
window.set_collection_menu_confirming(false);
window.set_collection_menu_title(name.as_str().into());
window.set_collection_menu_detail(menu_detail(holds, children).into());
window.set_collection_menu_nested(parent.is_some());
window.set_collection_menu_smart(kind == CollectionKind::Smart as i64);
window.set_collection_menu_confirm_detail(delete_warning(&name, holds, children).into());
window.set_collection_menu_delete_label("Delete".into());
// TRACES: FR-NC-6a
// Read off the row the sidebar already drew rather than recomputed: the
// roll-up is one pass over the whole tree, and a menu that computed its own
// could disagree with the tray beside the name it is titled with.
let pinned = ctl
.row_ids
.borrow()
.iter()
.position(|&c| c == id)
.and_then(|row| window.get_collection_rows().row_data(row))
.is_some_and(|r| r.pinned);
window.set_collection_menu_pinned(pinned);
}
/// Shut the row menu, both halves.
///
/// The controller's copy is what the handlers act on and the window property is
/// what draws the sheet; clearing one without the other leaves either an
/// invisible menu that still answers, or a visible one that acts on nothing.
fn close_row_menu(window: &AppWindow, ctl: &Rc<CollectionsController>) {
*ctl.menu_for.borrow_mut() = None;
ctl.menu_confirming.set(false);
window.set_collection_menu_title(slint::SharedString::new());
window.set_collection_menu_confirming(false);
}
/// TRACES: FR-CAT-7 | FR-UI-4
/// Rebuild the membership sheet from the selection as it stands.
///
/// Called on open and after every removal rather than patched: a removal can
/// take the last selected photograph out of a collection, and that row then has
/// to go — a patched model would leave a "Remove" that removes nothing.
fn refresh_membership(window: &AppWindow, ctl: &Rc<CollectionsController>, catalog: &Catalog) {
let chosen = ctl.selected();
let total = chosen.len();
let rows = match coll::membership_of(catalog.connection(), &chosen) {
Ok(r) => r,
Err(e) => {
window.set_collection_error(format!("reading collections: {e}").into());
Vec::new()
}
};
let out: Vec<crate::MembershipRow> = rows
.into_iter()
.map(|m| crate::MembershipRow {
id: m.id.0 as i32,
name: m.name.as_str().into(),
holding: m.holding as i32,
// "1 of 1" is arithmetic nobody asked for; with one photograph
// selected the name already says everything true about it.
detail: if total <= 1 {
slint::SharedString::new()
} else {
format!("{} of {total}", m.holding).into()
},
})
.collect();
window.set_membership_rows(slint::ModelRc::new(slint::VecModel::from(out)));
}
/// Close the rename field, whatever the outcome.
@@ -2697,6 +3325,48 @@ fn apply_rename(
Ok(Rename::Applied(name.to_string()))
}
/// TRACES: FR-CAT-7
/// Make a collection inside `parent` and open its name field.
///
/// Shared by the header's `+` and the row menu's "New collection inside",
/// which differ only in where the parent comes from — and must not differ in
/// anything else, or building a tree from the tree would behave unlike
/// building one from the header.
fn create_child(
window: &AppWindow,
ctl: &Rc<CollectionsController>,
catalog: &Catalog,
parent: Option<CollectionId>,
) {
let name = unique_name(catalog.connection(), parent);
match coll::create(catalog.connection(), &name, parent, CollectionKind::Manual) {
Ok(id) => {
window.set_collection_error(slint::SharedString::new());
// A new child is useless if its parent is collapsed.
if let Some(p) = parent {
ctl.collapsed.borrow_mut().remove(&p);
}
// Straight into the name field, with "New collection" selected.
// The name is a placeholder nobody wants to keep, so making the
// user find the rename gesture afterwards is asking them to finish
// a job we started.
//
// Opened *after* the rebuild, and the order is load-bearing:
// `refresh_tree` replaces the row model, which destroys and
// recreates every row. A field opened before it would be torn down
// along with the `init` that focuses it, leaving an edit box
// nothing had typed into. Setting the property afterwards puts the
// field on a row that already exists.
refresh_tree(window, ctl, catalog);
*ctl.renaming.borrow_mut() = Some(id);
window.set_collection_renaming(id.0 as i32);
log::info!("created collection {} ({name})", id.0);
}
Err(e) => window.set_collection_error(format!("creating collection: {e}").into()),
}
}
/// A name no sibling is already using.
///
/// Duplicate names are legal in the schema, and two identically-named
@@ -3719,4 +4389,84 @@ mod tests {
let top = coll::create(c, "New collection", None, CollectionKind::Manual).unwrap();
assert_eq!(unique_name(c, Some(top)), "New collection");
}
#[test]
fn an_empty_collection_is_deleted_without_being_asked_about() {
// There is nothing to warn about, and a dialogue asking "delete this
// empty thing?" is the kind that teaches people to dismiss dialogues
// without reading them — including the one that mattered.
assert_eq!(decide_delete(0, 0, false), DeleteStep::Now);
}
#[test]
fn a_collection_holding_anything_is_asked_about_once() {
assert_eq!(decide_delete(12, 0, false), DeleteStep::Confirm);
assert_eq!(decide_delete(0, 1, false), DeleteStep::Confirm);
// And once asked, it goes.
assert_eq!(decide_delete(12, 3, true), DeleteStep::Now);
}
#[test]
fn a_collection_holding_only_children_still_asks() {
// The old gesture refused on `deep_count > 0`, which counted
// descendants' photographs — so an empty parent of empty children was
// deletable in one press and an empty parent of a *full* child was not
// deletable at all. Both are now the same question, asked once.
assert_eq!(decide_delete(0, 2, false), DeleteStep::Confirm);
}
#[test]
fn the_menu_line_counts_what_delete_acts_on() {
assert_eq!(menu_detail(0, 0), "no photographs");
assert_eq!(menu_detail(1, 0), "1 photograph");
assert_eq!(menu_detail(12, 1), "12 photographs · 1 collection inside");
assert_eq!(menu_detail(0, 3), "no photographs · 3 collections inside");
}
#[test]
fn the_warning_says_the_photographs_survive() {
// The single most likely misreading of "Delete" here. If this clause
// ever goes, a user deletes a collection expecting to lose the
// pictures — and either panics, or does not do it at all.
let w = delete_warning("Iceland", 12, 0);
assert!(w.contains("12 photographs"), "{w}");
assert!(w.contains("stay in your library"), "{w}");
}
#[test]
fn the_warning_says_nested_collections_are_promoted_not_taken() {
// `dr_catalog::collections::delete` promotes children to the deleted
// collection's parent rather than cascading. A warning that did not
// say so would describe a data loss that does not happen.
let w = delete_warning("Trips", 0, 2);
assert!(w.contains("2 collections inside it move up"), "{w}");
// And nothing about photographs surviving, because none were lost.
assert!(!w.contains("stay in your library"), "{w}");
}
#[test]
fn a_hold_that_did_not_move_opens_the_menu() {
assert_eq!(
decide_release(Some(CollectionId(4)), false),
Release::OpenMenu(CollectionId(4))
);
}
#[test]
fn a_hold_that_became_a_drag_opens_nothing() {
// The drop has already done the work. A menu here would put a sheet
// over the tree the user has just finished rearranging — and over the
// row they would have to look at to see whether it worked.
assert_eq!(
decide_release(Some(CollectionId(4)), true),
Release::Nothing
);
}
#[test]
fn an_ordinary_tap_opens_nothing() {
// Nothing was ever picked up, so this is the press that selects a
// collection and scopes the grid. A menu on every tap would make the
// sidebar unusable.
assert_eq!(decide_release(None, false), Release::Nothing);
}
}
+35
View File
@@ -113,6 +113,34 @@ pub const GESTURES: &[Gesture] = &[
pointer: "Click the ring at the head of its row",
keys: "",
},
Gesture {
title: "Pick a collection up to rearrange the tree",
section: "Collections sidebar",
touch: "Press and hold it until it lifts, then drag it",
pointer: "Drag it, or hold it until it lifts and then drag",
keys: "",
},
Gesture {
title: "Act on a collection — rename, nest, un-nest, delete",
section: "Collections sidebar",
touch: "Press and hold the collection, then let go without moving",
pointer: "Right-click it",
keys: "",
},
Gesture {
title: "Take a collection back out of the one it is nested in",
section: "Collections sidebar",
touch: "Hold it, then drag it onto \"All photographs\" — or let go and choose \"Move to top level\"",
pointer: "Drag it onto \"All photographs\", or right-click it and choose \"Move to top level\"",
keys: "",
},
Gesture {
title: "Rename a collection",
section: "Collections sidebar",
touch: "Hold the collection, then \"Rename\"",
pointer: "Double-click its name, or right-click it and choose \"Rename\"",
keys: "",
},
Gesture {
title: "Pull a face out of the wrong person",
section: "People",
@@ -232,4 +260,11 @@ pub const GESTURES: &[Gesture] = &[
pointer: "While selecting, press \"Select all\"",
keys: "",
},
Gesture {
title: "Take photographs out of a collection",
section: "Library grid",
touch: "Select them, then \"Collections…\" in the selection bar",
pointer: "Select them, then \"Collections…\" in the selection bar",
keys: "",
},
];
+5 -44
View File
@@ -357,11 +357,6 @@ pub struct LibraryController {
/// subject back out of a string property would act on whatever the sidebar
/// had been rebuilt to say since.
offline_target: std::cell::Cell<Option<dr_types::CollectionId>>,
/// TRACES: FR-NC-6a | FR-UI-2
/// The timer that turns a held sidebar row into that question. Dropped on
/// release, so a tap — or a press the Flickable takes for a scroll — is not
/// a dialogue a moment later.
row_hold_timer: RefCell<Option<slint::Timer>>,
/// Narrow the grid to images whose original is stored locally.
///
/// A `Cell` beside `filter` rather than a field inside it: the rating
@@ -455,7 +450,6 @@ impl LibraryController {
pending_anchor: std::cell::Cell::new(None),
pin_timer: RefCell::new(None),
offline_target: std::cell::Cell::new(None),
row_hold_timer: RefCell::new(None),
local_only: std::cell::Cell::new(false),
// The catalog's own floor until the settings page reports what the
// user has stored, which it does at startup before any fetch.
@@ -6594,48 +6588,15 @@ pub fn wire<F>(
let ctl = ctl.clone();
window.on_collection_offline_menu(move |id| {
let Some(w) = weak.upgrade() else { return };
*ctl.row_hold_timer.borrow_mut() = None;
open_offline_prompt(&w, &ctl, dr_types::CollectionId(id as u64));
});
}
// TRACES: FR-NC-6a | FR-UI-2 | FR-UI-4
// And the touch way in: hold the collection's name.
//
// The release that ends the hold still reaches the row's `clicked` and
// scopes the grid to that collection. Left deliberately: the user is now
// looking at the photographs they are being asked about, which is context
// rather than a side effect — and suppressing it would mean a second
// "ignore the next click" flag threaded through the sidebar for no gain.
{
let weak = window.as_weak();
let ctl = ctl.clone();
let coll_for_press = coll_ctl.clone();
window.on_collection_row_press(move |id, down| {
let Some(w) = weak.upgrade() else { return };
// A drag of this row, if one follows, carries this collection.
if down {
coll_for_press.note_row_press(dr_types::CollectionId(id as u64));
}
if !down {
*ctl.row_hold_timer.borrow_mut() = None;
return;
}
let timer = slint::Timer::default();
let weak = w.as_weak();
let ctl_cb = ctl.clone();
timer.start(
slint::TimerMode::SingleShot,
std::time::Duration::from_millis(crate::collections_ui::HOLD_DELAY_MS),
move || {
let Some(w) = weak.upgrade() else { return };
open_offline_prompt(&w, &ctl_cb, dr_types::CollectionId(id as u64));
},
);
*ctl.row_hold_timer.borrow_mut() = Some(timer);
});
}
// The hold on a sidebar row — which arms the drag that rearranges the tree
// and opens the row menu — is wired in `collections_ui`, with the rest of
// what a collection row does. It used to be here because it opened the
// offline question and nothing else; that question is now one item in that
// menu, so the gesture belongs with the menu rather than with the transfers.
// The three answers.
{