feat: add boundary check to scrolling #956
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
The current implementation for scrolling never checks if the max columns is reached, letting the user scroll way out of bounds. While it looks not that pretty it is normally not a big problem, but I noticed that there are some corner cases where this get in the way of the user.
E.g. if you create large tiles or girds with only a few column you are constantly triggering scrolling since you are always need the sensitivity threshold.
Some examples:
out-of-bounds.mp4
no-boundary.mp4
What I did:
I added a check that the scroll will be canceld when the maxScrollPosition is reached.
After:
boundary-fixed.mp4
Open quesions for you :)
Since are quite a lot of settings, is there a case which you need to consider when computing the max scroll position other than relying on "maxCols" and the column width?
Also btw when I tested this patch I noticed that scrolling is quite broken when used in combination with "setGridSize" and grid type "scroll vertical", so the question is for which grid type should scrolling work when used in combination with setGridSize?