sproutcore / sproutcore

JavaScript Application Framework - JS library only
sproutcore.com
Other
2.15k stars 291 forks source link

Some controls doesn't respond to touch anymore #1250

Closed nicolasbadia closed 10 years ago

nicolasbadia commented 10 years ago

Because of this commit https://github.com/sproutcore/sproutcore/commit/50ddf31649ba9482f611a21ca9ef7c90201c9cd7 which adds if (evt.which !== 1) return false; in many views to accept only primary button clicks, some controls (SC.CheckboxView, SC.RadioView, SC.SliderView) doesn't work anymore on touch devices. Indeed a touch event doesn't have a which property (thanks to @dcporter for helping debugging this).

dcporter commented 10 years ago

Any view with Tyler's left-click-only update which also proxies its touch events through its mouse event handlers is going to be affected. The quick and dirty solution would beto add an || SC.none(evt.which) check to each. A more elegant solution may be to add a method to SC.Control which takes an event and returns whether it's from the primary pointer. If that can be turned into an elegant method name, we can hide as much complexity in it as we want. Thoughts?

publickeating commented 10 years ago

That was a short-sighted mistake. I should have remembered immediately that touchStart is often forwarded on to mouseDown. Of course, that approach is also a short-sighted design. If we're going to share code between touch and mouse, it should be a separate function that each can call as appropriate.

I'd say we either rollback the commit or else spend some time cleaning up each of these views properly.

dcporter commented 10 years ago

Given how nice it is to be able to %$!$ing right-click on buttons to inspect them, I vote the latter.