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

Put the adjustment groups in the rail where a finger is driving

Browse Source 
Reported from the tablet: the tool rail is very useful there, and the same
interface under a mouse and keyboard is not. That is `ui-navigation.md` D-N2's
central assumption failing in use, and the interesting part is which half of
it failed.

D-N2 was right that platform is the wrong axis and width is the wrong axis: a
tablet in landscape wants what a desktop wants, and a desktop window dragged
narrow wants what a small screen wants. `apply_layout_class` still decides the
layout class from the window and nothing here changes that. What D-N2 got
wrong is the sentence "touch changes hit regions, not layout" — it identified
input as the real difference between the targets and then assumed that
difference could never reach the layout.

Two controls answer one question — which group of adjustments am I looking at
— and neither is better in general. A horizontal strip above the column is one
gesture to a target the eye has already found, and it pans when the operation
set is rich, so a group can sit off the end with nothing saying so: a pointer
user tolerates that, a finger user never discovers it. The same list down the
rail is every entry visible at once, each finger-sized, on the edge of the
screen the hand is already holding, and it costs no width because the rail is
already there.

So `ToolRail` grows a second section, and `GroupStrip` stands down when it
does. The two are never both on screen, which is why they can share
`adjust-tab-picked`: Rust is not told which was pressed and has no reason to
want to. Mode and group stay independent axes as N1 requires — one entry lit
in each section, and choosing a group while a tool is held still filters
without putting the tool down.

They stay drawn differently, which N1 also required. The tools fill with
`active-dim` and invert their ink; the groups take a bar down the leading
edge — the strip's underline turned ninety degrees — so a lit entry says which
kind of state it is without the reader having to remember which section it was
in. The rule between the sections is the second signal.

The rail scrolls now. Its own note argued against a Flickable because "this
list is four entries written in this file"; with the groups in it the list
comes from the operation set, which is exactly the "something the user's data
decides" that note excluded this control from.

The axis is input, and it is a preference because the automatic answer is a
guess that cannot be made reliable. Neither platform can be asked what the
user is holding: an Android tablet in a keyboard case is being driven like a
desktop, and a touchscreen laptop is whichever its owner says.
`dr_plat::is_touch_first` reports the usual case per platform, and
`GroupNavigation` lets it be overridden. Settings names what Automatic
resolves to on this device rather than leaving it to be found by pressing.

