Messaging · GIF picker: a GIF focused past the fold scrolls into view

Each GIF holds its own focus handle, and a canvas around it, ring and
all, sets the results box's offset once per focus from its painted
bounds. layout::bring_into_view now works along either axis; the data
table's header and the video timeline pass their sideways one.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

AGENTS.md

@@ -92,7 +92,7 @@ Ely GPUI Component. A component library for GPUI, in light and dark.
 - Only gpui can build `ExternalPaths` with paths, so drop rules live in `forms::files::dropped`, where tests reach them. `taken` adds file kinds by extension, for drops and the dialog alike (`DropZone::kinds`).
 - A stroke drawn with the pointer goes through `forms::pen`: a press starts it, a drag extends it, and the lift hands it over, in or out of the pad (`SignaturePad`, `generative::InpaintCanvas`).
 - 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 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, and a strip its focused item, through `forms::options::reveal` and `revealer`. A focused Tab stop deeper in a scroll box sets the box's offset whole from its own painted bounds in prepaint, once per focus, through `layout::bring_into_view`. 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, and a strip its focused item, through `forms::options::reveal` and `revealer`. A focused Tab stop deeper in a scroll box sets the box's offset whole from its own painted bounds in prepaint, once per focus, through `layout::bring_into_view` along the box's axis; the canvas measures the stop with its ring (`messaging::GifPicker`). 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/generative/video.rs

@@ -1,7 +1,7 @@
 use std::{rc::Rc, time::Duration};
 
 use gpui::{
-    App, ElementId, InteractiveElement, IntoElement, MouseButton, ParentElement, RenderOnce,
+    App, Axis, ElementId, InteractiveElement, IntoElement, MouseButton, ParentElement, RenderOnce,
     ScrollHandle, SharedString, StatefulInteractiveElement, Styled, Window, canvas, div,
     prelude::*, relative, transparent_black,
 };
