dtourolleandClaude Opus 5
59917c5183
Benchmarks / CPU and I/O (per commit) (push) Successful in 14m35s
Benchmarks / Frame budget (on demand) (push) Skipped
Build and test / Desktop (Linux) (push) Failing after 1h20m20s
Build and test / Layer separation (push) Successful in 51s
Traceability / Requirement traces (push) Successful in 2m8s
🐳 Android image / Build and push (push) Successful in 5s
Build and test / android-image (push) Successful in 5s
Build and test / Android (aarch64) (push) Successful in 1h5m51s
The rail entry read "Crop" while the panel it opens is headed COMPOSE and the button leaving it said "Done Cropping". One mode, three names, and the odd one out was named after a single control rather than after the decision — which is what made cropping look like a category of its own in the first place. Straightening, the quarter turns and the flips are already in that panel, and perspective will be. `ViewMode.crop` keeps its name: it identifies a canvas interaction, which is exactly what it still is. Found by looking at the running application rather than by reading, which is also how the two halves of this were noticed to disagree at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
397 lines
18 KiB
Plaintext
397 lines
18 KiB
Plaintext
// The develop view's tool rail: which tool the photographer is holding, and —
|
|
// where the interface is driven by a finger — which group of adjustments they
|
|
// are looking at.
|
|
//
|
|
// **Two sections, two sources, one rule between them.**
|
|
//
|
|
// The tools are the array literal in `tools` below. A tool is a row in it — an
|
|
// icon name, a word, and the `ViewMode` it arms — and adding one is that row
|
|
// plus a drawing in `icons.slint` plus a variant on the enum. Nothing in
|
|
// `app.slint` is touched and there is no second list that could fall out of
|
|
// step with it.
|
|
//
|
|
// The groups are not written here at all, and must not be: they are whatever
|
|
// the operation set declares itself to be about, resolved in Rust and handed
|
|
// over as `tabs` (FR-DEV-3a). This file names no group, exactly as
|
|
// `GroupStrip` names none — it takes the same model, because the two controls
|
|
// answer the same question and only one of them is on screen at a time.
|
|
//
|
|
// **Why a rail is the right shape for a finger and the wrong one for a mouse.**
|
|
// See `groups-in-rail` below, and D-N6 in `docs/ui-navigation.md` for the
|
|
// decision it reverses and the half of that decision that still stands.
|
|
//
|
|
// **Why it left the chip strip.** These four used to be chips at the top of
|
|
// the develop column, sharing a row with the adjustment groups — two kinds of
|
|
// state in one strip, told apart by the shape of their highlight. Three things
|
|
// were wrong with that, and only the third is about tidiness:
|
|
//
|
|
// 1. The column can be put away. The strip went with it, so the way *out* of
|
|
// crop went away with the way in, and the fix was a second "Done
|
|
// Cropping" button floating over the canvas — one control duplicated
|
|
// because the first one was reachable only sometimes.
|
|
// 2. The strip sized the column. Its chips are generated from the operation
|
|
// set, so the widest thing in the develop sidebar was a row nobody had
|
|
// chosen the contents of, and a richer operation set silently took width
|
|
// from the photograph.
|
|
// 3. A mode and a filter are not the same kind of thing. "Crop" changes what
|
|
// a click on the photograph does; "Light" changes which sliders are on
|
|
// screen. Putting them in one row and distinguishing them by underline
|
|
// versus fill asks the eye to carry a distinction the layout could just
|
|
// make.
|
|
//
|
|
// The rail is always up, so (1) is gone; it is a fixed width that no operation
|
|
// set can influence, so (2) is gone; and it is somewhere else entirely, so (3)
|
|
// is gone. What is left in the column is a row of group filters and nothing
|
|
// else — see `GroupStrip` in `adjust.slint`.
|
|
//
|
|
// **Down the left, not the right.** The develop column is on the right and
|
|
// holds the *consequences* of a choice — the sliders the tool exposes. The
|
|
// choice itself goes on the far side, so the eye's path across the window is
|
|
// tool, photograph, adjustment, in that order, and the rail never moves when
|
|
// the column opens and closes beside it.
|
|
|
|
import { Theme } from "theme.slint";
|
|
import { Icon } from "icons.slint";
|
|
import { ViewMode } from "adjust.slint";
|
|
|
|
// One tool. A struct rather than four parallel arrays so a row cannot be
|
|
// half-added — the compiler will not let a new entry omit its icon.
|
|
struct Tool {
|
|
/// A name from `icons.slint`'s vocabulary. A typo here draws nothing,
|
|
/// which is loud: the entry becomes a label with a hole above it.
|
|
icon: string,
|
|
/// What it is called. One word — see `rail-width` in `style.yaml`.
|
|
label: string,
|
|
/// What arming it puts the canvas into.
|
|
mode: ViewMode,
|
|
}
|
|
|
|
export component ToolRail inherits Rectangle {
|
|
/// Which tool is held. Owned by Rust, like every other piece of session
|
|
/// state: the rail asks for a mode and is told what the mode became, so a
|
|
/// change made anywhere else — the keyboard, the back gesture, the button
|
|
/// over the canvas — lights the same entry.
|
|
in property <ViewMode> mode: ViewMode.photo;
|
|
/// Whether there is a photograph to point a tool at. The rail collapses
|
|
/// rather than greying out: an empty develop view has no tools, and four
|
|
/// dead icons beside a blank canvas suggest otherwise.
|
|
in property <bool> enabled: true;
|
|
|
|
/// The adjustment groups, already resolved — the same model `GroupStrip`
|
|
/// takes, because it is the same list answering the same question.
|
|
///
|
|
/// Empty unless this rail is carrying them. Nothing here names a group:
|
|
/// the strings arrive from whatever the operations declared themselves to
|
|
/// be about (FR-DEV-3a).
|
|
in property <[string]> tabs;
|
|
/// Index into `tabs`, or -1 for "everything".
|
|
in property <int> active-tab: -1;
|
|
/// TRACES: FR-UI-1 | FR-UI-7
|
|
/// Whether the groups live in this rail or in the strip above the develop
|
|
/// column.
|
|
///
|
|
/// **The one property that makes this rail two different controls**, and
|
|
/// it is set from how the interface is being *driven* rather than from
|
|
/// which binary is running — see `input_class` in `lib.rs`.
|
|
///
|
|
/// With a mouse, a horizontal run of words above the column is a tab bar,
|
|
/// which is what a pointer is good at: it is one gesture to a target the
|
|
/// eye has already found, and the strip costs a row of a column that has
|
|
/// plenty of height. With a finger it is the wrong control twice over. The
|
|
/// strip pans when the operation set is rich, so a group can be off the
|
|
/// end of a row with nothing saying so; and it sits at the top of a
|
|
/// column, which on a tablet held in two hands is the furthest point from
|
|
/// either thumb.
|
|
///
|
|
/// Down the rail the same list is a column of finger-sized targets, all of
|
|
/// them visible at once, on the edge of the screen a hand is already at.
|
|
in property <bool> groups-in-rail: false;
|
|
|
|
callback picked(ViewMode);
|
|
/// A group was chosen: an index into `tabs`, or -1 for "everything".
|
|
///
|
|
/// Deliberately the same signature `GroupStrip` emits, and routed to the
|
|
/// same callback in `app.slint`. The two controls are alternatives, not
|
|
/// peers — only one is on screen at a time — and giving them one contract
|
|
/// means Rust cannot tell which of them the user pressed, and has no
|
|
/// reason to want to.
|
|
callback group-picked(int);
|
|
|
|
/// The groups this rail actually draws.
|
|
///
|
|
/// A conditional model rather than an `if` wrapped around the repeater:
|
|
/// Slint has no way to nest one inside the other, and putting the
|
|
/// condition on the model keeps the entries as direct children of the
|
|
/// layout below — which is the shape that matters here. A nested layout
|
|
/// under-reports its height and its siblings get drawn on top of each
|
|
/// other; `app.slint`'s develop column carries the same note.
|
|
private property <[string]> rail-tabs: root.groups-in-rail ? root.tabs : [];
|
|
|
|
// **The table.** Add a row to get a tool.
|
|
//
|
|
// Order is the order they appear, and it is not arbitrary: `photo` first
|
|
// because it is the resting state and the way back from everywhere else,
|
|
// then the three that arm a gesture on the canvas, roughly in the order a
|
|
// photograph is worked — frame it, then adjust parts of it, then clean it
|
|
// up.
|
|
//
|
|
// `MaskSource::Brush` is the next one to land here. The core already has
|
|
// the stroke calls; what is missing is the canvas interaction, and when it
|
|
// arrives this file's share of the work is one line.
|
|
private property <[Tool]> tools: [
|
|
{ icon: "photo", label: "Photo", mode: ViewMode.photo },
|
|
// "Compose", not "Crop", and the panel it opens says the same. The
|
|
// tool arms a crop gesture, but what it is *for* is deciding the
|
|
// frame — straightening, the quarter turns and the flips are in that
|
|
// panel too, and perspective will be. Naming the tool after one of its
|
|
// controls was what made cropping look like a category of its own.
|
|
//
|
|
// `ViewMode.crop` keeps its name: it identifies a canvas interaction,
|
|
// which is exactly what it still is.
|
|
{ icon: "crop", label: "Compose", mode: ViewMode.crop },
|
|
{ icon: "mask", label: "Local", mode: ViewMode.local },
|
|
{ icon: "repair", label: "Repair", mode: ViewMode.spots },
|
|
];
|
|
|
|
// TRACES: FR-UI-1
|
|
// Fixed, and the point of the exercise. This rail flanks the photograph,
|
|
// so its width is taken out of the picture — and a rail measured from its
|
|
// contents would hand that decision to whichever tool label happens to be
|
|
// longest. `style.yaml` carries the number and the reasoning.
|
|
width: root.enabled ? Theme.rail-width : 0px;
|
|
visible: root.enabled;
|
|
background: Theme.surface;
|
|
clip: true;
|
|
|
|
// **A Flickable now, and this comment used to argue the opposite.** It
|
|
// said the rail held four entries written in this file, on an axis with
|
|
// room for fourteen, and that a rail which scrolls is the answer only if
|
|
// that stops being true. It has stopped being true: with `groups-in-rail`
|
|
// the list is four tools plus one entry per group the operation set
|
|
// declares, and an operation set is exactly the "something the user's data
|
|
// decides" the old note excluded this control from.
|
|
//
|
|
// Four tools, "All" and today's five groups is ten entries — comfortable
|
|
// on any supported screen. The point is not today's count but that the
|
|
// count is no longer written here, and a rail that overflows loses its
|
|
// last entries silently, on the one control the develop view is navigated
|
|
// by.
|
|
Flickable {
|
|
width: 100%;
|
|
height: 100%;
|
|
viewport-width: self.width;
|
|
viewport-height: max(self.height, layout.preferred-height);
|
|
|
|
layout := VerticalLayout {
|
|
height: parent.viewport-height;
|
|
padding-top: Theme.gap-sm;
|
|
spacing: 0px;
|
|
alignment: start;
|
|
|
|
for tool in root.tools: entry := TouchArea {
|
|
height: Theme.rail-entry-height;
|
|
mouse-cursor: pointer;
|
|
|
|
property <bool> on: root.mode == tool.mode;
|
|
|
|
// Pressing the tool you are holding puts it down, exactly as the
|
|
// chips did: the same control both directions. `photo` is the
|
|
// exception — it *is* putting the tool down, so pressing it while
|
|
// it is lit is a no-op rather than a toggle into itself.
|
|
clicked => {
|
|
root.picked(entry.on ? ViewMode.photo : tool.mode);
|
|
}
|
|
|
|
// **The word below is drawn; this is what makes it a control.**
|
|
// Without these the rail reaches a screen reader as four pieces of
|
|
// static text — the labels get through, because a `Text` announces
|
|
// itself, and nothing says any of them can be pressed. That is the
|
|
// develop view's primary navigation reduced to a caption.
|
|
//
|
|
// `checkable` unconditionally, unlike `Button`'s: a rail entry is
|
|
// always a held-or-not state, so an unheld one should say "not
|
|
// pressed" rather than pass for an ordinary button. The action
|
|
// repeats the click handler rather than calling it, because a
|
|
// `TouchArea`'s `clicked` is raised by the pointer and cannot be
|
|
// raised from here.
|
|
accessible-role: button;
|
|
accessible-label: tool.label;
|
|
accessible-checkable: true;
|
|
accessible-checked: entry.on;
|
|
accessible-action-default => {
|
|
root.picked(entry.on ? ViewMode.photo : tool.mode);
|
|
}
|
|
|
|
// The lit tile, and the only marker there is. Inset from the
|
|
// rail's edges so the run of four reads as four things rather than
|
|
// as one striped column.
|
|
//
|
|
// A bright bar against the outer edge was tried alongside it, on
|
|
// the theory that a rail is scanned from the side and needs
|
|
// something unambiguous. It is not needed and it is not
|
|
// unambiguous: `active-dim` is near-white and `hover` is a shade
|
|
// above the surface, so the held tool and a tool under the pointer
|
|
// are not two similar greys — they are opposite ends of the
|
|
// palette. The bar sat against the lit tile and merged with it.
|
|
Rectangle {
|
|
x: Theme.gap-sm / 2;
|
|
width: parent.width - Theme.gap-sm;
|
|
height: parent.height - 2px;
|
|
y: 1px;
|
|
border-radius: Theme.radius;
|
|
background: entry.on
|
|
? Theme.active-dim
|
|
: (entry.has-hover ? Theme.hover : transparent);
|
|
}
|
|
|
|
VerticalLayout {
|
|
alignment: center;
|
|
spacing: 3px;
|
|
|
|
HorizontalLayout {
|
|
alignment: center;
|
|
Icon {
|
|
name: tool.icon;
|
|
size: 20px;
|
|
// Dark on the lit tile, which is near-white: the same
|
|
// inversion `Button`'s primary state makes, and the
|
|
// same one the chips made before this.
|
|
ink: entry.on
|
|
? Theme.ground
|
|
: (entry.has-hover ? Theme.ink : Theme.ink-dim);
|
|
}
|
|
}
|
|
|
|
// Named, not just drawn. An icon-only rail is a quiz — these
|
|
// four are conventional enough to guess and not conventional
|
|
// enough to be sure of, and "sure" is what a tool that changes
|
|
// what a click does has to be. The word is `text-sm` and dim,
|
|
// so it reads as the icon's caption rather than as a button in
|
|
// its own right.
|
|
Text {
|
|
text: tool.label;
|
|
font-size: Theme.text-sm;
|
|
color: entry.on
|
|
? Theme.ground
|
|
: (entry.has-hover ? Theme.ink : Theme.ink-faint);
|
|
horizontal-alignment: center;
|
|
// Elided rather than wrapped: a two-line label would make
|
|
// this entry taller than the three beside it, and a rail
|
|
// whose rows are different heights reads as a list of
|
|
// unrelated things.
|
|
overflow: elide;
|
|
}
|
|
}
|
|
}
|
|
|
|
// **The seam between the two kinds of entry.**
|
|
//
|
|
// A rule and a gap, because above it are things that change what a
|
|
// click on the photograph *does* and below it are things that change
|
|
// which sliders are on screen. `ui-navigation.md` §N1 made that
|
|
// distinction by drawing a mode and a group differently in one strip;
|
|
// it holds here by separating them, which is the cheaper signal when
|
|
// the axis is vertical and there is a whole rail's width to draw a
|
|
// line across.
|
|
if root.groups-in-rail: Rectangle {
|
|
height: Theme.gap;
|
|
background: transparent;
|
|
|
|
Rectangle {
|
|
x: Theme.gap-sm;
|
|
width: parent.width - 2 * Theme.gap-sm;
|
|
height: 1px;
|
|
y: (parent.height - 1px) / 2;
|
|
background: Theme.rule;
|
|
}
|
|
}
|
|
|
|
// "All", and it is not decoration. A group filter that cannot be
|
|
// cleared is a way to make controls unreachable, and this is the only
|
|
// entry in the run below that is not generated.
|
|
if root.groups-in-rail: all := TouchArea {
|
|
height: Theme.touch-target;
|
|
mouse-cursor: pointer;
|
|
clicked => { root.group-picked(-1); }
|
|
|
|
accessible-role: button;
|
|
accessible-label: "All adjustments";
|
|
accessible-checkable: true;
|
|
accessible-checked: root.active-tab == -1;
|
|
accessible-action-default => { root.group-picked(-1); }
|
|
|
|
Rectangle {
|
|
x: 0;
|
|
width: 2px;
|
|
height: parent.height;
|
|
background: root.active-tab == -1 ? Theme.ink : transparent;
|
|
}
|
|
|
|
Text {
|
|
text: "All";
|
|
font-size: Theme.text-sm;
|
|
color: root.active-tab == -1
|
|
? Theme.ink
|
|
: (all.has-hover ? Theme.ink : Theme.ink-faint);
|
|
horizontal-alignment: center;
|
|
vertical-alignment: center;
|
|
width: 100%;
|
|
height: 100%;
|
|
}
|
|
}
|
|
|
|
// **A bar down the leading edge, not a filled tile.**
|
|
//
|
|
// The tools above fill with `active-dim` and invert their ink; these
|
|
// do not, and the difference is the one §N1 insisted on — a mode and a
|
|
// filter are not the same kind of state, and a reader should not have
|
|
// to remember which section a lit entry was in to know which they are
|
|
// looking at. An underline is what said so when the groups were a
|
|
// horizontal strip; turned ninety degrees, that is a bar down the
|
|
// edge.
|
|
for tab[i] in root.rail-tabs: group := TouchArea {
|
|
height: Theme.touch-target;
|
|
mouse-cursor: pointer;
|
|
clicked => { root.group-picked(i); }
|
|
|
|
accessible-role: button;
|
|
accessible-label: tab;
|
|
accessible-checkable: true;
|
|
accessible-checked: root.active-tab == i;
|
|
accessible-action-default => { root.group-picked(i); }
|
|
|
|
Rectangle {
|
|
x: 0;
|
|
width: 2px;
|
|
height: parent.height;
|
|
background: root.active-tab == i ? Theme.ink : transparent;
|
|
}
|
|
|
|
Text {
|
|
text: tab;
|
|
font-size: Theme.text-sm;
|
|
color: root.active-tab == i
|
|
? Theme.ink
|
|
: (group.has-hover ? Theme.ink : Theme.ink-faint);
|
|
horizontal-alignment: center;
|
|
vertical-alignment: center;
|
|
width: 100%;
|
|
height: 100%;
|
|
// A group's name comes from the operation set and this rail is
|
|
// a mandated width, so a long one has to give somewhere.
|
|
overflow: elide;
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
// The rail's own edge. Drawn here rather than by whatever contains it, so
|
|
// the rail is a complete thing wherever it is put — and on the right,
|
|
// where it meets the canvas.
|
|
Rectangle {
|
|
x: parent.width - 1px;
|
|
width: 1px;
|
|
background: Theme.rule;
|
|
}
|
|
}
|