Implement scroll_view widget
#146
Conversation
nicoburns
force-pushed
the
scrollview
branch
6 times, most recently
from
ac3d105 to
97bf9b1
Compare
nicoburns
changed the title
scroll_view widgetscroll_view widget
nicoburns
force-pushed
the
scrollview
branch
12 times, most recently
from
a82af1e to
1627f3b
Compare
There was a problem hiding this comment.
Thanks :)
I think it generally makes sense to add small examples showing the widgets in action (I personally like self-contained examples showing each widgets capabilities on its own, for this maybe a lorem ipsum text in a scroll view), but not a requirement.
I'm not (yet) deep in layouting and vello, so I may be wrong here: Wouldn't it make sense, instead of setting the child origin, to use the vello layer with a transform that has the offset instead?
| @@ -94,7 +95,7 @@ pub struct Pod { | |||
| } | |||
|
|
|||
| #[derive(Debug)] | |||
| pub(crate) struct WidgetState { | |||
| pub struct WidgetState { | |||
There was a problem hiding this comment.
Out of curiosity, why this change?
There was a problem hiding this comment.
This is to allow set_origin to be called with multiple different context types. As set_origin is public, and now takes WidgetState as a parameter, WidgetState also needs to be public.
|
|
||
| self.state.flags.insert(PodFlags::VIEW_CONTEXT_CHANGED); | ||
| } | ||
| } | ||
|
|
||
| /// Get the widgets size (as returned by the layout method) | ||
| pub fn get_size(&mut self) -> Size { |
There was a problem hiding this comment.
I think we should omit get_ prefixes to stay consistent
| pub fn get_size(&mut self) -> Size { | |
| pub fn size(&mut self) -> Size { |
| if !cx.is_handled() { | ||
| if let RawEvent::MouseWheel(mouse) = event { | ||
| let new_offset = (self.offset + mouse.wheel_delta.y).max(0.0); | ||
| if let Event::MouseWheel(mouse) = event { |
There was a problem hiding this comment.
Out of a UX perspective it would be interesting (not entirely sure how this is done in web, but I think it's similar as here?), how to handle nested scroll-views.
When I'm getting this right, the nested scroll view handles the scroll event, unless it doesn't scroll (e.g. is at the end).
Just thinking out loud, not suggesting anything.
There was a problem hiding this comment.
I believe that ideally it would be a little more sophisticated than that, and that the inner view could handle part of a scroll event (some of the pixel delta) and leave the rest for an outer scroll view (perhaps by making the event mutable?). But you suggestion would probably cover the basics.
I believe that would draw the widget in the correct place, but would not allow input to work correctly. Whereas set_origin applies to both. |
That's a good point, might be interesting to research something like virtualized pointer events or similar that can be modified while traversing down the tree (in |
nicoburns
force-pushed
the
scrollview
branch
from
1627f3b to
7a39d6d
Compare
This allows it to be called from the event method
nicoburns
force-pushed
the
scrollview
branch
from
7a39d6d to
53a3a33
Compare
|
@nicoburns are you expecting to revive this branch? It seems to be on the Xilem repo and not deleted. |
|
No. #155 was merged instead |
|
I'll delete the branch then |
Builds on top of #140 which should be merged first.I've now made this PR independent of #140Notes
set_originmethod take&mut WidgetStaterather than&mut LayoutCx. Because otherwise I couldn't call it fromeventand had to request layout every time the scroll position changed.Video
Screen.Recording.2023-11-17.at.00.13.47.mov