D-N6 records the reversal beside the decision it reverses, including the half
that still stands and the question it opens: whether Local is a mode at all,
or a scope that would collapse the two sections into one list.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
dtourolle
2026-09-05 18:14:03 +02:00
co-authored by Claude Opus 5
parent 474dcf0bf6
commit 6a97fdf6f9
10 changed files with 573 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ui/dr-ui/src/lib.rs
+33
View File
@@ -2330,6 +2330,12 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
// photographer works rather than one per place it is asked.
presets::wire_scope(&window, settings.clone(), clipboard.clone());
presets::render_scope(&window, &settings);
// TRACES: FR-UI-1
// Rendered from the same settings controller and at the same moment,
// so the rail and the strip cannot both be on screen after a change to
// the preference that chooses between them.
window.set_groups_in_rail(groups_in_rail(&settings));
presets::render(&window, &clipboard, &settings);
}
@@ -3353,6 +3359,33 @@ struct PanelChoices {
collections: std::cell::Cell<Option<bool>>,
}
/// TRACES: FR-UI-1 | FR-UI-7
/// Where the adjustment groups are chosen from, for this device and this
/// photographer.
///
/// **Not the layout class, and not derived from one.** `apply_layout_class`
/// below answers "how much room is there" from the window's width, and was
/// right to refuse a device check — a narrow desktop window wants the compact
/// layout exactly as a small screen would. This answers "what is the user
/// pointing with", which width cannot stand in for: DarkRoom's two targets are
/// a 12-inch tablet and a desktop, the same size and the same layout class,
/// and only one of them has no hover, no modifier keys and a finger for a
/// cursor.
///
/// `ui-navigation.md` D-N2 decided against any divergence between the targets
/// and this is the exception to it, argued on D-N2's own ground: it identified
/// input as the real difference and then assumed touch changes hit regions
/// rather than layout. A horizontal strip that pans, at the top of a column,
/// against a rail of finger-sized targets down the edge the hand is already
/// on, is where that assumption runs out.
fn groups_in_rail(settings: &settings_ui::SettingsController) -> bool {
settings
.snapshot()
.develop
.group_navigation
.groups_in_rail(dr_plat::is_touch_first())
}
fn apply_layout_class(window: &AppWindow, width: f32, panels: &PanelChoices) {
let expanded = width >= EXPANDED_MIN_WIDTH;
window.set_expanded(expanded);
ui/dr-ui/src/settings_ui.rs
+52 -2
View File
@@ -26,8 +26,8 @@ use std::rc::Rc;
use dr_types::settings::budget;
use dr_types::{
CollisionPolicy, ColourSpace, ExportFormat, ExportTarget, LibrarySettings, OutputSharpening,
ScreenSize, Settings, SizingMode,
CollisionPolicy, ColourSpace, ExportFormat, ExportTarget, GroupNavigation, LibrarySettings,
OutputSharpening, ScreenSize, Settings, SizingMode,
};
use slint::ComponentHandle;
@@ -154,6 +154,36 @@ pub fn render(window: &AppWindow, controller: &SettingsController) {
window.set_settings_thumbnail_budget(budget::label(s.cache.thumbnail_budget_bytes).into());
window.set_settings_thumbnail_unlimited(s.cache.thumbnail_budget_bytes.is_none());
window.set_settings_keep_opened(s.cache.keep_opened_originals);
// --- develop -------------------------------------------------------
//
// TRACES: FR-UI-1
// Where the adjustment groups are chosen from. The labels are written here
// rather than derived from the enum, because "Automatic" is a sentence
// about behaviour and the other two name controls the photographer can
// see; `GroupNavigation`'s variants are identifiers and would read as
// jargon in a picker.
window.set_settings_group_nav_labels(slint::ModelRc::new(slint::VecModel::from(vec![
slint::SharedString::from("Automatic"),
slint::SharedString::from("Tool rail"),
slint::SharedString::from("Tab strip"),
])));
window.set_settings_group_nav_selected(match s.develop.group_navigation {
GroupNavigation::Auto => 0,
GroupNavigation::Rail => 1,
GroupNavigation::Tabs => 2,
});
// Said plainly rather than left to be discovered by pressing it. The
// automatic answer depends on which build this is, which is exactly the
// sort of thing a photographer cannot see and should not have to infer.
window.set_settings_group_nav_auto_says(
if dr_plat::is_touch_first() {
"the tool rail"
} else {
"the tab strip"
}
.into(),
);
window.set_settings_cache_usage(controller.usage_label.borrow().clone().into());
// --- library -------------------------------------------------------
@@ -458,6 +488,26 @@ pub fn wire<F, G>(
{
let weak = window.as_weak();
let ctl = controller.clone();
{
let weak = window.as_weak();
let ctl = controller.clone();
window.on_settings_group_nav_picked(move |i| {
let Some(w) = weak.upgrade() else { return };
let choice = match i {
1 => GroupNavigation::Rail,
2 => GroupNavigation::Tabs,
_ => GroupNavigation::Auto,
};
ctl.edit(|s| s.develop.group_navigation = choice);
// Both, and in this order. `render` refreshes the picker;
// the second line is what actually moves the groups from one
// control to the other, and without it the preference would
// save correctly and change nothing until the next launch.
render(&w, &ctl);
w.set_groups_in_rail(choice.groups_in_rail(dr_plat::is_touch_first()));
});
}
window.on_settings_format_picked(move |i| {
let Some(w) = weak.upgrade() else { return };
if let Some(f) = ExportFormat::ALL.get(i as usize).copied() {
ui/dr-ui/ui/app.slint
+40 -1
View File
@@ -725,6 +725,18 @@ export component AppWindow inherits Window {
in property <bool> film-print: false;
callback film-picked(int);
callback film-print-toggled(bool);
/// TRACES: FR-UI-1 | FR-UI-7
/// Whether the adjustment groups are chosen from the tool rail or from the
/// strip above the develop column.
///
/// Owned by Rust, which composes it from the input modality and the
/// photographer's override — see `ToolRail::groups-in-rail` for why the
/// axis is input rather than platform or width.
///
/// The two controls are mutually exclusive by construction: this shows one
/// and hides the other, so there is never a moment with two ways to
/// answer the same question, and no state to keep in step between them.
in property <bool> groups-in-rail: false;
in property <[string]> adjust-tabs;
in property <int> adjust-active-tab: -1;
callback adjust-tab-picked(int);
@@ -882,6 +894,11 @@ export component AppWindow inherits Window {
in property <int> settings-timeline-bars-selected: 0;
callback settings-timeline-bars-picked(int);
/// TRACES: FR-UI-1
/// The group-navigation preference, and what "Automatic" resolves to here.
in property <[string]> settings-group-nav-labels;
in property <int> settings-group-nav-selected: 0;
in property <string> settings-group-nav-auto-says;
in property <[string]> settings-format-labels;
in property <int> settings-format-selected: 0;
in property <int> settings-quality: 90;
@@ -917,6 +934,7 @@ export component AppWindow inherits Window {
in property <int> settings-target-selected: 0;
in property <string> settings-error: "";
callback settings-group-nav-picked(int);
callback settings-format-picked(int);
callback settings-quality-changed(int);
callback settings-colour-picked(int);
@@ -1311,6 +1329,9 @@ in property <bool> panel-visible: true;
}
keep-opened-toggled(on) => { root.settings-keep-opened-toggled(on); }
group-nav-labels: root.settings-group-nav-labels;
group-nav-selected: root.settings-group-nav-selected;
group-nav-auto-says: root.settings-group-nav-auto-says;
format-labels: root.settings-format-labels;
format-selected: root.settings-format-selected;
quality: root.settings-quality;
@@ -1346,6 +1367,7 @@ in property <bool> panel-visible: true;
target-selected: root.settings-target-selected;
error: root.settings-error;
group-nav-picked(i) => { root.settings-group-nav-picked(i); }
format-picked(i) => { root.settings-format-picked(i); }
quality-changed(q) => { root.settings-quality-changed(q); }
colour-picked(i) => { root.settings-colour-picked(i); }
@@ -1780,6 +1802,17 @@ in property <bool> panel-visible: true;
enabled: root.total > 0 && root.load-error == "";
mode: root.view-mode;
picked(m) => { root.mode-picked(m); }
// The same model and the same callback the strip below
// takes, because only one of the two is ever on screen.
// Routing both to `adjust-tab-picked` means Rust is not
// told which control was pressed and has no reason to
// care — the choice between them is a presentation
// decision, and it stays one.
groups-in-rail: root.groups-in-rail;
tabs: root.adjust-tabs;
active-tab: root.adjust-active-tab;
group-picked(i) => { root.adjust-tab-picked(i); }
}
// The canvas: compute output composited directly. No CPU
@@ -2343,7 +2376,13 @@ in property <bool> panel-visible: true;
// being outside the Flickable rather than by any coordinate.
VerticalLayout {
GroupStrip {
enabled: root.adjust-enabled;
// Stands down when the rail is carrying the
// groups. `enabled` rather than an `if`, because
// the strip already collapses itself to zero
// height on it — and a conditional child in this
// layout is the shape that has caused binding
// loops in this file before.
enabled: root.adjust-enabled && !root.groups-in-rail;
tabs: root.adjust-tabs;
active-tab: root.adjust-active-tab;
picked(i) => { root.adjust-tab-picked(i); }
ui/dr-ui/ui/settings.slint
+38
View File
@@ -101,8 +101,16 @@ export component SettingsPage inherits Rectangle {
/// TRACES: FR-DEV-6
/// Which kinds of edit a copy or preset carries. Supersedes the boolean
/// above, which named only the one kind anybody wanted to exclude.
/// TRACES: FR-UI-1
/// Where the develop view's adjustment groups are chosen from.
in property <[string]> group-nav-labels;
in property <int> group-nav-selected: 0;
/// What "Automatic" resolves to on this device, so the caption can say it
/// rather than leaving the photographer to press it and find out.
in property <string> group-nav-auto-says;
in property <[ScopeKind]> copy-scope-kinds;
in property <bool> copy-scope-empty: false;
callback group-nav-picked(int);
callback copy-scope-toggled(string);
/// What the cache currently holds. Empty hides the line.
in property <string> cache-usage;
@@ -554,6 +562,36 @@ export component SettingsPage inherits Rectangle {
PanelHeading { text: "DEVELOP"; }
// TRACES: FR-UI-1 | FR-UI-7
// Where the adjustment groups are chosen from.
//
// A preference rather than a fixed rule because the
// automatic answer is a guess and cannot be otherwise.
// Neither platform can be asked what the user is
// actually holding: an Android tablet in a keyboard
// case is being driven like a desktop, and a
// touchscreen laptop is whichever its owner says. The
// guess is right often enough to be the default and
// wrong often enough to need a way out.
VerticalLayout {
spacing: 4px;
Segmented {
label: "Adjustment groups";
options: root.group-nav-labels;
selected: root.group-nav-selected;
picked(i) => { root.group-nav-picked(i); }
}
Caption {
text: "Down the tool rail, or in a strip above "
+ "the panel. Automatic follows how this "
+ "device is driven — here, "
+ root.group-nav-auto-says + ".";
wrap: word-wrap;
}
}
// TRACES: FR-DEV-6
// Which kinds of edit a copy, a paste or a preset
// carries.
ui/dr-ui/ui/toolrail.slint
+189 -16
View File
@@ -1,12 +1,24 @@
// The develop view's tool rail: which tool the photographer is holding.
// 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.
//
// **One table, one file.** Everything that decides what this rail contains is
// 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, nothing here is per-tool, and there is no second list anywhere that
// could fall out of step with this one. That is the whole design: the rail is
// generated, so it cannot be *partly* updated.
// **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
@@ -65,7 +77,55 @@ export component ToolRail inherits Rectangle {
/// 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.
//
@@ -95,14 +155,27 @@ export component ToolRail inherits Rectangle {
background: Theme.surface;
clip: true;
// No Flickable. Every other strip in this view has one, because every
// other strip is generated from something the user's data decides and can
// therefore outgrow its space. This list is four entries written in this
// file, running down an axis with a whole window of room — the shortest
// supported window fits fourteen. If that ever stops being true the answer
// is a rail that scrolls, not a rail that overflows, and this comment is
// where to start.
VerticalLayout {
// **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;
@@ -202,6 +275,106 @@ export component ToolRail inherits Rectangle {
}
}
}
// **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