scrollbar helper functions #21937
scrollbar helper functions #21937
Conversation
ickshonpe
added
A-UI
kfc35
left a comment
kfc35
left a comment
There was a problem hiding this comment.
Just minor, non blocking comments, LGTM
| )) | ||
| } | ||
|
|
||
| /// Compute the bounds of the horizontal scrollbar and the thumb |
There was a problem hiding this comment.
| /// Compute the bounds of the horizontal scrollbar and the thumb | |
| /// Compute the bounds of the vertical scrollbar and the thumb |
| } | ||
| let thumb_len = gutter_length * gutter_length / content_length; | ||
| let thumb_min = gutter_min | ||
| + scroll_position / (content_length - gutter_length) * (gutter_length - thumb_len); |
There was a problem hiding this comment.
You have the option to simplify this to:
| + scroll_position / (content_length - gutter_length) * (gutter_length - thumb_len); | |
| + scroll_position * gutter_length / content_length; |
If you find it too verbose
I understand if you want to keep it written the way you have it since they’re equivalent / you may find it clearer the way you wrote it.
At the very least, I think it’s easier to read the line was changed to have the multiplicative term first i.e.
| + scroll_position / (content_length - gutter_length) * (gutter_length - thumb_len); | |
| + scroll_position * (gutter_length - thumb_len) / (content_length - gutter_length); |
so it can be seen as multiplying by the ratio of “scrollable length in scrollbar” to “original hidden content length"
The new tests you wrote pass for me when I replace with the first suggested line.
| let content_inset = self.content_inset(); | ||
| let half_size = 0.5 * self.size; | ||
| let min_x = -half_size.x + content_inset.left; | ||
| let max_x = half_size.x - content_inset.right - self.scrollbar_size.x; |
There was a problem hiding this comment.
These will probably have to be changed dependent on the timing of your PR titled "UI scrollbars clipping fix" being merged #21910 , so that these subtractions of scrollbar_size can be removed when applicable
| let max_x = half_size.x - content_inset.right; | ||
| let min_x = max_x - self.scrollbar_size.x; | ||
| let min_y = -half_size.y + content_inset.top; | ||
| let max_y = half_size.y - content_inset.bottom - self.scrollbar_size.y; |
There was a problem hiding this comment.
Same comment here about your other PR and the timing of merges for the removal of - self.scrollbar_size.y
2 participants
Objective
Add helper functions to
ComputedNodeto compute the scrollbar areas of a UI node.Solution
Add
horizontal_scrollbarandvertical_scrollbarfunctions toComputedNode. If a scrollbar is present, these return an option wrapping a tuple of the node-centered boundingRectfor the scrollbar, and the start and end points of the thumb.Testing
Includes a couple of tests in the
uinodemodule.