Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slider to be be responsive to click #92

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

ajaswal
Copy link

@ajaswal ajaswal commented Nov 29, 2017

Hi @jeanregisser

I was using the Slider as a seek bar for a video player and required to make it clickable. I have included this as a prop called trackClickable, (sorry, I am bad with var naming conventions) so that the changes don't break anything.

@UI-Animation-Chen
Copy link

thanks a lot :)

@eyalcohen4
Copy link

eyalcohen4 commented Dec 4, 2017

Hey guys! Is there something needed to do here? I really want this option 😃
cc @UI-Animation-Chen @ajaswal

@UI-Animation-Chen
Copy link

just wait for the merge :) @eyalcohen4

@ajaswal
Copy link
Author

ajaswal commented Dec 5, 2017

@eyalcohen4 @UI-Animation-Chen Thanks, guys. As of now, I am pointing to my fork's release to make things work for me. You can try out the same if it's very urgent for you but I strongly recommend to revert to this once merged.

@xiesw
Copy link

xiesw commented Dec 6, 2017

thanks 👍

@dummerbd
Copy link

dummerbd commented Dec 6, 2017

@jeanregisser any chance this could be merged and released soon? I'd really love to have this feature

src/Slider.js Outdated
_handlePanResponderGrant = (/*e: Object, gestureState: Object*/) => {
this._previousLeft = this._getThumbLeft(this._getCurrentValue());
_handlePanResponderGrant = (e: Object, gestureState: Object) => {
this._previousLeft = this.props.trackClickable ? gestureState.x0 - (this.state.thumbSize.width/2) : this._getThumbLeft(this._getCurrentValue());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gestureState.x0 is relative to the root component, this should be e.nativeEvent.locationX which is relative to the panResponder component.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also need to change:
this.state.thumbSize -> this.props.thumbTouchSize

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dummerbd @sreym Thanks for the review. I will make the changes and test them once. Haven't been working on RN lately.

@ouabing
Copy link

ouabing commented Dec 26, 2017

Hope it to be merged too :)

@zaguiini
Copy link

So...

sreym pushed a commit to hola/react-native-slider that referenced this pull request May 16, 2018
make Slider responsive to clicks
src/Slider.js Outdated
@@ -315,7 +322,7 @@ export default class Slider extends PureComponent {

_handlePanResponderGrant = (e: Object, gestureState: Object) => {
// this._previousLeft = this._getThumbLeft(this._getCurrentValue());
this._previousLeft = gestureState.x0 - (this.state.thumbSize.width/2);
Copy link

@ghost ghost Jul 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

react-native-slider is still not clickable
Does anyone have a solution for this?
I made the above changes to the Slider.js file. It still does not work for Android or IOS.
Slider.txt

@ajaswal ajaswal force-pushed the master branch 2 times, most recently from 7cbd788 to 48c3395 Compare July 13, 2018 01:31
@Lizhooh
Copy link

Lizhooh commented Jul 21, 2018

Half a year has passed, and there is still no merger. Do you have any questions about this?

@GerardCasadevall
Copy link

any progress with the PR?

@Jacob-o
Copy link

Jacob-o commented Apr 15, 2020

Could this be merged to master please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet