Fix range slider initial position, allow thumb positions to be equal #2182
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
…e accessed when thumb positions are the same, fix initial position of min thumb
Inbal-Tish
reviewed
Sep 8, 2022
Inbal-Tish
reviewed
Sep 8, 2022
| @@ -678,7 +678,9 @@ export default class Slider extends PureComponent<SliderProps, State> { | |||
| > | |||
| {this.renderTrack()} | |||
| <View style={styles.touchArea} onTouchEnd={this.handleTrackPress}/> | |||
| {useRange && this.renderMinThumb()} | |||
| <View style={[!this.isDefaultThumbActive() ? {zIndex: 1} : {zIndex: 0}, {top: '-50%'}]}> | |||
There was a problem hiding this comment.
Suggested change
| <View style={[!this.isDefaultThumbActive() ? {zIndex: 1} : {zIndex: 0}, {top: '-50%'}]}> | |
| <View style={{zIndex: this.isDefaultThumbActive() ? 0 : 1, top: '-50%'}}> |
|
@ajp8164 Hi. Thank you for your contribution, PR looks excellent! I saw you wrote that adding support for a gap is not in the scope of this PR, but without it, we're looking at UI-breaking change. We need to support it and pass 4 (MIN_RANGE_GAP) as the default, allowing users to choose the gap distance or pass 0 as no gap (or just choose no gap as boolean prop). Do you think you can add this to this PR? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 range slider thumbs exhibited an arbitrary gap when they are moved next to each other. This gap should be zero so that the selected range is n..n (e.g. 10..10) when the thumbs are coincident. The arbitrary gap calculation imposed by the prior implementation results in different behavior based on step and range values.
This solution allows the range endpoints to be equal (no gap). The solution also ensures that the thumbs can always be accessed when thumb positions are the same (100% overlap). Fix initial position of minimum range thumb. The prior implementation was using the user defined range value as a "pixel", "x" value which resulted in the min track having the incorrect stop (filled in to the wrong x position).
There could be a new feature added to maintain a "gap" feature. This is out of scope for this PR.
Can ignore the import re-ordering - done by my lint rules.
Changelog
Eliminate the minimum gap between range slider thumbs (minimum gap is now 0).
Fix the initial position of the minimum range thumb.