Fix scrollbar regression

lib/ruin_app/src/lib.rs

@@ -1824,11 +1824,12 @@ fn focused_element_for_pointer(
     interaction_tree: &InteractionTree,
     event: &PointerEvent,
 ) -> Option<ElementId> {
-    interaction_tree
+    let hit_path = interaction_tree.hit_path(event.position);
-        .hit_path(event.position)
+    hit_path
         .iter()
         .rev()
         .find_map(|target| target.focusable.then_some(target.element_id).flatten())
+        .or_else(|| hit_path.iter().rev().find_map(|target| target.element_id))
 }
 
 fn element_contains_element(
@@ -1869,7 +1870,6 @@ fn key_handler_for_focus<'a>(
     let focused_element = focused_element?;
     focused_ancestor_chain(&interaction_tree.root, focused_element)?
         .into_iter()
-        .rev()
         .find_map(|element_id| handlers.get(&element_id))
 }
 
@@ -1933,6 +1933,7 @@ struct SignalInner<T> {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use ruin_ui::{KeyboardModifiers, Point, UiRuntime, WindowSpec};
 
     #[derive(Clone, Debug, PartialEq, Eq)]
     struct NamedValue(&'static str);
@@ -1989,4 +1990,470 @@ mod tests {
 
         assert_eq!(*seen_value.borrow(), Some(NamedValue("inner")));
     }
+
+    #[test]
+    fn key_dispatch_prefers_the_nearest_focused_ancestor_handler() {
+        let outer_id = ElementId::new(41);
+        let inner_id = ElementId::new(42);
+        let root = Element::column().pointer_events(false).child(
+            Element::column()
+                .id(outer_id)
+                .width(160.0)
+                .height(120.0)
+                .focusable(true)
+                .child(
+                    Element::column()
+                        .id(inner_id)
+                        .width(120.0)
+                        .height(80.0)
+                        .focusable(true),
+                ),
+        );
+        let snapshot = ruin_ui::layout_snapshot(1, UiSize::new(200.0, 120.0), &root);
+        let outer_hits = Rc::new(StdCell::new(0usize));
+        let inner_hits = Rc::new(StdCell::new(0usize));
+
+        let mut handlers = HashMap::<ElementId, KeyHandler>::new();
+        handlers.insert(
+            outer_id,
+            Rc::new({
+                let outer_hits = Rc::clone(&outer_hits);
+                move |_, _| {
+                    outer_hits.set(outer_hits.get() + 1);
+                    true
+                }
+            }),
+        );
+        handlers.insert(
+            inner_id,
+            Rc::new({
+                let inner_hits = Rc::clone(&inner_hits);
+                move |_, _| {
+                    inner_hits.set(inner_hits.get() + 1);
+                    true
+                }
+            }),
+        );
+
+        let handler = key_handler_for_focus(&handlers, Some(inner_id), &snapshot.interaction_tree)
+            .expect("focused element should resolve a key handler");
+        let _ = handler(
+            &KeyboardEvent::new(
+                0,
+                KeyboardEventKind::Pressed,
+                KeyboardKey::ArrowDown,
+                KeyboardModifiers::default(),
+                None,
+            ),
+            &snapshot.interaction_tree,
+        );
+
+        assert_eq!(inner_hits.get(), 1);
+        assert_eq!(outer_hits.get(), 0);
+    }
+
+    #[test]
+    fn scroll_box_arrow_keys_work_after_clicking_text_content() {
+        let offset_slot = Rc::new(RefCell::new(None::<Signal<f32>>));
+        let render = render_with_context(Rc::new(RenderState::default()), {
+            let offset_slot = Rc::clone(&offset_slot);
+            move || {
+                let offset = use_signal(|| 0.0_f32);
+                *offset_slot.borrow_mut() = Some(offset.clone());
+                scroll_box()
+                    .height(120.0)
+                    .offset_y(offset)
+                    .children(text().children(
+                        "line 01\nline 02\nline 03\nline 04\nline 05\nline 06\nline 07\nline 08\nline 09\nline 10\nline 11\nline 12",
+                    ))
+            }
+        });
+        let offset = offset_slot
+            .borrow()
+            .clone()
+            .expect("scroll signal should have been captured");
+        let scrollbox_id = render
+            .view
+            .element
+            .id
+            .expect("scroll box should receive an element id");
+        let snapshot =
+            ruin_ui::layout_snapshot(1, UiSize::new(260.0, 160.0), render.view.element());
+        let focused = focused_element_for_pointer(
+            &snapshot.interaction_tree,
+            &PointerEvent::new(
+                1,
+                Point::new(12.0, 12.0),
+                PointerEventKind::Down {
+                    button: PointerButton::Primary,
+                },
+            ),
+        );
+
+        assert_eq!(focused, Some(scrollbox_id));
+
+        render.view.bindings.dispatch_key(
+            focused,
+            &KeyboardEvent::new(
+                0,
+                KeyboardEventKind::Pressed,
+                KeyboardKey::ArrowDown,
+                KeyboardModifiers::default(),
+                None,
+            ),
+            &snapshot.interaction_tree,
+        );
+
+        assert!(offset.get() > 0.0);
+    }
+
+    #[test]
+    fn scroll_box_thumb_drag_updates_offset_signal() {
+        let offset_slot = Rc::new(RefCell::new(None::<Signal<f32>>));
+        let render = render_with_context(Rc::new(RenderState::default()), {
+            let offset_slot = Rc::clone(&offset_slot);
+            move || {
+                let offset = use_signal(|| 0.0_f32);
+                *offset_slot.borrow_mut() = Some(offset.clone());
+                scroll_box()
+                    .height(120.0)
+                    .offset_y(offset)
+                    .children(text().children(
+                        "line 01\nline 02\nline 03\nline 04\nline 05\nline 06\nline 07\nline 08\nline 09\nline 10\nline 11\nline 12",
+                    ))
+            }
+        });
+        let offset = offset_slot
+            .borrow()
+            .clone()
+            .expect("scroll signal should have been captured");
+        let scrollbox_id = render
+            .view
+            .element
+            .id
+            .expect("scroll box should receive an element id");
+        let snapshot =
+            ruin_ui::layout_snapshot(1, UiSize::new(260.0, 160.0), render.view.element());
+        let metrics = snapshot
+            .interaction_tree
+            .scroll_metrics_for_element(scrollbox_id)
+            .expect("scroll metrics should exist for the scroll box");
+        let thumb = metrics
+            .scrollbar_thumb
+            .expect("overflowing scroll box should expose a scrollbar thumb");
+        let thumb_center = Point::new(
+            thumb.origin.x + (thumb.size.width * 0.5),
+            thumb.origin.y + (thumb.size.height * 0.5),
+        );
+        let hovered_targets = snapshot.interaction_tree.hit_path(thumb_center);
+        let handler = scroll_handler_for_event(
+            &render.view.bindings.on_scroll,
+            Some(scrollbox_id),
+            &hovered_targets,
+        )
+        .expect("scroll box should resolve its scroll handler")
+        .clone();
+
+        handler(
+            &RoutedPointerEvent {
+                kind: RoutedPointerEventKind::Down {
+                    button: PointerButton::Primary,
+                },
+                target: hovered_targets
+                    .last()
+                    .cloned()
+                    .expect("thumb center should hit the scroll box"),
+                pointer_id: 1,
+                position: thumb_center,
+            },
+            &snapshot.interaction_tree,
+        );
+        handler(
+            &RoutedPointerEvent {
+                kind: RoutedPointerEventKind::Move,
+                target: hovered_targets
+                    .last()
+                    .cloned()
+                    .expect("thumb center should hit the scroll box"),
+                pointer_id: 1,
+                position: Point::new(thumb_center.x, thumb_center.y + 24.0),
+            },
+            &snapshot.interaction_tree,
+        );
+
+        assert!(offset.get() > 0.0);
+    }
+
+    #[test]
+    fn live_input_path_scrolls_a_scroll_box_rendered_inside_a_branch() {
+        let offset_slot = Rc::new(RefCell::new(None::<Signal<f32>>));
+        let render = render_with_context(Rc::new(RenderState::default()), {
+            let offset_slot = Rc::clone(&offset_slot);
+            move || match true {
+                true => {
+                    let offset = use_signal(|| 0.0_f32);
+                    *offset_slot.borrow_mut() = Some(offset.clone());
+                    column()
+                        .background(surfaces::raised())
+                        .gap(10.0)
+                        .children((
+                            text().children(("bytes = ", 4096)),
+                            scroll_box()
+                                .height(420.0)
+                                .offset_y(offset.clone())
+                                .padding(12.0)
+                                .background(surfaces::canvas())
+                                .border_radius(10.0)
+                                .border((2.0, colors::muted()))
+                                .children(
+                                    text()
+                                        .color(colors::muted())
+                                        .font_family(TextFontFamily::Monospace)
+                                        .children(
+                                            "line 01\nline 02\nline 03\nline 04\nline 05\nline 06\nline 07\nline 08\nline 09\nline 10\nline 11\nline 12\nline 13\nline 14\nline 15\nline 16\nline 17\nline 18\nline 19\nline 20\nline 21\nline 22\nline 23\nline 24",
+                                        ),
+                                ),
+                        ))
+                }
+                false => View::from_element(Element::column()),
+            }
+        });
+        let offset = offset_slot
+            .borrow()
+            .clone()
+            .expect("scroll signal should have been captured");
+        let snapshot =
+            ruin_ui::layout_snapshot(1, UiSize::new(1080.0, 760.0), render.view.element());
+        let interaction_tree = RefCell::new(Some(snapshot.interaction_tree.clone()));
+        let bindings = RefCell::new(render.view.bindings.clone());
+        let mut pointer_router = PointerRouter::new();
+        let mut input_state = InputState::new();
+        let window = UiRuntime::headless()
+            .create_window(WindowSpec::new("scrollbox-test"))
+            .expect("headless window should be created");
+
+        MountedApp::<View>::handle_pointer_event(
+            &window,
+            &interaction_tree,
+            &bindings,
+            &mut pointer_router,
+            &mut input_state,
+            PointerEvent::new(
+                1,
+                Point::new(24.0, 64.0),
+                PointerEventKind::Down {
+                    button: PointerButton::Primary,
+                },
+            ),
+        )
+        .expect("pointer down should succeed");
+        MountedApp::<View>::handle_keyboard_event(
+            &interaction_tree,
+            &bindings,
+            &RefCell::new(Vec::new()),
+            &input_state,
+            KeyboardEvent::new(
+                0,
+                KeyboardEventKind::Pressed,
+                KeyboardKey::ArrowDown,
+                KeyboardModifiers::default(),
+                None,
+            ),
+        )
+        .expect("keyboard event should succeed");
+
+        assert!(offset.get() > 0.0);
+    }
+
+    #[test]
+    fn scroll_box_stays_interactive_when_it_appears_on_a_later_render() {
+        let state = Rc::new(RenderState::default());
+        let ready_slot = Rc::new(RefCell::new(None::<Signal<bool>>));
+        let offset_slot = Rc::new(RefCell::new(None::<Signal<f32>>));
+
+        let render_once =
+            |state: Rc<RenderState>,
+             ready_slot: Rc<RefCell<Option<Signal<bool>>>>,
+             offset_slot: Rc<RefCell<Option<Signal<f32>>>>| {
+                render_with_context(state, move || {
+                    let ready = use_signal(|| false);
+                    let offset = use_signal(|| 0.0_f32);
+                    *ready_slot.borrow_mut() = Some(ready.clone());
+                    *offset_slot.borrow_mut() = Some(offset.clone());
+
+                    if ready.get() {
+                        column()
+                        .background(surfaces::raised())
+                        .gap(10.0)
+                        .children((
+                            text().children(("bytes = ", 4096)),
+                            scroll_box()
+                                .height(420.0)
+                                .offset_y(offset.clone())
+                                .padding(12.0)
+                                .background(surfaces::canvas())
+                                .border_radius(10.0)
+                                .border((2.0, colors::muted()))
+                                .children(
+                                    text()
+                                        .color(colors::muted())
+                                        .font_family(TextFontFamily::Monospace)
+                                        .children(
+                                            "line 01\nline 02\nline 03\nline 04\nline 05\nline 06\nline 07\nline 08\nline 09\nline 10\nline 11\nline 12\nline 13\nline 14\nline 15\nline 16\nline 17\nline 18\nline 19\nline 20\nline 21\nline 22\nline 23\nline 24",
+                                        ),
+                                ),
+                        ))
+                    } else {
+                        text().children("Loading file contents...")
+                    }
+                })
+            };
+
+        let _initial = render_once(
+            Rc::clone(&state),
+            Rc::clone(&ready_slot),
+            Rc::clone(&offset_slot),
+        );
+        let ready = ready_slot
+            .borrow()
+            .clone()
+            .expect("ready signal should have been captured");
+        let offset = offset_slot
+            .borrow()
+            .clone()
+            .expect("offset signal should have been captured");
+        let _ = ready.set(true);
+
+        let render = render_once(state, ready_slot, Rc::clone(&offset_slot));
+        let snapshot =
+            ruin_ui::layout_snapshot(1, UiSize::new(1080.0, 760.0), render.view.element());
+        let interaction_tree = RefCell::new(Some(snapshot.interaction_tree.clone()));
+        let bindings = RefCell::new(render.view.bindings.clone());
+        let mut pointer_router = PointerRouter::new();
+        let mut input_state = InputState::new();
+        let window = UiRuntime::headless()
+            .create_window(WindowSpec::new("scrollbox-transition-test"))
+            .expect("headless window should be created");
+
+        MountedApp::<View>::handle_pointer_event(
+            &window,
+            &interaction_tree,
+            &bindings,
+            &mut pointer_router,
+            &mut input_state,
+            PointerEvent::new(
+                1,
+                Point::new(24.0, 64.0),
+                PointerEventKind::Down {
+                    button: PointerButton::Primary,
+                },
+            ),
+        )
+        .expect("pointer down should succeed after branch switch");
+        MountedApp::<View>::handle_keyboard_event(
+            &interaction_tree,
+            &bindings,
+            &RefCell::new(Vec::new()),
+            &input_state,
+            KeyboardEvent::new(
+                0,
+                KeyboardEventKind::Pressed,
+                KeyboardKey::ArrowDown,
+                KeyboardModifiers::default(),
+                None,
+            ),
+        )
+        .expect("keyboard event should succeed after branch switch");
+
+        assert!(offset.get() > 0.0);
+    }
+
+    #[test]
+    fn live_input_path_scrolls_with_real_cargo_lock_contents() {
+        let offset_slot = Rc::new(RefCell::new(None::<Signal<f32>>));
+        let render = render_with_context(Rc::new(RenderState::default()), {
+            let offset_slot = Rc::clone(&offset_slot);
+            move || {
+                let offset = use_signal(|| 0.0_f32);
+                *offset_slot.borrow_mut() = Some(offset.clone());
+                column().background(surfaces::raised()).gap(10.0).children((
+                    text().children(("bytes = ", include_str!("../../../Cargo.lock").len())),
+                    scroll_box()
+                        .height(420.0)
+                        .offset_y(offset.clone())
+                        .padding(12.0)
+                        .background(surfaces::canvas())
+                        .border_radius(10.0)
+                        .border((2.0, colors::muted()))
+                        .children(
+                            text()
+                                .color(colors::muted())
+                                .font_family(TextFontFamily::Monospace)
+                                .children(include_str!("../../../Cargo.lock")),
+                        ),
+                ))
+            }
+        });
+        let offset = offset_slot
+            .borrow()
+            .clone()
+            .expect("scroll signal should have been captured");
+        let snapshot =
+            ruin_ui::layout_snapshot(1, UiSize::new(1080.0, 760.0), render.view.element());
+        let interaction_tree = RefCell::new(Some(snapshot.interaction_tree.clone()));
+        let bindings = RefCell::new(render.view.bindings.clone());
+        let mut pointer_router = PointerRouter::new();
+        let mut input_state = InputState::new();
+        let window = UiRuntime::headless()
+            .create_window(WindowSpec::new("scrollbox-cargo-lock-test"))
+            .expect("headless window should be created");
+
+        MountedApp::<View>::handle_pointer_event(
+            &window,
+            &interaction_tree,
+            &bindings,
+            &mut pointer_router,
+            &mut input_state,
+            PointerEvent::new(
+                1,
+                Point::new(24.0, 64.0),
+                PointerEventKind::Scroll {
+                    delta: Point::new(0.0, 48.0),
+                },
+            ),
+        )
+        .expect("wheel event should succeed");
+
+        assert!(offset.get() > 0.0);
+    }
+
+    #[test]
+    fn rerendered_scroll_box_element_carries_the_updated_offset() {
+        let state = Rc::new(RenderState::default());
+        let offset_slot = Rc::new(RefCell::new(None::<Signal<f32>>));
+        let render_once =
+            |state: Rc<RenderState>, offset_slot: Rc<RefCell<Option<Signal<f32>>>>| {
+                render_with_context(state, move || {
+                    let offset = use_signal(|| 0.0_f32);
+                    *offset_slot.borrow_mut() = Some(offset.clone());
+                    scroll_box()
+                        .height(120.0)
+                        .offset_y(offset.clone())
+                        .children(
+                            text().children("line 01\nline 02\nline 03\nline 04\nline 05\nline 06"),
+                        )
+                })
+            };
+
+        let _initial = render_once(Rc::clone(&state), Rc::clone(&offset_slot));
+        let offset = offset_slot
+            .borrow()
+            .clone()
+            .expect("offset signal should have been captured");
+        let _ = offset.set(96.0);
+        let render = render_once(state, offset_slot);
+        let debug = format!("{:?}", render.view.element());
+
+        assert!(debug.contains("offset_y: 96.0"), "{debug}");
+    }
 }

lib/ui/src/layout.rs

@@ -313,9 +313,11 @@ fn layout_element(
                 perf_stats,
             );
             let provisional_content_height = content_size.height.max(viewport_rect.size.height);
+            let requested_offset_y = scroll_box.offset_y.max(0.0);
             let provisional_max_offset_y =
                 (provisional_content_height - viewport_rect.size.height).max(0.0);
-            let mut offset_y = scroll_box.offset_y.max(0.0).min(provisional_max_offset_y);
+            let mut offset_y = requested_offset_y.min(provisional_max_offset_y);
+            let child_scene_start = scene.items.len();
 
             if viewport_rect.size.width > 0.0 && viewport_rect.size.height > 0.0 {
                 scene.push_clip(viewport_rect, 0.0);
@@ -345,7 +347,31 @@ fn layout_element(
                 .max(actual_content_height.ceil())
                 .max(viewport_rect.size.height);
             let max_offset_y = (content_height - viewport_rect.size.height).max(0.0);
-            offset_y = offset_y.min(max_offset_y);
+            let corrected_offset_y = requested_offset_y.min(max_offset_y);
+            if (corrected_offset_y - offset_y).abs() > f32::EPSILON
+                && viewport_rect.size.width > 0.0
+                && viewport_rect.size.height > 0.0
+            {
+                offset_y = corrected_offset_y;
+                scene.items.truncate(child_scene_start);
+                scene.push_clip(viewport_rect, 0.0);
+                interaction.children = layout_container_children(
+                    element,
+                    Rect::new(
+                        viewport_rect.origin.x,
+                        viewport_rect.origin.y - offset_y,
+                        viewport_rect.size.width,
+                        content_height,
+                    ),
+                    &interaction.path,
+                    scene,
+                    text_system,
+                    perf_stats,
+                );
+                scene.pop_clip();
+            } else {
+                offset_y = corrected_offset_y;
+            }
             interaction.scroll_metrics = Some(ScrollMetrics {
                 viewport_rect,
                 content_height,
@@ -1701,6 +1727,41 @@ mod tests {
         assert!(text_bottom >= viewport_bottom - 1.0);
     }
 
+    #[test]
+    fn scroll_box_preserves_requested_offset_for_direct_text_children() {
+        let scrollbox_id = ElementId::new(23);
+        let root = Element::column().child(
+            Element::scroll_box(240.0)
+                .id(scrollbox_id)
+                .width(320.0)
+                .height(120.0)
+                .padding(Edges::all(8.0))
+                .child(Element::text(
+                    "line 01\nline 02\nline 03\nline 04\nline 05\nline 06\nline 07\nline 08\nline 09\nline 10\nline 11\nline 12\nline 13\nline 14\nline 15\nline 16\nline 17\nline 18\nline 19\nline 20\nline 21\nline 22\nline 23\nline 24\nline 25\nline 26",
+                    TextStyle::new(16.0, Color::rgb(0xFF, 0xFF, 0xFF)).with_line_height(20.0),
+                )),
+        );
+
+        let snapshot = layout_snapshot(1, UiSize::new(360.0, 220.0), &root);
+        let scroll_metrics = snapshot
+            .interaction_tree
+            .scroll_metrics_for_element(scrollbox_id)
+            .expect("scroll box should expose scroll metrics");
+        let visible_text = snapshot
+            .scene
+            .items
+            .iter()
+            .find_map(|item| match item {
+                DisplayItem::Text(text) => Some(text),
+                _ => None,
+            })
+            .expect("scroll box should emit direct text children");
+
+        assert!(scroll_metrics.max_offset_y > 240.0);
+        assert_eq!(scroll_metrics.offset_y, 240.0);
+        assert!(visible_text.origin.y < scroll_metrics.viewport_rect.origin.y);
+    }
+
     #[test]
     fn interaction_tree_hit_test_returns_deepest_pointer_target() {
         let root = Element::column()