Forms: lists reveal their cursor once laid out

- Select, Combobox and MultiSelect opened a long list at its top, the
  choice out of view: gpui skips a scroll asked before the list's first
  frame. Picker and Suggestions share forms::options::reveal, which Popup
  runs from a canvas inside the list
- test: a_long_list_opens_with_its_choice_in_view
- AGENTS: the scroll rule names the reveal

AGENTS.md

@@ -82,7 +82,7 @@ Ely GPUI Component. A component library for GPUI, in light and dark.
 - A window keeps one `feedback::Toaster` and renders one `ToastViewport`; anything holding the entity pushes to it. Messages share `primitives::Severity`.
 - Only gpui can build `ExternalPaths` with paths, so drop rules live in `forms::files::dropped`, where tests reach them.
 - gpui runs key bindings before key listeners. A container takes a child's bound key through the action with `capture_action`, as `Form` takes `Submit`.
-- `ScrollHandle::scroll_to_item` runs in the container's prepaint before its bounds are stored, so a first-frame call misfires. Call it from a child's prepaint once the handle has bounds, then request one frame (`navigation::editor_tabs`). Never poll layout with `request_animation_frame`.
+- `ScrollHandle::scroll_to_item` runs in the container's prepaint against last frame's bounds and overflow, so a call before a list's first frame does nothing. Call it from a child's prepaint once the handle has bounds, then request one frame (`navigation::editor_tabs`). A floating list reveals its cursor through `forms::options::reveal`. Never poll layout with `request_animation_frame`.
 - gpui animations run on the wall clock; timers run on the executor's clock, which tests advance by hand. A test that needs an animation settled turns on the theme's reduced motion. Clocks read `background_executor().now()` and wake only their own view.
 - Machine-read marks, QR codes and barcodes, paint `ink` on `paper`: dark on light in both themes, so cameras read them.
 - A layer over pressable content that must still pass scrolls to its ancestors uses `block_mouse_except_scroll`; `occlude` hides them from scrolls too.

src/forms/combobox.rs

@@ -165,6 +165,7 @@ impl RenderOnce for Combobox {
             step(&rows, rows.len().saturating_sub(1), 1)
         };
         let (anchor, scroll) = (picker.read(cx).anchor, picker.read(cx).scroll.clone());
