extnet / Ext.NET

35 stars 41 forks source link

Slider component spams preventDefault warning when interacting on Chrome #1432

Open fabriciomurta opened 7 years ago

fabriciomurta commented 7 years ago

When moving the Ext.form.Slider position around. Google chrome spams a warning on every notch the slider moves.

This might have been an issue introduced on newer chrome versions, and the warning points to https://www.chromestatus.com/features/5093566007214080.

The issue is triggered from Ext.dom.TouchAction.touchMove() (not documented, Ext.dom.TouchAction is a private ExtJS class).

This issue was found while merging ExtJS 6.2.1 (modern) into Ext.NET Mobile -- #1425.

fabriciomurta commented 7 years ago

This has been reproduced on ExtJS examples for the Slider component.

RByers commented 7 years ago

Sorry for the trouble, this is a breaking change in Chrome 56 to improve scroll performance. You probably need to add an appropriate touch-action CSS rule to explicitly disable touch scrolling.

fabriciomurta commented 7 years ago

Hello @RByers, thanks for your follow-up!

Well, a slider component without the ability to be scrolled in touch events pretty much means a static or disabled slider, right? So I guess we should explore the other alternatives in the link provided above and, probably, write specialized code for chrome, unfortunately.

RByers commented 7 years ago

No - if you're driving a scroll in response to touch events, then you want to disable the browser's built in scrolling (so that you can reliably provide your own replacement handling for the gesture in JS). In the example here, adding a rule like .x-draggable { touch-action: none; } fixes the problem. You might want to get a little fancier and use .x-draggable { touch-action: pan-y pinch-zoom; } to disable only horizontal scrolling so that a user can still scroll a page vertically (and pinch-zoom) even when touching on a slider element.

RByers commented 7 years ago

When testing that page on Microsoft Edge, I see it DOES apply an appropriate touch-action style (long been necessary to work with touch on IE/Edge). So there must be code for that somewhere already that's just not getting used on Chrome for some reason (perhaps because Chrome supports touch events in addition to pointer events?).

Also it uses a Microsoft-proprietary touch-action value double-tap-zoom, which would make the rule fail to parse in Chrome (since it's non-standard). I suggest you just remove the the double-tap-zoom value since it causes taps to be delayed by ~300ms anyway.

fabriciomurta commented 7 years ago

long been necessary to work with touch on IE/Edge

The application is actually meant to be run on mobile devices, thus we usually test the pages using chrome, developer tools enabled and in device emulation mode for android and ios devices.

But yes, yes! I got what you mean now. I actually was suspecting this would be the case. That's tough, either a visible performance improvement from chrome would make it to most people for the price of some apps needing to adequate to it, or leave the setting optional and have almost nobody notice about it, so I can understand the breaking change although it is a pity the "different browsers' code wars" is far from getting to an end :)

fabriciomurta commented 7 years ago

And yes, seems the best is either of the CSS overrides you suggested.

RByers commented 7 years ago

can understand the breaking change although it is a pity the "different browsers' code wars" is far from getting to an end :)

Thanks for your understanding! And our intention is definitely to get to a place where all browsers agree. I've been talking with them for years about how we solve this problem, and much of the design of passive event listeners came from feedback from Apple and Mozilla. And Microsoft deserves the credit for designing pointer events with this model from the start. The spec issue is tracked here.