This repository has been archived by the owner on Dec 28, 2021. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Scrollbar and ScrollArea #1614
Scrollbar and ScrollArea #1614
Changes from 1 commit
c6c5a94
46aba91
b4abaa0
406773d
bdf2e54
755274d
8f630bf
9aef6c9
f207f51
537e8af
d807ba6
274a72a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like pretty complex FRP - it seems it includes dragging etc. Why such things as dragging are not completely handled by slider instead? I was thinking that this component would just wrap slider and just set a few FRP values to it, without its own complex FRP utility. Also, there are no docs describing what really these functions do, which makes it much harder to understand - it contains some dragging, some mouse IO, while all of these things should be handled by the slider component.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The actual drag implementation and underlying logic is shared. But this is the logic how to interpret it, which is different in each component: (1) single number selectors have clicking at the target and dragging only on the background (2) range slider need to act on dragging either the track piece or the edges of the track piece (3) the scrollbar needs to have dragging of the track piece and "jumping" when clicking on some other part of the background.
That is essentially what is encoded here. Maybe some things could be refactored and abstracted, but not too much.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why cant these things be just coded in (moved to) the slider implementation so you'd be able to choose which mode you wants with FRP settings? Just like choosing colors, we could tell "I want this slider to jump to the place when clicking the background". This should be part of the sldier configuration rather than extracted here imo. Can we move it there, please?