@@ -233,7 +233,7 @@ impl RenderOnce for VideoGenerationTimeline {
                                         revealed.update(cx, |revealed, _| {
                                             *revealed = Some(key.clone())
                                         });
-                                        if bring_into_view(&scroll, bounds) {
+                                        if bring_into_view(&scroll, bounds, Axis::Horizontal) {
                                             log::info!(
                                                 "video timeline: shot {key} scrolled into view"
                                             );

src/layout/scroll.rs

@@ -16,18 +16,25 @@ use crate::{motion, theme::ActiveTheme};
 
 const IDLE: Duration = Duration::from_millis(900);
 
-/// Sets `scroll`'s sideways offset whole so `item`, as painted, sits inside the box, its start first when it is wider; true when it moved.
+/// Sets `scroll`'s offset along `axis` whole so `item`, as painted, sits inside the box, its start first when it is longer; true when it moved.
-pub(crate) fn bring_into_view(scroll: &ScrollHandle, item: Bounds<Pixels>) -> bool {
+pub(crate) fn bring_into_view(scroll: &ScrollHandle, item: Bounds<Pixels>, axis: Axis) -> bool {
     let (frame, offset) = (scroll.bounds(), scroll.offset());
-    let shift = if item.left() < frame.left() {
+    let ((start, end), (first, last)) = match axis {
-        frame.left() - item.left()
+        Axis::Horizontal => ((item.left(), item.right()), (frame.left(), frame.right())),
-    } else if item.right() > frame.right() {
+        Axis::Vertical => ((item.top(), item.bottom()), (frame.top(), frame.bottom())),
-        (frame.right() - item.right()).max(frame.left() - item.left())
+    };
+    let shift = if start < first {
+        first - start
+    } else if end > last {
+        (last - end).max(first - start)
     } else {
         Pixels::ZERO
     };
     if shift != Pixels::ZERO {
-        scroll.set_offset(point(offset.x + shift, offset.y));
+        scroll.set_offset(match axis {
+            Axis::Horizontal => point(offset.x + shift, offset.y),
+            Axis::Vertical => point(offset.x, offset.y + shift),
+        });
     }
     shift != Pixels::ZERO
 }

src/messaging/gifs.rs

@@ -1,17 +1,17 @@
 use std::rc::Rc;
 
 use gpui::{
-    App, Context, ElementId, Entity, ImageSource, InteractiveElement, IntoElement, MouseButton,
+    App, Axis, Context, ElementId, Entity, FocusHandle, ImageSource, InteractiveElement,
-    ParentElement, RenderOnce, SharedString, StatefulInteractiveElement, Styled, Subscription,
+    IntoElement, MouseButton, ParentElement, RenderOnce, ScrollHandle, SharedString,
-    Window, div, prelude::*, rems,
+    StatefulInteractiveElement, Styled, Subscription, Window, canvas, div, prelude::*, rems,
 };
 
 use crate::{
     buttons::{Button, ButtonVariant},
     forms::{Input, InputEvent, OnValue, TextInput},
-    layout::{Masonry, on_axis},
+    layout::{Masonry, bring_into_view, on_axis},
     motion::Skeleton,
-    primitives::{FocusRing, Icon, IconName, Image, Tooltip, checked_ratio, framed},
+    primitives::{FocusRing, Icon, IconName, Image, Tooltip, checked_ratio, framed, tab_stop},
     theme::{ActiveTheme, ControlSize, IconSize, Radius, TextSize},
 };
 
@@ -147,6 +147,26 @@ impl RenderOnce for GifPicker {
         search.update(cx, |search, _| search.on_query = self.on_query.clone());
         let input = search.read(cx).input.clone();
         let empty = input.read(cx).text().trim().is_empty();
+        let scroll = window
+            .use_keyed_state((self.id.clone(), "scroll"), cx, |_, _| ScrollHandle::new())
+            .read(cx)
+            .clone();
+        let revealed =
+            window.use_keyed_state(
+                (self.id.clone(), "revealed"),
+                cx,
+                |_, _| None::<SharedString>,
+            );
+        let focuses: Vec<FocusHandle> = match &self.gifs {
+            Gifs::Loading => Vec::new(),
+            Gifs::Found(found) => found
+                .iter()
+                .map(|gif| {
+                    let id = (self.id.clone(), format!("gif-focus-{}", gif.key)).into();
+                    tab_stop(id, true, window, cx)
+                })
+                .collect(),
+        };
         let theme = cx.theme();
         let colors = &theme.colors;
         let round = theme.radius(Radius::Md);
@@ -186,15 +206,46 @@ impl RenderOnce for GifPicker {
                 .collect(),
             Gifs::Found(found) => found
                 .into_iter()
-                .map(|gif| {
+                .zip(focuses)
+                .map(|(gif, focus)| {
                     let (key, on_pick, owner) =
                         (gif.key.clone(), self.on_pick.clone(), self.id.clone());
-                    div()
+                    let (shown, revealed, scroll) =
+                        (gif.key.clone(), revealed.clone(), scroll.clone());
+                    let reveal = canvas(
+                        {
+                            let focus = focus.clone();
+                            move |bounds, window, cx| {
+                                let focused = focus.is_focused(window);
+                                let held = revealed.read(cx).as_ref() == Some(&shown);
+                                if focused && !held {
+                                    revealed
+                                        .update(cx, |revealed, _| *revealed = Some(shown.clone()));
+                                    if bring_into_view(&scroll, bounds, Axis::Vertical) {
+                                        log::info!("gif picker: {shown} scrolled into view");
+                                        window.request_animation_frame();
+                                    }
+                                } else if !focused && held {
+                                    revealed.update(cx, |revealed, _| *revealed = None);
+                                }
+                            }
+                        },
+                        |_, _, _, _| {},
+                    )
+                    .absolute()
+                    .top_0()
+                    .left_0()
+                    .size_full();
+                    let tile = div()
                         .id((self.id.clone(), format!("gif-{}", gif.key)))
+                        .debug_selector({
+                            let key = gif.key.clone();
+                            move || format!("gif {key}")
+                        })
                         .rounded(round)
                         .border_1()
                         .border_color(gpui::transparent_black())
-                        .tab_index(0)
+                        .track_focus(&focus)
                         .focus_ring(cx)
                         .cursor_pointer()
                         .tooltip(Tooltip::text(gif.title.clone()))
@@ -214,7 +265,11 @@ impl RenderOnce for GifPicker {
                                 .size_full()
                                 .rounded(round),
                             ),
-                        )
+                        );
+                    div()
+                        .relative()
+                        .child(tile)
+                        .child(reveal)
                         .into_any_element()
                 })
                 .collect(),
@@ -256,6 +311,8 @@ impl RenderOnce for GifPicker {
                 on_axis(
                     div()
                         .id((self.id.clone(), "results"))
+                        .debug_selector(|| "gif-results".into())
+                        .track_scroll(&scroll)
                         .max_h(theme.list_max_height())
                         .overflow_y_scroll(),
                 )

src/messaging/tests/pickers.rs

@@ -180,6 +180,23 @@ fn a_gif_is_a_tab_stop_that_space_picks(cx: &mut TestAppContext) {
     assert_eq!(heard(&host, cx), ["pick b"]);
 }
 
+#[gpui::test]
+fn a_gif_past_the_fold_comes_into_view_when_focused(cx: &mut TestAppContext) {
+    let keys = [
+        "g0", "g1", "g2", "g3", "g4", "g5", "g6", "g7", "g8", "g9", "g10", "g11",
+    ];
+    let (_, cx) = gifing(Gifs::Found(keys.iter().map(|key| gif(key)).collect()), cx);
+    tab_to(15, cx);
+    let (tile, results) = (
+        cx.debug_bounds("gif g11").expect("the last GIF draws"),
+        cx.debug_bounds("gif-results").expect("the results draw"),
+    );
+    assert!(
+        tile.top() >= results.top() && tile.bottom() <= results.bottom(),
+        "{tile:?} inside {results:?}"
+    );
+}
+
 #[gpui::test]
 fn nothing_found_says_so(cx: &mut TestAppContext) {
     let (_, cx) = gifing(Gifs::Found(Vec::new()), cx);

src/tables/header.rs

@@ -1,5 +1,5 @@
 use gpui::{
-    App, Bounds, Div, DragMoveEvent, ElementId, Entity, EntityId, FocusHandle, FontWeight,
+    App, Axis, Bounds, Div, DragMoveEvent, ElementId, Entity, EntityId, FocusHandle, FontWeight,
     InteractiveElement, IntoElement, MouseButton, ParentElement, Pixels, Rems, SharedString,
     Stateful, StatefulInteractiveElement, Styled, Window, canvas, div, prelude::*,
     transparent_black,
@@ -97,7 +97,7 @@ fn reveal(
     }
     let scroll = view.read(cx).sideways.clone();
     view.update(cx, |view, _| view.revealed = Some(key.clone()));
-    if bring_into_view(&scroll, head) {
+    if bring_into_view(&scroll, head, Axis::Horizontal) {
         log::info!("data table: header {key} scrolled into view");
         window.request_animation_frame();
     }