-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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] Fixed behaviour when Slider is in a scrolling container #4511
base: master
Are you sure you want to change the base?
[Slider] Fixed behaviour when Slider is in a scrolling container #4511
Conversation
@@ -43,10 +44,11 @@ public View onCreateDemoView( | |||
for (int i = 0; i < 50; i++) { | |||
Slider slider = new Slider(layoutInflater.getContext()); | |||
slider.setValueTo(11f); | |||
slider.setOrientation(SliderOrientation.VERTICAL); |
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.
Could you create a separate demo for this?
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.
I created it.
@@ -2864,17 +2865,24 @@ public boolean onTouchEvent(@NonNull MotionEvent event) { | |||
} | |||
|
|||
float eventCoordinate = isVertical() ? event.getY() : event.getX(); |
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.
Rename this to eventCoordinateX
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.
I renamed it.
&& abs(eventCoordinate - touchDownX) < scaledTouchSlop) { | ||
return false; | ||
} | ||
// Check if we're trying to scroll horizontally instead of dragging this Slider | ||
if (isVertical() && isPotentialHorizontalScroll(event) | ||
&& abs(eventCoordinateY - touchDownY) < scaledTouchSlop / 1.2) { |
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.
Can you explain this 1.2
?
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.
It is difficult for me to explain it...
If scaledTouchSlop is not divided by 1.2, it is difficult to slide (drag) vertical sliders. Try it.
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.
Is this also applicable for the vertical scroll or is it specific to this horizontal scroll?
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.
It seems to be specific to horizontal scroll. Don't you think so?
closes #4510