Open the film list as a popup, so every stock can be scrolled to

The open stock list showed ten rows, None to Kodak Kodachrome 64, and
the other eighteen - all seven black-and-white stocks among them - could
not be reached. The data was whole; the list could not be scrolled.

Reproduced on the manual rig (Xvfb, xdotool, the automation hook):

- a drag on the list scrolled the develop column, never the list;
- a wheel run over the list scrolled the column past it, whenever the
  column had scrolled under that pointer in the last 800 ms - which is
  how the list is reached, by wheeling the column down to it. After a
  pause and a pointer move the wheel did reach the list;
- no key did anything.

The cause is Slint's routing, not the list. Since 2d878c2 the list was a
Flickable inside the develop column's Flickable, and Slint offers every
pointer event to the outermost Flickable first
(input_event_filter_before_children, i-slint-core 1.17.1 flickable.rs).
The column holds a press back (DelayForwarding) and intercepts the first
move past 8 px on the axis it can scroll, so the list never saw a drag.
For the wheel it intercepts while its own last wheel event is under
800 ms old and within 2 px, and always for a touchpad gesture that opens
with TouchPhase::Started - so on a touchpad the list could get no wheel
at all.

The list is now a PopupWindow under the Film row. A popup is its own
item tree: while it is open, events go to it and to nothing beneath it,
so the list scrolls by wheel, drag and flick however the panel is nested
and whatever the column did last. The alternative, standing the column
down while the pointer is over the list (the sliders' hover trick), fixes
the drag but not the wheel - `interactive: false` does not gate wheel
interception - so it would have left the bug for touchpad users.

The reason 2d878c2 bounded the list still holds: it is at most 320 px
and never lengthens the column, and now it covers the sliders instead of
pushing them down. The column cannot be scrolled while it is open, which
suits a one-click question; it closes on choosing, on Escape or Back, or
on a press outside it.

Keys, with the list open: Up and Down move along it from the chosen
stock and scroll it into view, Enter chooses, Escape or Back closes it
unchanged. A popup is its own focus tree, so the keys are taken when it
opens and Slint returns focus to the develop view's scope when it closes.
The Film row gains a button role, so a screen reader and the automation
hook can name it.

docs/gestures.md

@@ -5,7 +5,7 @@
 
 Every entry here is extracted from the comment beside the code that implements it, so this file cannot describe a gesture the application does not have. Add one by writing a `GESTURE:` block next to the implementation; there is nowhere else to write it.
 
-74 gestures, in 7 places.
+75 gestures, in 7 places.
 
 ## Develop
 
@@ -17,7 +17,18 @@ Every entry here is extracted from the comment beside the code that implements i
 
 Sampling a neutral is the first move of the tonal pass — every colour judgement afterwards is measured against where the grey was put — and guessing at two sliders until a wall stops looking green is the wrong way round. One click is one sample and one step to undo; the sliders stay, because a sampled neutral is where the decision starts rather than where it ends.
 
-<sub>`ui/dr-ui/ui/adjust.slint:158`</sub>
+<sub>`ui/dr-ui/ui/adjust.slint:159`</sub>
+
+### Choose a film stock from the keyboard
+
+- **Touch** — Tap the Film row, flick the list, tap a stock
+- **Pointer** — Click the Film row, then a stock; the wheel or a drag scrolls the list
+- **Keyboard** — With the list open, `↑` and `↓` move along it, `Enter` chooses, and `Escape` or `Back` closes it unchanged
+- **See it** — [in the manual](manual/README.md#film)
+
+The list is longer than it is tall, so a way to walk it that cannot be lost to the column's scrolling is part of it being reachable at all. Focus goes back to the photograph's keys when it closes.
+
+<sub>`ui/dr-ui/ui/adjust.slint:1465`</sub>
 
 ### Magnify the photograph by any amount
 

docs/manual/README.md

@@ -233,7 +233,9 @@ opacity are in the panel. Heal blends; clone copies.
 
 The `Film` chooser at the head of Adjust applies a spectral simulation of a
 named stock; below it, the print exposure and push controls a film has and a
-sensor does not.
+sensor does not. The list opens over the column and scrolls on its own — by
+wheel, drag or flick, or with `Up`, `Down` and `Enter` — down to the
+black-and-white stocks at its end.
 
 ![Choosing Velvia, then holding Before](media/film.gif)
 

docs/manual/index.html

@@ -296,7 +296,9 @@ opacity are in the panel. Heal blends; clone copies.</p>
 <h3 id="film">Film</h3>
 <p>The <code>Film</code> chooser at the head of Adjust applies a spectral simulation of a
 named stock; below it, the print exposure and push controls a film has and a
-sensor does not.</p>
+sensor does not. The list opens over the column and scrolls on its own — by
+wheel, drag or flick, or with <code>Up</code>, <code>Down</code> and <code>Enter</code> — down to the
+black-and-white stocks at its end.</p>
 <figure><img loading="lazy" src="media/film.gif" alt="Choosing Velvia, then holding Before"><figcaption>Choosing Velvia, then holding Before</figcaption></figure>
 <h3 id="history-snapshots-presets">History, snapshots, presets</h3>
 <p>Every change is a step; <code>Undo</code> and the History panel walk them. <code>Snapshot</code>

ui/dr-ui/src/gesture_book.rs

@@ -32,6 +32,14 @@ pub const GESTURES: &[Gesture] = &[
         keys: "",
         manual: "white-balance-from-the-photograph",
     },
+    Gesture {
+        title: "Choose a film stock from the keyboard",
+        section: "Develop",
+        touch: "Tap the Film row, flick the list, tap a stock",
+        pointer: "Click the Film row, then a stock; the wheel or a drag scrolls the list",
+        keys: "With the list open, ↑ and ↓ move along it, Enter chooses, and Escape or Back closes it unchanged",
+        manual: "film",
+    },
     Gesture {
         title: "Magnify the photograph by any amount",
         section: "Develop",

ui/dr-ui/ui/adjust.slint

@@ -9,6 +9,7 @@
 
 import { Theme } from "theme.slint";
 import { Develop } from "session.slint";
+import { Keys } from "keys.slint";
 import { PanelHeading, Label, Value, Caption, Button, IconButton, Swatch } from "widgets.slint";
 import { SliderTrack, ControlRow, CurveEditor, Segmented, ChoiceChip, ChipGrid, Check } from "controls.slint";
 
@@ -1232,16 +1233,9 @@ export component GroupStrip inherits Rectangle {
 // The column is closed as a whole from the status strip instead, which is the
 // control that was actually wanted.
 export component AdjustPanel inherits Rectangle {
-    /// Whether the stock list is open. Pure interface state: it changes no
-    /// parameter and the core never hears about it.
-    ///
-    /// Private to the panel rather than in `Adjustments`, and deliberately: it
-    /// is a lid, and two drawings of this panel may honestly have their lids
-    /// in different positions.
-    private property <bool> film-expanded: false;
     /// One stock. Shorter than a touch target on purpose — see the row below.
     private property <length> film-row-height: 32px;
-    /// How much column the open list may take before it scrolls instead.
+    /// How tall the open list may be before it scrolls instead.
     private property <length> film-list-max: 320px;
 
     background: Theme.surface;
@@ -1317,19 +1311,51 @@ export component AdjustPanel inherits Rectangle {
             // thousand pixels of column standing between the photographer and
             // every slider below it.
             //
-            // No PopupWindow, no scroller of its own: the develop column
+            // **Opened as a popup over the column, not as rows inside it.**
-            // scrolls as one, so the list is simply taller content while it is
+            //
-            // open, and there is no second gesture to lose a drag to.
+            // It has been both of the other things, and each lost stocks:
+            //
+            //   - Laid out at full height, two dozen stocks were the tallest
+            //     thing in the column, and reaching Velvia scrolled every
+            //     slider below it off the screen — the answer to a question
+            //     asked once pushing aside the controls used constantly.
+            //   - Bounded, with a Flickable of its own (2d878c2), it was a
+            //     scroller inside the column's scroller, and Slint routes a
+            //     gesture to the *outermost* Flickable first. A drag or a
+            //     flick on the list was claimed by the column before the list
+            //     saw the press, and a wheel was too while the column had
+            //     scrolled in the last 800ms under a still pointer — which is
+            //     exactly how a list is arrived at, wheeling the column down
+            //     to it. The eighteen stocks past Kodachrome 64, every
+            //     black-and-white one among them, could not be reached, and
+            //     looked deleted.
+            //
+            // A PopupWindow is a separate tree: while it is open, input goes
+            // to it and to nothing under it, so the list scrolls by wheel,
+            // drag and flick however deeply the panel is nested and whatever
+            // the column did last. It keeps the reason the list was bounded —
+            // it never lengthens the column, so no slider moves — and, as it
+            // covers the column rather than pushing it, it does not move the
+            // sliders even while it is open.
+            //
+            // The column cannot be scrolled while it is open. That is the
+            // right trade for a question answered in one click: the popup is
+            // closed by choosing, by Escape or Back, or by pressing anywhere
+            // outside it.
             summary := TouchArea {
                 height: Theme.touch-target;
                 mouse-cursor: pointer;
-                clicked => { root.film-expanded = !root.film-expanded; }
+                clicked => { film-list.show(); }
+                accessible-role: button;
+                accessible-label: "Film";
+                accessible-value: Adjustments.film-stocks[Adjustments.film-selected];
+                accessible-action-default => { film-list.show(); }
 
                 Rectangle {
                     border-radius: Theme.radius;
                     border-width: 1px;
                     border-color: Theme.rule;
-                    background: summary.pressed ? Theme.pressed
+                    background: summary.pressed || film-list.is-open ? Theme.pressed
                                 : (summary.has-hover ? Theme.hover : transparent);
 
                     HorizontalLayout {
@@ -1352,58 +1378,74 @@ export component AdjustPanel inherits Rectangle {
                             vertical-alignment: center;
                         }
                         Label {
-                            text: root.film-expanded ? "\u{25be}" : "\u{25b8}";
+                            text: film-list.is-open ? "\u{25be}" : "\u{25b8}";
                             vertical-alignment: center;
                         }
                     }
                 }
-            }
 
-            // **Its own scroller, bounded.** Two dozen stocks laid out at full
+                // Directly under the summary and as wide as it. Slint keeps a
-            // height made the list the tallest thing in the column, so the
+                // popup inside the window, so a summary near the bottom edge
-            // develop panel scrolled as one and reaching Velvia dragged every
+                // opens its list upward over itself rather than off-screen.
-            // slider below it off the screen — the answer to a question that
+                film-list := PopupWindow {
-            // is asked once pushing aside the controls used constantly.
+                    x: 0px;
-            //
+                    y: parent.height + Theme.gap-sm;
-            // The viewport is counted rather than measured, for the reason the
+                    width: parent.width;
-            // tool rail records: a viewport that asks a layout how tall it
-            // wants to be, while the layout takes its height from the viewport,
-            // is a cycle Slint settles by handing back the height it was given
-            // — and the content is then clipped in silence instead of
-            // scrolling. Every row here is `film-row-height` by construction,
-            // so multiplying is exact.
-            if root.film-expanded: Flickable {
                     height: min(Adjustments.film-stocks.length * root.film-row-height, root.film-list-max);
+                    close-policy: PopupClosePolicy.close-on-click-outside;
+
+                    Rectangle {
+                        background: Theme.surface;
+                        border-radius: Theme.radius;
+                        border-width: 1px;
+                        border-color: Theme.rule;
+                        clip: true;
+
+                        // The viewport is counted rather than measured, for
+                        // the reason the tool rail records: a viewport that
+                        // asks a layout how tall it wants to be, while the
+                        // layout takes its height from the viewport, is a cycle
+                        // Slint settles by handing back the height it was given
+                        // — and the content is then clipped in silence instead
+                        // of scrolling. Every row is `film-row-height` by
+                        // construction, so multiplying is exact.
+                        film-flick := Flickable {
+                            width: parent.width;
+                            height: parent.height;
                             viewport-width: self.width;
                             viewport-height: Adjustments.film-stocks.length * root.film-row-height;
 
-            list := VerticalLayout {
+                            VerticalLayout {
                                 height: parent.viewport-height;
                                 spacing: 0px;
                                 alignment: start;
 
                                 for stock[i] in Adjustments.film-stocks: film-row := TouchArea {
-                    // Shorter than a touch target, which is a deliberate
+                                    // Shorter than a touch target, which is a
-                    // exception and the only one here: a list row's neighbours
+                                    // deliberate exception and the only one
-                    // are other rows, so growing the hit area the way a chip
+                                    // here: a list row's neighbours are other
-                    // does would put each row's target over the one above it
+                                    // rows, so growing the hit area the way a
-                    // and hand taps to the wrong film.
+                                    // chip does would put each row's target
-                    height: 32px;
+                                    // over the one above it and hand taps to
+                                    // the wrong film.
+                                    height: root.film-row-height;
                                     mouse-cursor: pointer;
+                                    // Closed on choosing. The answer is now in
+                                    // the summary row, and leaving two dozen
+                                    // entries open after the question has been
+                                    // answered is what this control was
+                                    // collapsed to avoid.
                                     clicked => {
                                         Adjustments.film-picked(i);
-                        // Closed on choosing. The answer is now in the summary
+                                        film-list.close();
-                        // row, and leaving two dozen entries open after the
-                        // question has been answered is the behaviour this
-                        // control was collapsed to avoid.
-                        root.film-expanded = false;
                                     }
 
                                     Rectangle {
                                         border-radius: Theme.radius;
                                         background: i == Adjustments.film-selected ? Theme.active
                                                     : (film-row.pressed ? Theme.pressed
-                                       : (film-row.has-hover ? Theme.hover : transparent));
+                                                       : (film-row.has-hover || i == film-keys.cursor
+                                                          ? Theme.hover : transparent));
 
                                         HorizontalLayout {
                                             padding-left: Theme.gap;
@@ -1420,6 +1462,79 @@ export component AdjustPanel inherits Rectangle {
                             }
                         }
 
+                        // GESTURE: Choose a film stock from the keyboard
+                        //   where:   Develop
+                        //   touch:   Tap the Film row, flick the list, tap a stock
+                        //   pointer: Click the Film row, then a stock; the wheel
+                        //            or a drag scrolls the list
+                        //   keys:    With the list open, `Up` and `Down` move
+                        //            along it, `Enter` chooses, and `Escape` or
+                        //            `Back` closes it unchanged
+                        //   why:     The list is longer than it is tall, so a
+                        //            way to walk it that cannot be lost to the
+                        //            column's scrolling is part of it being
+                        //            reachable at all. Focus goes back to the
+                        //            photograph's keys when it closes.
+                        //   manual:  film
+                        //
+                        // The keys, on a holder of no size inside the popup.
+                        // A popup is its own focus tree, so this takes the keys
+                        // when the list opens — its contents are built afresh
+                        // each time — and Slint hands them back to the develop
+                        // view's scope when it closes, however it closed.
+                        film-keys := FocusScope {
+                            width: 0px;
+                            height: 0px;
+                            /// The row the keys are on. Starts on the chosen
+                            /// stock, so opening and pressing Enter changes
+                            /// nothing.
+                            property <int> cursor: Adjustments.film-selected;
+
+                            init => {
+                                self.focus();
+                                self.reveal(self.cursor);
+                            }
+
+                            /// Scroll just far enough that row `i` is whole.
+                            function reveal(i: int) {
+                                if (i * root.film-row-height + film-flick.viewport-y < 0px) {
+                                    film-flick.viewport-y = -i * root.film-row-height;
+                                } else if ((i + 1) * root.film-row-height + film-flick.viewport-y > film-flick.height) {
+                                    film-flick.viewport-y = film-flick.height - (i + 1) * root.film-row-height;
+                                }
+                            }
+
+                            function step(by: int) {
+                                self.cursor = clamp(self.cursor + by, 0, Adjustments.film-stocks.length - 1);
+                                self.reveal(self.cursor);
+                            }
+
+                            // KEYMAP: Develop
+                            key-pressed(event) => {
+                                if (Keys.chord(event) == "Down") {
+                                    self.step(1);
+                                    return accept;
+                                }
+                                if (Keys.chord(event) == "Up") {
+                                    self.step(-1);
+                                    return accept;
+                                }
+                                if (Keys.chord(event) == "Enter") {
+                                    Adjustments.film-picked(self.cursor);
+                                    film-list.close();
+                                    return accept;
+                                }
+                                if (Keys.chord(event) == "Escape" || Keys.chord(event) == "Back") {
+                                    film-list.close();
+                                    return accept;
+                                }
+                                return reject;
+                            }
+                        }
+                    }
+                }
+            }
+
             // Only for a negative. A reversal stock has no paper — it is the
             // photograph as it comes — so offering the choice would be
             // offering something that cannot happen.