CoreScrollbar widget. #19803
CoreScrollbar widget. #19803
Conversation
|
The generated |
alice-i-cecile
added
C-Feature
| pub min_thumb_size: f32, | ||
| } | ||
|
|
||
| /// Marker component to indicate that the entity is a scrollbar thumb. This should be a child |
There was a problem hiding this comment.
These docs need to explain what a "thumb" is, for those who haven't heard the term.
| /// * `target` - The scrollable entity that this scrollbar will control. | ||
| /// * `orientation` - The orientation of the scrollbar (horizontal or vertical). | ||
| /// * `min_thumb_size` - The minimum size of the scrollbar's thumb, in pixels. | ||
| pub fn new(target: Entity, orientation: Orientation, min_thumb_size: f32) -> Self { |
There was a problem hiding this comment.
min_thumb_size feels like a weird thing to put on the CoreScrollbar component. How painful would it be to move that to CoreScrollbarThumb instead?
There was a problem hiding this comment.
Hmmm, it's used in a lot of calculations. Let me think about it.
There was a problem hiding this comment.
I'd like to punt on this for now.
| /// Minimum size of the scrollbar thumb, in pixel units. | ||
| pub min_thumb_size: f32, |
There was a problem hiding this comment.
The name and comment needs to be more explicit I think, it's not going to be obvious to users whether the min_thumb_size constraint is applied to the length of the main or cross axis of the thumb node (or both).
There was a problem hiding this comment.
I expanded the comment in a way which hopefully clears this up. While I didn't specifically mention main/cross axis (not sure I want to introduce those terms), I think people can understand that the thumb only grows and shrinks along the main axis.
There was a problem hiding this comment.
The problem I have is with the word "size", I think something like min_thumb_length is clearer because it's unambiguously 1-dimensional.
There was a problem hiding this comment.
Done. Also revised the comment some more.
There was a problem hiding this comment.
Because it's common these days for scrollbar thumbs to be rounded, I considered adding a discussion about what happens when the thumb size is smaller than the border radius, but honestly they can figure that out for themselves.
| // Convert the click coordinates into a scroll position. If it's greater than the | ||
| // current scroll position, scroll forward by one step (visible size) otherwise scroll | ||
| // back. | ||
| let visible_size = scroll_content.size() * scroll_content.inverse_scale_factor; | ||
| let content_size = scroll_content.content_size() * scroll_content.inverse_scale_factor; | ||
| let max_range = (content_size - visible_size).max(Vec2::ZERO); | ||
| match scrollbar.orientation { | ||
| ControlOrientation::Horizontal => { | ||
| if node.size().x > 0. { | ||
| let click_pos = local_pos.x * content_size.x / node.size().x; | ||
| scroll_pos.offset_x = (scroll_pos.offset_x | ||
| + if click_pos > scroll_pos.offset_x { | ||
| visible_size.x | ||
| } else { | ||
| -visible_size.x | ||
| }) | ||
| .clamp(0., max_range.x); | ||
| } | ||
| } | ||
| ControlOrientation::Vertical => { | ||
| if node.size().y > 0. { | ||
| let click_pos = local_pos.y * content_size.y / node.size().y; | ||
| scroll_pos.offset_y = (scroll_pos.offset_y | ||
| + if click_pos > scroll_pos.offset_y { | ||
| visible_size.y | ||
| } else { | ||
| -visible_size.y | ||
| }) | ||
| .clamp(0., max_range.y); | ||
| } | ||
| } |
There was a problem hiding this comment.
The scroll offset calculation is 1-dimensional, it doesn't need to be repeated for both orientations. It would be better to get the size and position values along the main axis first, then perform the calculations using those values axis-agnostic, and then set the scroll offset for the main axis.
There was a problem hiding this comment.
The concern I have is this: while the two forks have the same structure and formula, they access different fields (x vs. y, width vs. height, top vs. left) in lots of places. Trying to abstract out those differences means adding an additional layer of abstraction and indirection. I worry that this will make the code harder to read.
There was a problem hiding this comment.
I took a shot at factoring this and I didn't like the result. I did do the one later down.
The problem is that I'd have to introduce a function with six positional parameters to factor this formula.
There was a problem hiding this comment.
It's not so much about readability but maintenance. It's very easy for branching code like this to accumulate bugs that go unnoticed, I think I must have fixed dozens of them in Bevy by now.
With Vec2s you can use indices instead of .x and .y to refer to the components, so another option is something like:
let click_pos = local_pos[axis] * content_size[axis] / node.size()[axis];
There was a problem hiding this comment.
It's also possible to make a symmetrical slider construction where you don't need to use any axis-specific constraints like left or width. On the gutter node you set the orientation using FlexDirection and then have an intermediate node parenting the thumb node you use to position the thumb by setting its flex_basis. It could be refactored so that there is no need to know the node size even and everything uses percentage values.
Adding an extra node is a pain without reactivity though, so it's not perfect either.
| let distance = ev.event().distance; | ||
| let visible_size = scroll_content.size() * scroll_content.inverse_scale_factor; | ||
| let content_size = scroll_content.content_size() * scroll_content.inverse_scale_factor; | ||
| match scrollbar.orientation { | ||
| ControlOrientation::Horizontal => { | ||
| let range = (content_size.x - visible_size.x).max(0.); | ||
| let scrollbar_width = (node.size().x * node.inverse_scale_factor | ||
| - scrollbar.min_thumb_size) | ||
| .max(1.0); | ||
| scroll_pos.offset_x = if range > 0. { | ||
| (drag.drag_origin + (distance.x * content_size.x) / scrollbar_width) | ||
| .clamp(0., range) | ||
| } else { | ||
| 0. | ||
| } | ||
| } | ||
| ControlOrientation::Vertical => { | ||
| let range = (content_size.y - visible_size.y).max(0.); | ||
| let scrollbar_height = (node.size().y * node.inverse_scale_factor | ||
| - scrollbar.min_thumb_size) | ||
| .max(1.0); | ||
| scroll_pos.offset_y = if range > 0. { | ||
| (drag.drag_origin + (distance.y * content_size.y) / scrollbar_height) | ||
| .clamp(0., range) | ||
| } else { | ||
| 0. | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
Same with these calculations, it will be much clearer if there's just one set of calculations performed in 1-dimensional space.
| match scrollbar.orientation { | ||
| ControlOrientation::Horizontal => { | ||
| let thumb_size = if content_size.x > visible_size.x { | ||
| (track_length.x * visible_size.x / content_size.x) | ||
| .max(scrollbar.min_thumb_size) | ||
| .min(track_length.x) | ||
| } else { | ||
| track_length.x | ||
| }; | ||
|
|
||
| let thumb_pos = if content_size.x > visible_size.x { | ||
| scroll_area.0.offset_x * (track_length.x - thumb_size) | ||
| / (content_size.x - visible_size.x) | ||
| } else { | ||
| 0. | ||
| }; | ||
|
|
||
| thumb.top = Val::Px(0.); | ||
| thumb.bottom = Val::Px(0.); | ||
| thumb.left = Val::Px(thumb_pos); | ||
| thumb.width = Val::Px(thumb_size); | ||
| } | ||
| ControlOrientation::Vertical => { | ||
| let thumb_size = if content_size.y > visible_size.y { | ||
| (track_length.y * visible_size.y / content_size.y) | ||
| .max(scrollbar.min_thumb_size) | ||
| .min(track_length.y) | ||
| } else { | ||
| track_length.y | ||
| }; | ||
|
|
||
| let thumb_pos = if content_size.y > visible_size.y { | ||
| scroll_area.0.offset_y * (track_length.y - thumb_size) | ||
| / (content_size.y - visible_size.y) | ||
| } else { | ||
| 0. | ||
| }; | ||
|
|
||
| thumb.left = Val::Px(0.); | ||
| thumb.right = Val::Px(0.); | ||
| thumb.top = Val::Px(thumb_pos); | ||
| thumb.height = Val::Px(thumb_size); | ||
| } | ||
| }; |
There was a problem hiding this comment.
This one I was able to factor out. That being said, it's more code than before, because of the overhead of declaring the parameters that need to be passed in.
There was a problem hiding this comment.
I factored out a bit more, although the result is a bit more abstract than I like.
|
|
||
| /// Component used to manage the state of a scrollbar during dragging. | ||
| #[derive(Component, Default)] | ||
| pub struct ScrollbarDragState { |
There was a problem hiding this comment.
This could be unified with CoreSliderDragState into a LinearDragState component or something.
There was a problem hiding this comment.
I'd prefer not to do this: the two structs look the same now, but I'm not sure they will be the same forever.
See also my essay on refactoring duplicate code: https://medium.com/machine-words/refactoring-duplicate-code-5e17f20dcb51
There was a problem hiding this comment.
I mostly agree about code duplication, but my concern about this is that if we have a lot of similar components it could easily result in confusing errors where a SomeSliderProperty query is used when it should be a SomeScrollbarProperty etc. There's a balance I guess and it doesn't really matter too much at this stage. Another option when the requirements diverge is to split up the component.
| // Convert the click coordinates into a scroll position. If it's greater than the | ||
| // current scroll position, scroll forward by one step (visible size) otherwise scroll | ||
| // back. | ||
| let visible_size = scroll_content.size() * scroll_content.inverse_scale_factor; | ||
| let content_size = scroll_content.content_size() * scroll_content.inverse_scale_factor; | ||
| let max_range = (content_size - visible_size).max(Vec2::ZERO); | ||
| match scrollbar.orientation { | ||
| ControlOrientation::Horizontal => { | ||
| if node.size().x > 0. { | ||
| let click_pos = local_pos.x * content_size.x / node.size().x; | ||
| scroll_pos.offset_x = (scroll_pos.offset_x | ||
| + if click_pos > scroll_pos.offset_x { | ||
| visible_size.x | ||
| } else { | ||
| -visible_size.x | ||
| }) | ||
| .clamp(0., max_range.x); | ||
| } | ||
| } | ||
| ControlOrientation::Vertical => { | ||
| if node.size().y > 0. { | ||
| let click_pos = local_pos.y * content_size.y / node.size().y; | ||
| scroll_pos.offset_y = (scroll_pos.offset_y | ||
| + if click_pos > scroll_pos.offset_y { | ||
| visible_size.y | ||
| } else { | ||
| -visible_size.y | ||
| }) | ||
| .clamp(0., max_range.y); | ||
| } | ||
| } |
There was a problem hiding this comment.
It's not so much about readability but maintenance. It's very easy for branching code like this to accumulate bugs that go unnoticed, I think I must have fixed dozens of them in Bevy by now.
With Vec2s you can use indices instead of .x and .y to refer to the components, so another option is something like:
let click_pos = local_pos[axis] * content_size[axis] / node.size()[axis];
| // Convert the click coordinates into a scroll position. If it's greater than the | ||
| // current scroll position, scroll forward by one step (visible size) otherwise scroll | ||
| // back. | ||
| let visible_size = scroll_content.size() * scroll_content.inverse_scale_factor; | ||
| let content_size = scroll_content.content_size() * scroll_content.inverse_scale_factor; | ||
| let max_range = (content_size - visible_size).max(Vec2::ZERO); | ||
| match scrollbar.orientation { | ||
| ControlOrientation::Horizontal => { | ||
| if node.size().x > 0. { | ||
| let click_pos = local_pos.x * content_size.x / node.size().x; | ||
| scroll_pos.offset_x = (scroll_pos.offset_x | ||
| + if click_pos > scroll_pos.offset_x { | ||
| visible_size.x | ||
| } else { | ||
| -visible_size.x | ||
| }) | ||
| .clamp(0., max_range.x); | ||
| } | ||
| } | ||
| ControlOrientation::Vertical => { | ||
| if node.size().y > 0. { | ||
| let click_pos = local_pos.y * content_size.y / node.size().y; | ||
| scroll_pos.offset_y = (scroll_pos.offset_y | ||
| + if click_pos > scroll_pos.offset_y { | ||
| visible_size.y | ||
| } else { | ||
| -visible_size.y | ||
| }) | ||
| .clamp(0., max_range.y); | ||
| } | ||
| } |
There was a problem hiding this comment.
It's also possible to make a symmetrical slider construction where you don't need to use any axis-specific constraints like left or width. On the gutter node you set the orientation using FlexDirection and then have an intermediate node parenting the thumb node you use to position the thumb by setting its flex_basis. It could be refactored so that there is no need to know the node size even and everything uses percentage values.
Adding an extra node is a pain without reactivity though, so it's not perfect either.
examples/ui/scrollbars.rs
Outdated
| let color: Color = if *is_hovering { | ||
| // If hovering, use a lighter color | ||
| colors::GRAY3 |
There was a problem hiding this comment.
It should also use the lighter colour if the pointer unhovers while dragging.
There was a problem hiding this comment.
I agree; I just need to figure out how to do it. Unlike the slider where the Hovered component is on the entire slider, in this case the Hovered component is on the thumb. This means that the two input signals are coming from two different entities, which means that simple change detection query filters won't work.
There was a problem hiding this comment.
I suppose I could put a Pressed marker on the thumb while a drag is active. But then I have to deal with the pain of RemovedComponents, yuck.
There was a problem hiding this comment.
One solution would be to add a Dragging(bool) component on the thumb. It will be a required component, and not a marker, so a regular Changed query filter would work.
The other approach is to just wait for real reactivity and solve it then.
There was a problem hiding this comment.
OK I believe I have solved the problem: I moved the drag state to the thumb entity, allowing a single query filter for both hover and drag. (Yet another way in which scrollbars diverge from sliders).
|
This will be immediately useful to our entity inspector, as we don’t have a scrolling UI widget! I’m not too versed on the details of this change, but I’m definitely excited to get this in the editor |
There was a problem hiding this comment.
I'm slightly confused as to why we need to specify the ScrollBar component here directly? I would've thought this would get obfuscated from users - something like: if there are more elements larger than the size of the area, the scrollbar gets created automatically - and then any settings to configure scrolling we can specify when creating the initial area?
| grid_column: GridPlacement::start(2), | ||
| ..default() | ||
| }, | ||
| CoreScrollbar { |
There was a problem hiding this comment.
I'm slightly confused as to why we need to specify the ScrollBar component here directly? I would've thought this would get obfuscated from users - something like: if there are more elements larger than the size of the area, the scrollbar gets created automatically - and then any settings to configure scrolling we can specify when creating the initial area?
There was a problem hiding this comment.
The reason is flexibility. Remember, core widgets are intended to be low-level, allowing maximum customizability. They won't necessarily have the all of the conveniences of a high-level widget.
The user might want to style the scrollbars differently than the standard oval shape. They might want to have the scrollbars overlay the content, or push the content aside. For something like a scrolling credits page, they might not want to have scrollbars at all!
For automatic scrollbar creation, I had considered adding a ScrollArea widget to feathers.
This relates to an earlier PR #19344 which also attempted to automatically add scrollbars. That PR allowed very limited scrollbar customization.
There was a problem hiding this comment.
This makes sense! I think I missed the part that this was meant to be more low-level and highly customisable - I'll take a look at your other PR for a more automated approach to scroll bars 😄
There was a problem hiding this comment.
That other PR is not mine, and is somewhat in conflict with this one.
There was a problem hiding this comment.
Oh yes, I just checked it out. I'm purely thinking from an end-user standpoint that I don't think devs are necessarily going to care all that much about customising scrollbars when they have a text area, as an example. I think they most likely just want to create the text box (most likely with some size constraints), and bevy automagically adds the necessary scroll bars to fit. But that also may mean we need the changes in this PR before we can improve the ergonomics for end users!
There was a problem hiding this comment.
+1 for some easy "automatic scrollbar" API, as this seems valuable for users that don't care about customization (at this point, e.g., they're prototyping and want to sketch it and move on (making it prettier late in production / "blockout" their UI), or ever (happy with some default styles)).
Having it via overflow: scroll overflow: auto would make it more familiar to some. (and us more consistent with web styling we partially happened to align with)
We wouldn't have "whatever you spawn is whatever is stored in the World" (~WYSIWYG), as scrollbars are Entities (which's nice btw).
But perhaps it's not as important for these "don't actually care" use-cases. (and we have required components that do similar "behind the scenes" thing already, so I guess it's ok)
There was a problem hiding this comment.
I realize that folks will want an automatic API, but I would rather build that on top of something like this.
It's certainly true that the web supports "implicit" scrollbars, and we do borrow a lot of ideas from the web. However, the web also has CSS, which we don't have (and are not likely to adopt - there have been a number of attempts to add rule-based styling to Bevy, and none have gotten much traction in the Bevy user base). The lack of rule-based styling means that it's difficult for users to control the appearance of scrollbars that are completely automatic.
My approach here is to borrow ideas from earlier UI toolkits: Java Swing, .Net, Qt, Gtk, MSVC and so on. In those frameworks, you don't have the ability to add scrolling capability to any arbitrary UI component, instead you opt-in to scrolling by putting your content inside a ScrollArea widget which automatically provides scrollbars for you.
All that being said, the CoreScrollbar widget is a little bit automatic: you don't have to set up an observer to listen to scroll change events, all you have to do is spawn the damn thing and it takes care of synchronization (in both directions) for you.
There was a problem hiding this comment.
There also might be widget designs that need scrollbars, but which don't use the builtin Overflow and ScrollPosition api. Consider a world view for a 2d game editor where the world is rendered to a texture. The view of the world doesn't overflow, it's just drawn to the size of the node. So interacting with the scrollbars wouldn't displace the UI node displaying the view, instead they'd move the view within the world.
There was a problem hiding this comment.
Oh, you didn't just go there :)
I think it would be hard to build a single widget implementation that supported all these different use cases. I'm wary of excessive generality, as I've known too many architecture astronauts.
There was a problem hiding this comment.
Oh, customizable scrollbars as Entities is a great approach, meant what you said, that, additionally, it seems handy to be able to spin them up automatically on oveflow, but that could be a separate PR, I guess.
|
The time I saw Core Slider I thought "whoa, sweet, and scrollbar could be a slider", we could slam some ScrollbarOf / Scrollbars relationship and some logic to keep scroll positions and thumb size in sync for them. |
There was a problem hiding this comment.
It might be simpler to use grid, on the scrollbar entity set:
// for vertical orientation use rows instead
display: Display::Grid,
grid_template_columns: vec![
GridTrack::px(displacement),
GridTrack::px(thumb_length),
],
and on the thumb:
column: GridPlacement::start(2),
Then there shouldn't be any need to query for the thumb when updating its length and position.
| let visible_size = scroll_content.size() * scroll_content.inverse_scale_factor; | ||
| let content_size = scroll_content.content_size() * scroll_content.inverse_scale_factor; | ||
| let max_range = (content_size - visible_size).max(Vec2::ZERO); | ||
|
|
||
| fn adjust_scroll_pos(scroll_pos: &mut f32, click_pos: f32, step: f32, range: f32) { | ||
| *scroll_pos = | ||
| (*scroll_pos + if click_pos > *scroll_pos { step } else { -step }).clamp(0., range); | ||
| } | ||
|
|
||
| match scrollbar.orientation { | ||
| ControlOrientation::Horizontal => { | ||
| if node.size().x > 0. { | ||
| let click_pos = local_pos.x * content_size.x / node.size().x; | ||
| adjust_scroll_pos( | ||
| &mut scroll_pos.offset_x, | ||
| click_pos, | ||
| visible_size.x, | ||
| max_range.x, | ||
| ); | ||
| } | ||
| } | ||
| ControlOrientation::Vertical => { | ||
| if node.size().y > 0. { | ||
| let click_pos = local_pos.y * content_size.y / node.size().y; | ||
| adjust_scroll_pos( | ||
| &mut scroll_pos.offset_y, | ||
| click_pos, | ||
| visible_size.y, | ||
| max_range.y, | ||
| ); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This looks worse than the original version and I suspect you probably hate it. What I was thinking of was flattening the code paths like this (would be better if ScrollPosition just newtyped a Vec2):
let (axis, scroll_offset) = match scrollbar.orientation {
ControlOrientation::Horizontal => (0, &mut scroll_pos.offset_x),
ControlOrientation::Vertical => (1, &mut scroll_pos.offset_y),
};
if node.size()[axis] > 0. {
let visible_length = scroll_content.size()[axis] * scroll_content.inverse_scale_factor;
let content_length =
scroll_content.content_size()[axis] * scroll_content.inverse_scale_factor;
let max_range = (content_length - visible_length).max(0.);
let click_pos = local_pos[axis] * content_length / node.size()[axis];
let dir = if click_pos > *scroll_offset { 1. } else { -1. };
*scroll_offset = (*scroll_offset + dir * visible_length).clamp(0., max_range);
}
| // Size of the visible scrolling area. | ||
| let visible_size = scroll_area.1.size() * scroll_area.1.inverse_scale_factor; | ||
|
|
||
| // Size of the scrolling content. | ||
| let content_size = scroll_area.1.content_size() * scroll_area.1.inverse_scale_factor; | ||
|
|
||
| // Length of the scrollbar track. | ||
| let track_length = scrollbar_node.size() * scrollbar_node.inverse_scale_factor; | ||
|
|
||
| fn size_and_pos( | ||
| content_size: f32, | ||
| visible_size: f32, | ||
| track_length: f32, | ||
| min_size: f32, | ||
| offset: f32, | ||
| ) -> (f32, f32) { | ||
| let thumb_size = if content_size > visible_size { | ||
| (track_length * visible_size / content_size) | ||
| .max(min_size) | ||
| .min(track_length) | ||
| } else { | ||
| track_length | ||
| }; | ||
|
|
||
| let thumb_pos = if content_size > visible_size { | ||
| offset * (track_length - thumb_size) / (content_size - visible_size) | ||
| } else { | ||
| 0. | ||
| }; | ||
|
|
||
| (thumb_size, thumb_pos) | ||
| } | ||
|
|
||
| for child in children { | ||
| if let Ok(mut thumb) = q_thumb.get_mut(*child) { | ||
| match scrollbar.orientation { | ||
| ControlOrientation::Horizontal => { | ||
| let (thumb_size, thumb_pos) = size_and_pos( | ||
| content_size.x, | ||
| visible_size.x, | ||
| track_length.x, | ||
| scrollbar.min_thumb_length, | ||
| scroll_area.0.offset_x, | ||
| ); | ||
|
|
||
| thumb.top = Val::Px(0.); | ||
| thumb.bottom = Val::Px(0.); | ||
| thumb.left = Val::Px(thumb_pos); | ||
| thumb.width = Val::Px(thumb_size); | ||
| } | ||
| ControlOrientation::Vertical => { | ||
| let (thumb_size, thumb_pos) = size_and_pos( | ||
| content_size.y, | ||
| visible_size.y, | ||
| track_length.y, | ||
| scrollbar.min_thumb_length, | ||
| scroll_area.0.offset_y, | ||
| ); | ||
|
|
||
| thumb.left = Val::Px(0.); | ||
| thumb.right = Val::Px(0.); | ||
| thumb.top = Val::Px(thumb_pos); | ||
| thumb.height = Val::Px(thumb_size); | ||
| } | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
Similarly here I was thinking of a flattening like this:
| // Size of the visible scrolling area. | |
| let visible_size = scroll_area.1.size() * scroll_area.1.inverse_scale_factor; | |
| // Size of the scrolling content. | |
| let content_size = scroll_area.1.content_size() * scroll_area.1.inverse_scale_factor; | |
| // Length of the scrollbar track. | |
| let track_length = scrollbar_node.size() * scrollbar_node.inverse_scale_factor; | |
| fn size_and_pos( | |
| content_size: f32, | |
| visible_size: f32, | |
| track_length: f32, | |
| min_size: f32, | |
| offset: f32, | |
| ) -> (f32, f32) { | |
| let thumb_size = if content_size > visible_size { | |
| (track_length * visible_size / content_size) | |
| .max(min_size) | |
| .min(track_length) | |
| } else { | |
| track_length | |
| }; | |
| let thumb_pos = if content_size > visible_size { | |
| offset * (track_length - thumb_size) / (content_size - visible_size) | |
| } else { | |
| 0. | |
| }; | |
| (thumb_size, thumb_pos) | |
| } | |
| for child in children { | |
| if let Ok(mut thumb) = q_thumb.get_mut(*child) { | |
| match scrollbar.orientation { | |
| ControlOrientation::Horizontal => { | |
| let (thumb_size, thumb_pos) = size_and_pos( | |
| content_size.x, | |
| visible_size.x, | |
| track_length.x, | |
| scrollbar.min_thumb_length, | |
| scroll_area.0.offset_x, | |
| ); | |
| thumb.top = Val::Px(0.); | |
| thumb.bottom = Val::Px(0.); | |
| thumb.left = Val::Px(thumb_pos); | |
| thumb.width = Val::Px(thumb_size); | |
| } | |
| ControlOrientation::Vertical => { | |
| let (thumb_size, thumb_pos) = size_and_pos( | |
| content_size.y, | |
| visible_size.y, | |
| track_length.y, | |
| scrollbar.min_thumb_length, | |
| scroll_area.0.offset_y, | |
| ); | |
| thumb.left = Val::Px(0.); | |
| thumb.right = Val::Px(0.); | |
| thumb.top = Val::Px(thumb_pos); | |
| thumb.height = Val::Px(thumb_size); | |
| } | |
| }; | |
| } | |
| } | |
| let axis = match scrollbar.orientation { | |
| ControlOrientation::Horizontal => 0, | |
| ControlOrientation::Vertical => 1, | |
| }; | |
| // Size of the visible scrolling area. | |
| let visible_length = scroll_area.1.size()[axis] * scroll_area.1.inverse_scale_factor; | |
| // Size of the scrolling content. | |
| let content_length = | |
| scroll_area.1.content_size()[axis] * scroll_area.1.inverse_scale_factor; | |
| // Length of the scrollbar track. | |
| let track_length = scrollbar_node.size()[axis] * scrollbar_node.inverse_scale_factor; | |
| for child in children { | |
| if let Ok(mut thumb) = q_thumb.get_mut(*child) { | |
| let offset = match scrollbar.orientation { | |
| ControlOrientation::Horizontal => scroll_area.0.offset_x, | |
| ControlOrientation::Vertical => scroll_area.0.offset_y, | |
| }; | |
| let thumb_length = if content_length > visible_length { | |
| (track_length * visible_length / content_length) | |
| .max(scrollbar.min_thumb_length) | |
| .min(track_length) | |
| } else { | |
| track_length | |
| }; | |
| let thumb_pos = if content_length > visible_length { | |
| offset * (track_length - thumb_length) / (content_length - visible_length) | |
| } else { | |
| 0. | |
| }; | |
| match scrollbar.orientation { | |
| ControlOrientation::Horizontal => { | |
| thumb.top = Val::Px(0.); | |
| thumb.bottom = Val::Px(0.); | |
| thumb.left = Val::Px(thumb_pos); | |
| thumb.width = Val::Px(thumb_length); | |
| } | |
| ControlOrientation::Vertical => { | |
| thumb.left = Val::Px(0.); | |
| thumb.right = Val::Px(0.); | |
| thumb.top = Val::Px(thumb_pos); | |
| thumb.height = Val::Px(thumb_length); | |
| } | |
| } | |
| } | |
| } |
This is a somewhat contentious point: I'm firmly in the "scrollbars != sliders" camp. Scrollbars and sliders differ in a number of ways:
Trying to build a single widget that satisfies all of these requirements would end up being very complex and swiss-army-knifey. |
Yeah, I'm ok with things as-is. And I'm fine with you (or anyone else) refactoring the code later. |
Co-authored-by: ickshonpe <david.curthoys@googlemail.com>
Co-authored-by: ickshonpe <david.curthoys@googlemail.com>
Example now highlights thumb while dragging.
alice-i-cecile
added
S-Ready-For-Final-Review
Objective
Part of #19236
Demo
https://discord.com/channels/691052431525675048/743663673393938453/1387110701386039317