masonry: fix updating Portal on scrollbar drag

[tomcur]

Fixes scrolling Portal on scrollbar drag by recomposing instead of relayouting.

Regression probably caused in 59ee615 (#488) or ff7635e (#522).

[jaredoconnell]

This seems right, but I can't test it due to a bug that appears to be present in both main and this PR:

Node #166 of TreeUpdate includes duplicate child #165;

[PoignardAzur]

Huh. That's odd, I thought layout automatically triggered a compose. Maybe it hasn't merged yet.

[PoignardAzur]

I tested it (with the panic fix). Dragging the scrollbar still doesn't seem to work.

[tomcur]

I tested it (with the panic fix). Dragging the scrollbar still doesn't seem to work.

The scrollbars currently do not receive the pointer events due to a separate bug; the events are instead passed to the inner widget of the Portal, perhaps I should've provided that context in this PR. This patch by @DJMcNab fixes that separate issue, PR #565 also fixes it:

diff --git a/masonry/src/widget/widget_ref.rs b/masonry/src/widget/widget_ref.rs
index 2f85ea5ba..6bd00a32c 100644
--- a/masonry/src/widget/widget_ref.rs
+++ b/masonry/src/widget/widget_ref.rs
@@ -190,7 +190,7 @@ impl<'w> WidgetRef<'w, dyn Widget> {
                 }
             }
             // TODO - Use Widget::get_child_at_pos method
-            if let Some(child) = innermost_widget.children().into_iter().find(|child| {
+            if let Some(child) = innermost_widget.children().into_iter().rev().find(|child| {
                 !child.widget.skip_pointer() && child.state().window_layout_rect().contains(pos)
             }) {
                 innermost_widget = child;

(Zulip thread here: https://xi.zulipchat.com/#narrow/stream/317477-masonry/topic/Portal.20Scrollbars.20click-and-drag)

[PoignardAzur]

PR #565 is unlikely to be merged today.

If you could include that change in the current PR, we could test it and merge it now.

[tomcur]

If you could include that change in the current PR, we could test it and merge it now.

Done. The scroll bar should be responsive now, but note there is still an issue related to the drag anchor, so the scroll bar jumps around a bit. My patch on Zulip fixes that (https://xi.zulipchat.com/#narrow/stream/317477-masonry/topic/Portal.20Scrollbars.20click-and-drag), but that patch should only be used if PointerState::position (LogicalPosition) is intended to not be relative to the widget origin.

diff --git a/masonry/src/widget/scroll_bar.rs b/masonry/src/widget/scroll_bar.rs
index 3434a01..5a4e3fe 100644
--- a/masonry/src/widget/scroll_bar.rs
+++ b/masonry/src/widget/scroll_bar.rs
@@ -133,7 +133,8 @@ impl Widget for ScrollBar {
                 let cursor_min_length = theme::SCROLLBAR_MIN_SIZE;
                 let cursor_rect = self.get_cursor_rect(ctx.size(), cursor_min_length);

-                let mouse_pos = Point::new(state.position.x, state.position.y);
+                let mouse_pos =
+                    Point::new(state.position.x, state.position.y) - ctx.window_origin().to_vec2();
                 if cursor_rect.contains(mouse_pos) {
                     let (z0, z1) = self.axis.major_span(cursor_rect);
                     let mouse_major = self.axis.major_pos(mouse_pos);
@@ -147,7 +148,8 @@ impl Widget for ScrollBar {
                 ctx.request_paint();
             }
             PointerEvent::PointerMove(state) => {
-                let mouse_pos = Point::new(state.position.x, state.position.y);
+                let mouse_pos =
+                    Point::new(state.position.x, state.position.y) - ctx.window_origin().to_vec2();
                 if let Some(grab_anchor) = self.grab_anchor {
                     let cursor_min_length = theme::SCROLLBAR_MIN_SIZE;
                     self.cursor_progress = self.progress_from_mouse_pos(

[PoignardAzur]

but that patch should only be used if PointerState::position (LogicalPosition) is intended to not be relative to the widget origin.

It's not, it's supposed to be global. (Events are passed unmodified to all widgets right now.)

[tomcur]

It's not, it's supposed to be global. (Events are passed unmodified to all widgets right now.)

Shall I add the anchor fix to this PR as well?

[PoignardAzur]

Sure.

[tomcur]

@PoignardAzur This is ready for testing.

[PoignardAzur]

Looks good to me.

To-do list example now works on my machine.

[DJMcNab]

@tomcur if you're happy for this to land, I'll merge.

For context, we normally use an author-merges policy, but you aren't an org member yet (I expect you to be added in the 2024-09-05 office hours). See https://xi.zulipchat.com/#narrow/stream/419691-linebender/topic/Organisation.20members

[DJMcNab]

I can confirm this now works as expected on Android as well. Thank you!

[tomcur]

I'm happy for this to be merged, thank you. Nice to hear you're proposing to add me as org member!