diff --git a/core/dr-types/src/lib.rs b/core/dr-types/src/lib.rs index 9839b3d..6bcf9c5 100644 --- a/core/dr-types/src/lib.rs +++ b/core/dr-types/src/lib.rs @@ -19,8 +19,8 @@ pub use place::{Place, PlaceScope, Screen, StoredFilter}; pub use selector::{ColourLabel, DateSelector, FlagState, Selector, Tier}; pub use settings::{ CacheSettings, CollisionPolicy, ColourSpace, DevelopSettings, ExportFormat, ExportSettings, - ExportTarget, ImportSettings, LibrarySettings, OutputSharpening, ScreenSize, Settings, - SizingMode, + ExportTarget, GroupNavigation, ImportSettings, LibrarySettings, OutputSharpening, ScreenSize, + Settings, SizingMode, }; pub use time::{ civil_from_unix, civil_from_unix_at, format_date, parse_date, unix_from_civil, Civil, diff --git a/core/dr-types/src/settings.rs b/core/dr-types/src/settings.rs index f4f90a0..ec59839 100644 --- a/core/dr-types/src/settings.rs +++ b/core/dr-types/src/settings.rs @@ -317,6 +317,54 @@ pub struct DevelopSettings { /// derives the set from the old boolean exactly once, and every later read /// finds a real answer here. pub copy_attributes: Option>, + + /// TRACES: FR-UI-1 | FR-UI-7 + /// Where the adjustment groups are chosen from. + /// + /// `Auto` — the default — decides from how the interface is being driven + /// rather than from which binary is running. `ui-navigation.md` D-N2 ruled + /// out a platform split and was right about the reason: a tablet in + /// landscape wants what a desktop wants, so `cfg(target_os)` gives one + /// physical situation two answers. What it got wrong was the conclusion + /// that nothing therefore differs — it assumed touch changes hit regions + /// and not layout, and a rail full of finger-sized targets against a + /// horizontal strip that pans is a counter-example. + /// + /// So the axis is input, which is the one D-N2 itself identified as the + /// real difference between the targets, and it is overridable because the + /// automatic answer cannot be right for everyone: a tablet with a keyboard + /// case is being driven like a desktop, and a touchscreen laptop is + /// whichever its owner says. + pub group_navigation: GroupNavigation, +} + +/// TRACES: FR-UI-1 +/// Where the develop view's adjustment groups are chosen from. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum GroupNavigation { + /// Follow the input modality: the rail under a finger, tabs under a mouse. + #[default] + Auto, + /// Always down the tool rail. + Rail, + /// Always the strip above the develop column. + Tabs, +} + +impl GroupNavigation { + /// Whether the groups belong in the rail, given how this device is driven. + /// + /// Takes the modality rather than reading it, because deciding it needs a + /// `cfg` this crate has no business carrying: `dr-types` is the shared + /// vocabulary and sits below everything that knows what a platform is. + pub fn groups_in_rail(self, touch: bool) -> bool { + match self { + Self::Auto => touch, + Self::Rail => true, + Self::Tabs => false, + } + } } // --------------------------------------------------------------------------- @@ -1114,6 +1162,48 @@ pub mod budget { #[cfg(test)] mod tests { + + /// TRACES: FR-UI-1 + /// The automatic answer follows the input, and the overrides do not. + #[test] + fn group_navigation_follows_input_only_when_asked_to() { + // Auto is the only variant that consults the modality. The other two + // exist precisely because the guess is sometimes wrong — a tablet in a + // keyboard case, a touchscreen laptop — so a variant that quietly fell + // back to the platform would be no override at all. + assert!(GroupNavigation::Auto.groups_in_rail(true)); + assert!(!GroupNavigation::Auto.groups_in_rail(false)); + + for touch in [true, false] { + assert!( + GroupNavigation::Rail.groups_in_rail(touch), + "an explicit choice of the rail must survive a {touch} device" + ); + assert!( + !GroupNavigation::Tabs.groups_in_rail(touch), + "an explicit choice of tabs must survive a {touch} device" + ); + } + } + + /// A config written before this setting existed must read as `Auto`. + /// + /// Not a formality: `#[serde(default)]` on the struct is what makes an + /// older file load at all, and the derived `Default` on the enum is what + /// decides which answer it lands on. Getting that wrong would move every + /// existing photographer's develop panel on upgrade, for a preference they + /// never expressed. + #[test] + fn a_config_without_the_setting_defaults_to_automatic() { + let older: Settings = serde_json::from_str("{\"develop\": {}}").expect("older config"); + assert_eq!(older.develop.group_navigation, GroupNavigation::Auto); + + // And it round-trips once written, so a device that has never been + // touched does not keep re-deciding. + let text = serde_json::to_string(&older).expect("serialise"); + let back: Settings = serde_json::from_str(&text).expect("reload"); + assert_eq!(back.develop.group_navigation, GroupNavigation::Auto); + } use super::*; #[test] diff --git a/docs/ui-navigation.md b/docs/ui-navigation.md index ff68601..76ec960 100644 --- a/docs/ui-navigation.md +++ b/docs/ui-navigation.md @@ -147,7 +147,12 @@ describe what is being looked at — and without it the instrument and the controls disagree about what they are measuring. Costs a mask term in the histogram reduction, which already runs per frame over the displayed frame. -### D-N2 — One layout, because both targets are wide · **DECIDED** +### D-N2 — One layout, because both targets are wide · **PARTLY REVERSED** + +> **Reversed for navigation, 2026-09-05, by use.** The reasoning below is +> still right about *size* and still right that `cfg(target_os)` is the wrong +> axis. It is wrong in one place, and the wrong bit is the sentence "touch +> changes **hit regions, not layout**". See D-N6. The question was whether desktop and Android should diverge. The answer turns out to be that **neither the platform nor the width axis separates DarkRoom's @@ -196,6 +201,70 @@ throughout.** The consequences worth stating: and nothing about them is qualified by a modifier: what is drawn is all there is. +### D-N6 — The groups move to the rail under a finger · **DECIDED** + +Reported from a tablet: the tool rail is *"very useful"* there, and the same +interface with a mouse and keyboard is not ergonomic. That is D-N2's assumption +failing in the field, and it is worth being precise about which half failed. + +**What D-N2 got right.** Platform is the wrong axis, and width is the wrong +axis. A tablet in landscape wants what a desktop wants; 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 it got wrong.** It identified input as the real difference between the +targets and then concluded that input changes only hit regions. Two controls +answering one question — *which group of adjustments am I looking at* — +disprove it: + +- **A horizontal strip above the column.** One gesture to a target the eye has + already found; costs one row of a column with height to spare. It pans when + the operation set is rich, so a group can be off the end with nothing saying + so — which a pointer user tolerates and a finger user does not discover. +- **A vertical run down the rail.** Every entry visible at once, each a + finger-sized target, on the edge of the screen the hand is already holding. + Costs nothing extra in width, because the rail is already there and already + mandated. + +Neither is better in general. The first is better with a pointer and the second +is better with a finger, which is a divergence on **input modality** — the axis +D-N2 itself named. + +**The shape.** `ToolRail` grows a second section below a rule: the same +`adjust-tabs` model the strip takes, plus "All". `GroupStrip` stands down when +the rail carries them, so the two are never both on screen and there is no +state to keep in step. Mode and group stay independent axes exactly as N1 +requires — one entry lit in each section, and picking a group while a tool is +held still filters without putting the tool down. + +**Drawn differently, still.** N1 insisted a mode and a filter must not be told +apart by the shape of their highlight alone. The tools fill with `active-dim` +and invert their ink; the groups take a bar down the leading edge — the +underline from the horizontal strip, turned ninety degrees. The rule between +the sections is the second signal. + +**The rail scrolls now.** Its own note argued against a Flickable on the +grounds that four entries were written in the file. With the groups in it the +list is generated from the operation set, which is exactly the "something the +user's data decides" the note excluded it from. + +**And it is a preference, because the automatic answer is a guess.** 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. `dr_plat::is_touch_first` reports the usual case per +platform and `dr_types::GroupNavigation` lets it be overridden; Settings names +what Automatic resolves to on this device rather than leaving it to be found by +pressing. + +**Still open: whether Local is a mode at all.** The rail now holds two kinds of +entry, and a third reading is available — that Compose and Repair are +categories with a canvas gesture attached, while Local edits *nothing* and +instead changes what every other category applies to. That would make it a +**scope**, not a peer of the tools, and would collapse the two sections into +one list of seven. It is the tidier model and a much larger change; deferred +until the two-section rail has been lived with. §1.1's complaint was that scope +was invisible, so this is the same argument arriving from the other end. + ### D-N3 — Collapsible panels, not tool tabs · **OPEN** For the expanded layout, extend `ui-refinement.md` Workstream C from diff --git a/platform/dr-plat/src/input.rs b/platform/dr-plat/src/input.rs new file mode 100644 index 0000000..d76dc86 --- /dev/null +++ b/platform/dr-plat/src/input.rs @@ -0,0 +1,57 @@ +//! How the interface is being driven — a finger, or a pointer. +//! +//! # Why this is a platform question +//! +//! It is the last `cfg(target_os)` anyone should want, and it earns its place +//! the way the rest of this crate does: the answer differs per platform, every +//! entry point needs it, and resolving it in `apps/` would mean writing it +//! twice. +//! +//! # Why it is not the layout class +//! +//! [`crate::display`] answers "how much room is there", and `dr-ui` turns that +//! into a layout class from the window's width, deliberately not from the +//! device (FR-UI-1). That was the right call and this does not revisit it: a +//! narrow desktop window still gets the compact layout, and a tablet in +//! landscape still gets the expanded one. +//! +//! This answers a different question — "what is the user pointing with" — +//! which width cannot stand in for. The two 12-inch targets DarkRoom is built +//! for are the same size and the same layout class, and one of them has no +//! hover, no modifier keys and a 44-pixel minimum target. +//! +//! # Why it is a guess, and says so +//! +//! There is no reliable runtime answer on either platform. Android devices +//! have touchscreens and can have a mouse attached; desktops have mice and can +//! have touchscreens. Reporting the *usual* case per platform and letting the +//! photographer override it is honest; probing input devices and being +//! confidently wrong is not. The override lives in +//! `dr_types::GroupNavigation`, which takes this as an input rather than +//! reading it. + +/// What this build is usually driven with. +/// +/// `true` on Android, `false` everywhere else. Deliberately a plain bool +/// rather than an enum: there are exactly two answers today, and the thing +/// that consumes it — [`dr_types::GroupNavigation::groups_in_rail`] — is a +/// choice between two controls. +pub const fn is_touch_first() -> bool { + cfg!(target_os = "android") +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn the_desktop_build_is_pointer_first() { + // Asserted rather than assumed, because the value is a `cfg!` and a + // typo in the target name compiles to a silent `false` — which on + // desktop is the right answer for the wrong reason and would pass + // unnoticed until the tablet build shipped with the desktop layout. + assert_eq!(is_touch_first(), cfg!(target_os = "android")); + #[cfg(not(target_os = "android"))] + assert!(!is_touch_first()); + } +} diff --git a/platform/dr-plat/src/lib.rs b/platform/dr-plat/src/lib.rs index d92e17d..d87721c 100644 --- a/platform/dr-plat/src/lib.rs +++ b/platform/dr-plat/src/lib.rs @@ -13,6 +13,7 @@ pub mod crash; pub mod diagnostics; pub mod display; +pub mod input; pub mod secrets; pub mod state; pub mod storage; @@ -23,6 +24,7 @@ pub use display::{ Bounds, DisplayInfo, DisplayProfile, DisplayServer, DisplaySurvey, FallbackReason, ProfileSource, }; +pub use input::is_touch_first; pub use secrets::{ EphemeralSecretStore, PlatformSecretStore, SecretError, SecretKind, SecretRef, SecretStore, }; diff --git a/ui/dr-ui/src/lib.rs b/ui/dr-ui/src/lib.rs index 9b3c8f3..acfcbbe 100644 --- a/ui/dr-ui/src/lib.rs +++ b/ui/dr-ui/src/lib.rs @@ -2330,6 +2330,12 @@ pub fn run(paths: Vec) -> 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>, } +/// 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); diff --git a/ui/dr-ui/src/settings_ui.rs b/ui/dr-ui/src/settings_ui.rs index 7045eae..28a694b 100644 --- a/ui/dr-ui/src/settings_ui.rs +++ b/ui/dr-ui/src/settings_ui.rs @@ -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( { 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() { diff --git a/ui/dr-ui/ui/app.slint b/ui/dr-ui/ui/app.slint index 1595120..d9a76bd 100644 --- a/ui/dr-ui/ui/app.slint +++ b/ui/dr-ui/ui/app.slint @@ -725,6 +725,18 @@ export component AppWindow inherits Window { in property 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 groups-in-rail: false; in property <[string]> adjust-tabs; in property adjust-active-tab: -1; callback adjust-tab-picked(int); @@ -882,6 +894,11 @@ export component AppWindow inherits Window { in property 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 settings-group-nav-selected: 0; + in property settings-group-nav-auto-says; in property <[string]> settings-format-labels; in property settings-format-selected: 0; in property settings-quality: 90; @@ -917,6 +934,7 @@ export component AppWindow inherits Window { in property settings-target-selected: 0; in property 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 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 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 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 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); } diff --git a/ui/dr-ui/ui/settings.slint b/ui/dr-ui/ui/settings.slint index adbfc61..7633b74 100644 --- a/ui/dr-ui/ui/settings.slint +++ b/ui/dr-ui/ui/settings.slint @@ -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 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 group-nav-auto-says; in property <[ScopeKind]> copy-scope-kinds; in property 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 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. diff --git a/ui/dr-ui/ui/toolrail.slint b/ui/dr-ui/ui/toolrail.slint index 1753208..883bc0b 100644 --- a/ui/dr-ui/ui/toolrail.slint +++ b/ui/dr-ui/ui/toolrail.slint @@ -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 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 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 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