+        let reveal = Picker::reveal(&picker, open, highlighted, cx);
         let pick: Pick = {
             let (id, rows, state, typing, on_change) = (
                 self.id.clone(),
@@ -256,7 +257,7 @@ impl RenderOnce for Combobox {
                         pick,
                         dismiss: None,
                         scroll: Some(&scroll),
-                        reveal: None,
+                        reveal,
                     }
                     .render(window, cx),
                 )

src/forms/mention.rs

@@ -7,7 +7,7 @@ use gpui::{
 
 use super::{
     Highlight, Input, TextInput,
-    options::{Choice, Pick, Popup, Run, step},
+    options::{Choice, Pick, Popup, reveal, step},
     text::{Down, Enter, Up},
 };
 use crate::theme::ActiveTheme;
@@ -151,21 +151,16 @@ impl Suggestions {
         let rows: Rc<[Choice]> = self.rows.into();
         let shown = open && anchor.is_some();
         let highlighted = usable(&rows, picking.read(cx).highlighted);
-        let revealed = picking.read(cx).revealed.filter(|_| shown);
+        if highlighted != picking.read(cx).highlighted {
-        let stored = (picking.read(cx).highlighted, picking.read(cx).revealed);
+            picking.update(cx, |picking, _| picking.highlighted = highlighted);
-        if (highlighted, revealed) != stored {
-            picking.update(cx, |picking, _| {
-                picking.highlighted = highlighted;
-                picking.revealed = revealed;
-            });
         }
-        let reveal = (shown && revealed != Some(highlighted)).then(|| {
+        let reveal = reveal(
-            let done = picking.clone();
+            &picking,
-            let mark: Run = Rc::new(move |_, cx| {
+            |picking| &mut picking.revealed,
-                done.update(cx, |picking, _| picking.revealed = Some(highlighted))
+            shown,
-            });
+            highlighted,
-            (highlighted, mark)
+            cx,
-        });
+        );
         let scroll = picking.read(cx).scroll.clone();
         log::debug!(
             "suggestions {:?}: open {open}, {} rows, anchor {anchor:?}",

src/forms/multi.rs

@@ -92,6 +92,7 @@ impl RenderOnce for MultiSelect {
                 picker.scroll.clone(),
             )
         };
+        let reveal = Picker::reveal(&picker, open, highlighted, cx);
         let commit: OnValues = {
             let (id, on_change) = (self.id.clone(), self.on_change);
             Rc::new(move |next, window, cx| {
@@ -222,7 +223,7 @@ impl RenderOnce for MultiSelect {
                             Picker::show(&close, false, at, cx)
                         })),
                         scroll: Some(&scroll),
-                        reveal: None,
+                        reveal,
                     }
                     .render(window, cx),
                 )

src/forms/options.rs

@@ -1,9 +1,10 @@
 use std::rc::Rc;
 
 use gpui::{
-    Animation, AnimationExt, AnyElement, App, Bounds, Corner, Div, ElementId, InteractiveElement,
+    Animation, AnimationExt, AnyElement, App, Bounds, Corner, Div, ElementId, Entity,
-    IntoElement, MouseButton, ParentElement, Pixels, ScrollHandle, SharedString, Stateful,
+    InteractiveElement, IntoElement, MouseButton, ParentElement, Pixels, ScrollHandle,
-    StatefulInteractiveElement, Styled, Window, anchored, canvas, deferred, div, prelude::*,
+    SharedString, Stateful, StatefulInteractiveElement, Styled, Window, anchored, canvas, deferred,
+    div, prelude::*,
 };
 
 use crate::{
@@ -211,6 +212,29 @@ pub(crate) fn float_height(
         .into_any_element()
 }
 
+/// The row a list scrolls into view: `at` while `shown` and not yet revealed there. A hidden list forgets.
+pub(crate) fn reveal<T: 'static>(
+    state: &Entity<T>,
+    revealed: fn(&mut T) -> &mut Option<usize>,
+    shown: bool,
+    at: usize,
+    cx: &mut App,
+) -> Option<(usize, Run)> {
+    let last = state.update(cx, |state, _| {
+        let last = revealed(state);
+        if !shown {
+            *last = None;
+        }
+        *last
+    });
+    (shown && last != Some(at)).then(|| {
+        let state = state.clone();
+        let mark: Run =
+            Rc::new(move |_, cx| state.update(cx, |state, _| *revealed(state) = Some(at)));
+        (at, mark)
+    })
+}
+
 /// A list floating under `anchor`, at least as wide. Rows leave focus where it was.
 pub(crate) struct Popup<'a> {
     pub id: ElementId,

src/forms/select.rs

@@ -9,7 +9,7 @@ use gpui::{
 use super::{
     Choice,
     input::text_size,
-    options::{OnValue, Pick, Popup, step},
+    options::{OnValue, Pick, Popup, Run, reveal, step},
 };
 use crate::{
     primitives::{Icon, IconName, tab_stop},
@@ -24,6 +24,7 @@ pub(crate) struct Picker {
     pub highlighted: usize,
     pub anchor: Bounds<Pixels>,
     pub scroll: ScrollHandle,
+    pub revealed: Option<usize>,
 }
 
 impl Picker {
@@ -31,10 +32,19 @@ impl Picker {
         state.update(cx, |picker, cx| {
             picker.open = open;
             picker.highlighted = highlighted;
-            picker.scroll.scroll_to_item(highlighted);
             cx.notify();
         });
     }
+
+    /// The row to scroll into view while the list shows `highlighted`.
+    pub fn reveal(
+        state: &Entity<Picker>,
+        shown: bool,
+        highlighted: usize,
+        cx: &mut App,
+    ) -> Option<(usize, Run)> {
+        reveal(state, |picker| &mut picker.revealed, shown, highlighted, cx)
+    }
 }
 
 /// Sizes an invisible layer to its parent and records the bounds in `state`.
@@ -163,6 +173,7 @@ pub(crate) fn listing(
             picker.scroll.clone(),
         )
     };
+    let reveal = Picker::reveal(&picker, open, highlighted, cx);
     let pick: Pick = {
         let (rows, picker) = (rows.clone(), picker.clone());
         Rc::new(move |ix, window, cx| {
@@ -224,7 +235,7 @@ pub(crate) fn listing(
                         Picker::show(&close, false, at, cx)
                     })),
                     scroll: Some(&scroll),
-                    reveal: None,
+                    reveal,
                 }
                 .render(window, cx),
             )

src/forms/tests/choices.rs

@@ -1,11 +1,15 @@
 use gpui::{
-    AppContext as _, Context, Entity, IntoElement, KeyUpEvent, Keystroke, Render, SharedString,
+    AppContext as _, Context, Entity, IntoElement, KeyUpEvent, Keystroke, Modifiers, ParentElement,
-    TestAppContext, VisualTestContext, Window,
+    Render, SharedString, Styled, TestAppContext, VisualTestContext, Window, div, point, px,
 };
 
 use super::setup;
-use crate::forms::{
+use crate::{
-    Checkbox, Choice, Combobox, MultiSelect, RadioGroup, Rating, Select, Slider, TextInput,
+    forms::{
+        Checkbox, Choice, Combobox, MultiSelect, RadioGroup, Rating, Select, Slider, TextInput,
+    },
+    motion,
+    theme::{ActiveTheme, ControlSize, Theme},
 };
 
 fn abc(middle_off: bool) -> [Choice; 3] {
@@ -83,6 +87,71 @@ fn select_opens_moves_and_picks_from_the_keyboard(cx: &mut TestAppContext) {
     assert_eq!(view.read_with(cx, |view, _| view.chosen.clone()), "b");
 }
 
+/// A select of thirty rows, low in the window, and the rows it picked.
+struct Long {
+    chosen: SharedString,
+    picks: Vec<SharedString>,
+}
+
+impl Render for Long {
+    fn render(&mut self, _: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
+        let view = cx.entity();
+        let rows = (0..30).map(|ix| Choice::new(format!("r{ix:02}"), format!("Row {ix:02}")));
+        div().pt(px(200.0)).w(px(240.0)).child(
+            Select::new("long", rows)
+                .selected(self.chosen.clone())
+                .on_change(move |value, _, cx| {
+                    view.update(cx, |view, cx| {
+                        view.chosen = value.clone();
+                        view.picks.push(value.clone());
+                        cx.notify();
+                    })
+                }),
+        )
+    }
+}
+
+#[gpui::test]
+fn a_long_list_opens_with_its_choice_in_view(cx: &mut TestAppContext) {
+    setup(cx);
+    cx.update(|cx| Theme::update(cx, |theme| theme.reduced_motion = true));
+    let (view, cx) = cx.add_window_view(|_, _| Long {
+        chosen: "r25".into(),
+        picks: Vec::new(),
+    });
+    let (trigger, list) = cx.update(|window, cx| {
+        let theme = cx.theme();
+        (
+            theme
+                .control_height(ControlSize::Md)
+                .to_pixels(window.rem_size()),
+            theme.list_max_height().to_pixels(window.rem_size()),
+        )
+    });
+    let field = point(px(40.0), px(200.0) + trigger / 2.0);
+    cx.simulate_mouse_move(field, None, Modifiers::none());
+    cx.simulate_click(field, Modifiers::none());
+    frames(cx);
+    let foot = point(
+        px(40.0),
+        px(200.0) + trigger + motion::NUDGE + list - px(8.0),
+    );
+    cx.simulate_mouse_move(foot, None, Modifiers::none());
+    cx.simulate_click(foot, Modifiers::none());
+    frames(cx);
+    let picks = view.read_with(cx, |view, _| view.picks.clone());
+    assert_eq!(picks, ["r25"], "the list opens with its choice at the foot");
+}
+
+/// Frames 2ms apart, past reduced motion's 1ms entrances.
+fn frames(cx: &mut VisualTestContext) {
+    for _ in 0..3 {
+        std::thread::sleep(std::time::Duration::from_millis(2));
+        cx.update(|window, _| window.refresh());
+        cx.run_until_parked();
+    }
+}
+
 struct Level {
     value: f64,
 }