search

githuboftigran / rn-range-slider

A native slider with range
MIT License
234 stars 130 forks source link

ScrollView integration, stuttering on mount and other issues #125

Open C-Hg opened 1 year ago

C-Hg commented 1 year ago

Hi everyone, I reproduced many issues mentioned here like the slider stuttering on mount or update of the parent component, or the impossibility to scroll over the slider when it's embedded in a ScrollView.

Like many others, I forked and fixed these issues so feel free to have a look at my fork if you're interested. To fix the stuttering on mount or update, you need to provide default values at execution time so the component is mounted properly. I also managed to provide the expected behaviour of a slider : modify the value only if one of the thumb is touched, and provide an escape gesture.

Despite pull requests and issues being opened for some time now, this repository is inactive. If @githuboftigran is willing to maintain it, I'd be very happy to propose pull requests and help maintaining it, but I won't waste time doing it for nothing. If it is not maintained anymore, we'll have to publish another npm package altogether because there are other things to do like Typescript support and ViewPropTypes warning.

Thanks @githuboftigran for providing this useful package

githuboftigran commented 1 year ago

@C-Hg , thank you for this issue. Just added TS support and fixed the problem with ViewPropTypes. I had a look at your fork, that's a great idea. However user should be able to change the value by tapping on any point on the rail (doesn't work for now though). There is even an issue for that: https://github.com/githuboftigran/rn-range-slider/issues/115

I added a restriction on movement in version 2.2.1. So when user moves more horizontally, the slider handles the touches. Need to think about what's more natural and how to achieve this... Maybe it would be better to add a new prop like "touchThumbOnly" ? Not the best name, but you you got the point right?

C-Hg commented 1 year ago

Hi @githuboftigran and thanks for your response, I'm glad that you're maintaining this package. I'll update my work with the latest version of this package and I'll prepare pull requests with a single feature so it's easier to read. I think I'll have time to take a look at the issue #115 this week, which is related to what I've ween working on. I agree about the need to be able to disable it with a new prop.

githuboftigran commented 1 year ago

@C-Hg , I had a look at your fork. I will reuse your code and create a PR myself. Will add you as a reviewer, if you don't mind, so you won't spend much time on this.

C-Hg commented 1 year ago

it's fine by me, go ahead, thanks

C-Hg commented 1 year ago

@githuboftigran don't hesitate to ping me if you need help; otherwise give me a sign when you're done !