Make the People screen a place work can be done
Five faults, all on one screen, and the Slint and Rust halves of each have to land together. **Regroup froze the window.** It ran inside the Slint callback, on the UI thread. It is much faster now, but fast is not bounded — the work grows with the library, and the one thing that must not grow with the library is how long the window stops answering. It runs on a worker thread with an mpsc channel and a 250ms poll, like every other long pass in this module, and the button says what it is doing instead of the window going quiet. Cancellation is dropping the receiver. Reclustering also prunes the empty groups the previous pass left, so pressing the button twice no longer fills the rail with "Unnamed (0 faces)". **The faces were a single row running off the screen.** The comment on the layout claimed to be a wrapping row; Slint has no flow layout and a HorizontalLayout does not wrap, so a person with forty faces was a person whose faces could not be reviewed past the fifth. It is now laid out the way the library grid lays out thumbnails, with the same arithmetic: choose how many columns of roughly the requested size fit, then divide the width between them so the cells fill the row exactly and nothing overhangs. **The header did not fit a phone.** A 240px name field beside five buttons is wider than an Android screen — and worse than not fitting, a layout cannot be narrower than its children's minimums, so the row reported that oversized minimum upwards and inflated the whole screen. The faces grid is its sibling, so it would have been measured against a width that was never on the display. The header is now two rows, the actions sit in a Flickable that scrolls rather than overflowing, and the rail narrows to 132px on the compact class. **Strangers crowded out the people who matter.** Most clusters in a real library are passers-by and other people's guests. "Not interested" sets a group aside; the rail hides it and says how many are hidden, with one button to bring them back. Reversible, and never a deletion — see the catalog commit for why. **A face was a dead end.** Identifying someone and then having no way to see their photographs is a filing cabinet with no drawer handles. "Show photos" narrows the library grid to that person and leaves a chip on the filter bar saying so, which is also how it is cleared. It is a term on `RatingFilter` rather than a grid scope of its own, exactly as that struct's own doc says new narrowing terms should be — so the count and the cells are narrowed by the same thing, and it composes with the others for free. Suggested faces count, not only confirmed ones, or a freshly grouped person would show an empty grid. Crops are read from where they are now stored, falling back to cutting one out of the proxy for faces indexed before that existed. 480 tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+170
-22
@@ -32,6 +32,9 @@ export struct IdentityPerson {
|
||||
suggested-faces: int,
|
||||
cover: image,
|
||||
has-cover: bool,
|
||||
// Set aside by the user: correctly found, correctly grouped, and not
|
||||
// someone they want to identify. Hidden from the rail unless they ask.
|
||||
ignored: bool,
|
||||
}
|
||||
|
||||
export struct IdentityFace {
|
||||
@@ -61,10 +64,21 @@ component FaceCell inherits Rectangle {
|
||||
callback reject();
|
||||
callback toggle-pick();
|
||||
|
||||
property <length> edge: root.compact ? 96px : 128px;
|
||||
/// The drawn size, set by the grid that lays these out.
|
||||
///
|
||||
/// An `in` property with a default rather than a derived one: the grid
|
||||
/// divides its width into whole columns and tells each cell what it came
|
||||
/// to, exactly as the library grid does. The default is what a caller that
|
||||
/// does not care still gets.
|
||||
in property <length> edge: root.compact ? 96px : 128px;
|
||||
|
||||
/// Height of the caption strip under the crop. Named because the grid has
|
||||
/// to add it to the row pitch, and a grid using a different number from
|
||||
/// the cell is a grid whose rows creep.
|
||||
out property <length> caption-height: 34px;
|
||||
|
||||
width: edge;
|
||||
height: edge + 34px;
|
||||
height: edge + caption-height;
|
||||
background: face.picked ? Theme.selected : transparent;
|
||||
border-radius: Theme.radius;
|
||||
border-width: face.picked ? 1px : 0;
|
||||
@@ -148,8 +162,18 @@ export component IdentityScreen inherits Rectangle {
|
||||
|
||||
in property <[IdentityPerson]> people;
|
||||
in property <[IdentityFace]> faces;
|
||||
/// TRACES: FR-UI-1
|
||||
/// The narrow layout. Follows window width, not device type — a narrow
|
||||
/// desktop window gets it exactly as a phone does. Set from Rust beside
|
||||
/// `layout-class`, for the reason app.slint gives: a width read inside the
|
||||
/// layout that also feeds it is a binding loop.
|
||||
in property <bool> compact: false;
|
||||
in property <int> selected-person: -1;
|
||||
in property <string> selected-name;
|
||||
/// Whether the selected person is set aside, so one button can be both
|
||||
/// "Not interested" and "Bring back" — two buttons where only one is ever
|
||||
/// applicable is a row that teaches the user to ignore half of it.
|
||||
in property <bool> selected-ignored: false;
|
||||
// Faces belonging to nobody. Shown as a count rather than hidden, because
|
||||
// "why is this photograph not under anyone" deserves an answer.
|
||||
in property <int> unassigned: 0;
|
||||
@@ -206,6 +230,18 @@ export component IdentityScreen inherits Rectangle {
|
||||
/// cluster before. It also cannot be `changed selected-person`, since a
|
||||
/// merge can land the user back on the person they were already on.
|
||||
in property <int> name-revision: 0;
|
||||
/// Regrouping is running. It is off the UI thread, so the screen stays
|
||||
/// live and has to say what it is doing rather than simply stopping.
|
||||
in property <bool> regrouping: false;
|
||||
in property <string> regroup-status;
|
||||
/// People the user has set aside, and whether the rail is showing them.
|
||||
in property <int> ignored-count: 0;
|
||||
in property <bool> show-ignored: false;
|
||||
callback toggle-show-ignored();
|
||||
/// Set a person aside, or bring them back.
|
||||
callback ignore-person(int, bool);
|
||||
/// Show every photograph this person appears in, in the library grid.
|
||||
callback show-photos(int);
|
||||
callback recluster();
|
||||
callback index-faces();
|
||||
callback stop-indexing();
|
||||
@@ -225,7 +261,11 @@ export component IdentityScreen inherits Rectangle {
|
||||
HorizontalLayout {
|
||||
// ── the people rail ───────────────────────────────────────────────
|
||||
Panel {
|
||||
width: 260px;
|
||||
// 260px of a 360px phone is the screen. The rail still has to
|
||||
// carry a portrait and two lines of text, so it narrows rather
|
||||
// than disappearing — 132px keeps the 40px cover and elides the
|
||||
// labels, and leaves the faces grid enough width for two columns.
|
||||
width: root.compact ? 132px : 260px;
|
||||
|
||||
VerticalLayout {
|
||||
padding: Theme.gap;
|
||||
@@ -260,7 +300,12 @@ export component IdentityScreen inherits Rectangle {
|
||||
alignment: start;
|
||||
|
||||
for p[i] in root.people: Rectangle {
|
||||
height: 52px;
|
||||
// Height and visibility rather than filtering the
|
||||
// model in Rust: the rail is rebuilt on every
|
||||
// action, and re-deriving a second filtered list
|
||||
// per rebuild is work the toggle can do for free.
|
||||
visible: root.show-ignored || !p.ignored;
|
||||
height: (root.show-ignored || !p.ignored) ? 52px : 0px;
|
||||
border-radius: Theme.radius;
|
||||
background: p.id == root.selected-person
|
||||
? Theme.selected
|
||||
@@ -313,6 +358,17 @@ export component IdentityScreen inherits Rectangle {
|
||||
|
||||
Rectangle { }
|
||||
|
||||
// What the "not interested" button has hidden, and the way
|
||||
// back to it. A count with no way to reveal it would make the
|
||||
// action feel like a deletion, which is exactly what it is
|
||||
// not.
|
||||
if root.ignored-count > 0: Button {
|
||||
text: root.show-ignored
|
||||
? "Hide " + root.ignored-count + " set aside"
|
||||
: "Show " + root.ignored-count + " set aside";
|
||||
clicked => { root.toggle-show-ignored(); }
|
||||
}
|
||||
|
||||
// The NFR-SEC-5 control lives here rather than three levels
|
||||
// down in settings: this is where the user's face data
|
||||
// visibly is, and a delete-everything button they cannot find
|
||||
@@ -330,6 +386,16 @@ export component IdentityScreen inherits Rectangle {
|
||||
spacing: Theme.gap-sm;
|
||||
|
||||
// Header: who is selected, and what can be done to them.
|
||||
//
|
||||
// Two rows rather than one. The action row has grown — show
|
||||
// photos, set aside, confirm, split, index, regroup — and a
|
||||
// 240px name field beside five buttons does not fit a phone. Worse
|
||||
// than not fitting: **a layout cannot be narrower than its
|
||||
// children's minimums**, so an overflowing row reports that
|
||||
// oversized minimum upwards and inflates the whole screen, which
|
||||
// is the fault library.slint's filter bar documents at length. The
|
||||
// faces grid is a sibling of this row, so it would have been laid
|
||||
// out against a width that was never on the screen.
|
||||
HorizontalLayout {
|
||||
spacing: Theme.gap-sm;
|
||||
height: 32px;
|
||||
@@ -344,7 +410,11 @@ export component IdentityScreen inherits Rectangle {
|
||||
name-field := Field {
|
||||
text: root.selected-name;
|
||||
placeholder: "Name this person";
|
||||
width: root.selected-person >= 0 ? 240px : 0px;
|
||||
// Capped by what there is, so a narrow window shortens the
|
||||
// field instead of pushing the row past the edge.
|
||||
width: root.selected-person >= 0
|
||||
? min(240px, root.width - 2 * Theme.gap)
|
||||
: 0px;
|
||||
visible: root.selected-person >= 0;
|
||||
edited(t) => { root.name-edited(t); }
|
||||
accepted(t) => { root.rename(t); }
|
||||
@@ -357,6 +427,25 @@ export component IdentityScreen inherits Rectangle {
|
||||
}
|
||||
|
||||
Rectangle { }
|
||||
}
|
||||
|
||||
// The actions, in a strip that scrolls rather than overflowing.
|
||||
//
|
||||
// A Flickable's own minimum is nothing — it is built to be smaller
|
||||
// than what it holds — so wrapping the row stops it inflating the
|
||||
// screen and makes the buttons past the edge reachable instead of
|
||||
// merely absent. Same device as the library's filter chips, for
|
||||
// the same reason.
|
||||
Flickable {
|
||||
height: 36px;
|
||||
viewport-height: self.height;
|
||||
viewport-width: max(self.width, actions.preferred-width);
|
||||
|
||||
actions := HorizontalLayout {
|
||||
width: parent.viewport-width;
|
||||
height: parent.viewport-height;
|
||||
spacing: Theme.gap-sm;
|
||||
alignment: start;
|
||||
|
||||
if root.picked-count > 0: Text {
|
||||
text: root.picked-count + " selected";
|
||||
@@ -385,10 +474,28 @@ export component IdentityScreen inherits Rectangle {
|
||||
text: "Stop";
|
||||
clicked => { root.stop-indexing(); }
|
||||
}
|
||||
// The way back from a face to the photographs. This is the
|
||||
// point of having identified anybody, and without it the
|
||||
// screen is a filing cabinet with no drawer handles.
|
||||
if root.selected-person >= 0: Button {
|
||||
text: "Show photos";
|
||||
clicked => { root.show-photos(root.selected-person); }
|
||||
}
|
||||
// Most clusters in a real library are strangers — passers-by,
|
||||
// other people's guests, a face on a poster. Naming them is
|
||||
// not the job and neither is looking at them again.
|
||||
if root.selected-person >= 0: Button {
|
||||
text: root.selected-ignored ? "Bring back" : "Not interested";
|
||||
clicked => {
|
||||
root.ignore-person(root.selected-person, !root.selected-ignored);
|
||||
}
|
||||
}
|
||||
Button {
|
||||
text: "Regroup";
|
||||
text: root.regrouping ? "Regrouping…" : "Regroup";
|
||||
enabled: !root.regrouping;
|
||||
clicked => { root.recluster(); }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// The namesake offer. Sits directly under the name field that
|
||||
@@ -475,23 +582,64 @@ export component IdentityScreen inherits Rectangle {
|
||||
font-size: Theme.text-sm;
|
||||
}
|
||||
|
||||
Flickable {
|
||||
VerticalLayout {
|
||||
alignment: start;
|
||||
HorizontalLayout {
|
||||
// Slint has no flow layout, so the grid is a wrapping
|
||||
// row built from the model's own order; the cells are
|
||||
// fixed-size, which is what makes that tractable.
|
||||
spacing: Theme.gap-sm;
|
||||
alignment: start;
|
||||
if root.regroup-status != "": Text {
|
||||
text: root.regroup-status;
|
||||
color: Theme.ink-dim;
|
||||
font-size: Theme.text-sm;
|
||||
wrap: word-wrap;
|
||||
}
|
||||
|
||||
for f[i] in root.faces: FaceCell {
|
||||
face: f;
|
||||
confirm => { root.confirm-face(f.id); }
|
||||
reject => { root.reject-face(f.id); }
|
||||
toggle-pick => { root.toggle-pick(f.id); }
|
||||
}
|
||||
}
|
||||
// --- the faces, as a grid ------------------------------------
|
||||
//
|
||||
// This was one `HorizontalLayout` holding every face. The comment
|
||||
// on it claimed to be a wrapping row; Slint has no flow layout and
|
||||
// a HorizontalLayout does not wrap, so what it actually drew was a
|
||||
// single row running off the right-hand edge with everything past
|
||||
// the fourth or fifth face unreachable. A person with forty faces
|
||||
// was a person whose faces could not be reviewed.
|
||||
//
|
||||
// Laid out the way the library grid lays out thumbnails, for the
|
||||
// same reasons and with the same arithmetic: choose how many
|
||||
// columns of roughly the requested size fit, then divide the width
|
||||
// between them so the cells fill the row exactly and nothing
|
||||
// overhangs. Cells are placed absolutely inside the Flickable,
|
||||
// which is what makes the wrap possible at all.
|
||||
faces-area := Flickable {
|
||||
/// About this big. A request, not a measurement — `cell` is
|
||||
/// what is actually drawn.
|
||||
property <length> requested: root.compact ? 96px : 128px;
|
||||
|
||||
/// Rounded rather than floored, so a width nine tenths of the
|
||||
/// way to another column takes it instead of stranding it.
|
||||
property <int> columns:
|
||||
max(1, round((self.width - Theme.gap)
|
||||
/ (self.requested + Theme.gap)));
|
||||
|
||||
/// The cells share the width exactly: `columns` of them and
|
||||
/// the `columns + 1` gaps around them come to the full width,
|
||||
/// so there is no remainder left as a dead strip down the
|
||||
/// edge — which on a phone is a quarter of the screen.
|
||||
property <length> cell:
|
||||
max(64px, (self.width - Theme.gap * (self.columns + 1)) / self.columns);
|
||||
|
||||
/// The pitch between rows. The caption under each crop is part
|
||||
/// of the cell, so it is part of the pitch.
|
||||
property <length> row-pitch: self.cell + 34px + Theme.gap;
|
||||
property <int> rows: ceil(root.faces.length / max(1, self.columns));
|
||||
|
||||
// Horizontal extent is exactly the viewport: the grid wraps,
|
||||
// so there is nothing to scroll sideways to.
|
||||
viewport-width: self.width;
|
||||
viewport-height: Theme.gap + self.rows * self.row-pitch;
|
||||
|
||||
for f[i] in root.faces: FaceCell {
|
||||
x: Theme.gap + mod(i, faces-area.columns) * (faces-area.cell + Theme.gap);
|
||||
y: Theme.gap + floor(i / faces-area.columns) * faces-area.row-pitch;
|
||||
edge: faces-area.cell;
|
||||
face: f;
|
||||
confirm => { root.confirm-face(f.id); }
|
||||
reject => { root.reject-face(f.id); }
|
||||
toggle-pick => { root.toggle-pick(f.id); }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user