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

Clean up slider logic and fix bug with top end #8805

Merged
merged 4 commits into from
May 19, 2016
Merged

Conversation

kball
Copy link
Contributor

@kball kball commented May 18, 2016

Fixes issue discovered in QA with top end of slider. Also cleans up slider logic to be more readable.

@brettsmason
Copy link
Contributor

@kball I can confirm this fixes the issue in the QA I reported 👍

@designerno1
Copy link
Contributor

designerno1 commented May 18, 2016

@kball
this makes it better for drag but unusable for touch android.
the vertical slider isn't moveable by toch after this PR

@kball
Copy link
Contributor Author

kball commented May 18, 2016

@designerno1 nice catch, I'll dig into it

@kball
Copy link
Contributor Author

kball commented May 18, 2016

@designerno1 updated and I believe it resolves on android. Touch slider on android feels a little clunky to me but going back to the initial RC it feels clunky there too, and in 6.2.1 it was broken entirely, so I think this is good enough. Can you try it out again?

@designerno1
Copy link
Contributor

@kball
now it works on android with touch
but jumps back on drag from top to bottom
keyboard go to top with up arrow works also fine

@kball
Copy link
Contributor Author

kball commented May 18, 2016

Ah the jump makes sense... ok still tinkering. The discrepancy between the real events and faked event stuff is a pain.

@kball
Copy link
Contributor Author

kball commented May 18, 2016

@designerno1 Got another pass on this. This feels kinda clunky, but after mucking around a bit I couldn't figure out how to do the better solution (fixing the touch util faked events), and this seems to be working now both on my android test device and the non-android browsers I've tested

@designerno1
Copy link
Contributor

now works by drag
but still buggy on android swipe and touch move

@kball
Copy link
Contributor Author

kball commented May 18, 2016

@designerno1 Was it working in those cases before? If not, I'd like to merge this and open those as a separate issue to tackle

@designerno1
Copy link
Contributor

@kball
yeah in the v6.2.2 branch touch on vertical slider works great

@kball
Copy link
Contributor Author

kball commented May 18, 2016

ok I'll keep pounding on it then

@kball
Copy link
Contributor Author

kball commented May 18, 2016

@designerno1 one more fix in for an issue on android 6, but I'm not sure exactly what behavior you're seeing still on this branch that isn't there on v6.2.2. Can you explain more what you mean by swipe as distinct from drag for the vertical slider? Thanks

@designerno1
Copy link
Contributor

@kball
with drag i mean click and hold mouse and move
with swipe i mean touch hold and pull up
with touch i mean tap on slider to move

@designerno1
Copy link
Contributor

@kball
now it looks goog on chrome,edge,firefox, android 6
awesome

@kball kball merged commit d45f88b into v6.2.2 May 19, 2016
@kball kball deleted the fix-slider-top-increment branch April 20, 2017 18:37
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.

3 participants