MouseTracker - Pointer over/out event fixes #448

msalsbery · 2014-08-05T21:00:00Z

gasi · 2014-08-05T21:12:27Z

src/button.js

+        if ( this.element.style[ "touch-action" ] !== undefined ) {
+            this.element.style[ "touch-action" ] = "none";
+        } else if ( this.element.style[ "-ms-touch-action" ] !== undefined ) {
+            this.element.style[ "-ms-touch-action" ] = "none";


Shouldn’t this be this.element.style.msTouchAction? Found this here: http://blogs.msdn.com/b/thebeebs/archive/2012/01/11/with-vendor-prefixes-what-is-the-javascript-equivalent-of-ms.aspx

Hi Daniel, I thought so too, and I thought I found that in the IE docs, but now I'm going to look again!

Well I can't find the example of course :) So what's the proper way to do this? I'd much rather use the dot syntax over the key string syntax, which means I chose that way for a reason I wish I could remember. My only worry is cross-browser compatibility, and currently both ways work on the only two browsers that have touch-action implemented: IE 10/11 and Chrome (latest). Thanks!

Can you try this.element.style.touchAction and this.element.style.msTouchAction? Then again, I’d recommend creating a helper function that auto-prefixes vendor specific CSS properties, e.g. https://github.com/peutetre/vendor-prefix.

Then again, if it works, maybe just leave it and sorry for the intrusion 😉

Nice! Re: code snippet. Here’s another one I learned from our dear friend (and my former mentor) @iangilman: If you got three or more copies, consider extracting it 😉

He may make me do it now, even though I slipped it through review before. Thanks. ;)

Fixed, before he even saw it :)

gasi · 2014-08-06T04:23:14Z

src/openseadragon.js

+            } else if ( typeof element.style.msTouchAction !== 'undefined' ) {
+                element.style.msTouchAction = 'none';
+            }
+        },


👍 Well done, sir 😄

dom96 · 2014-08-06T14:42:20Z

I just noticed a possible bug that this PR introduces. Setting flickEnabled to True for gestureSettingsMouse causes panning issues on the desktop. Especially when around the edge of the image. Can anyone confirm?

msalsbery · 2014-08-06T14:49:20Z

@dom96 What kind of issues?

dom96 · 2014-08-06T14:50:05Z

@msalsbery the viewport gets stuck at the corners of the image.

msalsbery · 2014-08-06T14:56:03Z

@dom96 I can't see anything on Windows 8.1 IE or Firefox.

On the demo site or using those builds separately? OS and browser?

MouseTracker only processes browser events and calls common event callbacks so results in the viewport should be identical with mouse or touch.

dom96 · 2014-08-06T15:41:31Z

@msalsbery separately. On Windows 7 with Firefox. I'll see if i can
reproduce it on your demo site as soon as i can, likely tomorrow. Keep in
mind that i had to explicitly enable the 'flick' to get it going on the
desktop, it does not happen on tablets.

On Wednesday, August 6, 2014, Mark Salsbery notifications@github.com
wrote:

@dom96 https://github.com/dom96 I can't see anything on Windows 8.1 IE
or Firefox.

On the demo site or using those builds separately? OS and browser?

MouseTracker only processes browser events and calls common event
callbacks so results in the viewport should be identical with mouse or
touch.

—
Reply to this email directly or view it on GitHub
#448 (comment)
.

msalsbery · 2014-08-06T15:45:32Z

@dom96 Try setting visibilityRatio on the viewer to something like 0.25.

Flick is only enabled by default for true browser touch events, so if the tablet browser converts touch events to legacy mouse events then you have to manually enable it as you've seen.

Thanks for the testing and feedback!

iangilman · 2014-08-06T16:41:59Z

Looks good to me. Great to see all of the testing on the various issues! @msalsbery could you add changelog for it? (Usually I don't ask contributors to do so, but this is a big enough patch, you might as well). Also, do any tests need updating?

I'll hold off on merging until we've heard back from @dom96.

msalsbery · 2014-08-06T17:03:43Z

@iangilman Will-do on the changelog. Tested the tests, no changes requiring test updates.

Tested on:
Windows 7 IE, Firefox, Chrome (mouse)
Windows 8.1 IE, Firefox, Chrome (mouse and multi-touch)
iMac OSX 10.9.4 Safari, Firefox (mouse)
iOS 7.1.2 Safari (multi-touch)

msalsbery · 2014-08-06T17:13:36Z

@dom96 I see with mouse flick gesture enabled you can drag the image outside the viewport and it stays out of sight. Is that what you're referring to?

That's a bug, fixing...

dom96 · 2014-08-06T17:45:45Z

@msalsbery that might be it. It didn't really stay out of sight for me though. My viewport was at the very edge of the image, and then I tried panning in the opposite direction (away from the corner) at which point the 'flick' feature seems to have moved it back to the corner. The issue is a bit intermittent, and after a while of trying to pan in the opposite direction it does get unstuck.

…/releasePointer() functions

…abled

msalsbery · 2014-08-06T18:31:54Z

@dom96 I wasn't able to reproduce any unexpected behavior, but I've fixed it so you can't drag the image out of view when flick gesture is enabled (demo site updated with new OpenSeadragon builds). Hopefully your issue was related to that.

msalsbery · 2014-08-06T18:39:19Z

@iangilman New commits, changelog updated

dom96 · 2014-08-07T09:31:33Z

@msalsbery unfortunately that doesn't fix it. I can reproduce it in your demo site but it's very subtle. If you pan to a corner and then gently flick away from it, it will pan back to its original position. From what I can tell this is more prominent with custom tiles sources, or perhaps larger images. I tried a number of different settings and changing to a custom tile source seems to have the biggest effect. I have tried my custom tile source in your demo page too and I can reproduce it. The flick setting that I am using is gestureSettingsMouse: { flickEnabled: true, flickMinSpeed: 40, flickMomentum: 0.5 },

msalsbery · 2014-08-07T14:52:33Z

@dom96 That is as-designed (or as implemented anyway), the dreaded Viewport.applyConstraints() function. It shouldn't happen if the image is zoomed larger than the viewport. I've never liked its behavior but every time I go in to change it I bail out.

MouseTracker just handles browser events and passes them on to attached callbacks, so it's not relevant to this patch.

msalsbery · 2014-08-07T16:03:17Z

@dom96 Additionally, your flick settings are pretty aggressive, especially for the mouse device. I tried those settings originally when trying to work out reasonable defaults, but had to find a fairly dead compromise because of the applyConstraints behavior (that's why it's now flickMinSpeed: 120, flickMomentum: 0.25).

There's two other settings that come into play here: springStiffness and animationTime. These affect the "spring" physics that dampen the kinetic flick movement. Instead of pushing the flickMomentum setting up, you can try increasing the spring animation time and/or decreasing the spring stiffness.

When it seems to stick in the corner, it's actually taking the computed target based on the flick velocity and momentum setting, then trying to pan to that target. With the large momentum setting, the pan target is off the viewport so applyConstraints() brings it back in. The resulting appearance is it sticks. @iangilman called me out on it when I originally added the flick gesture with more aggressive settings :)

iangilman · 2014-08-07T16:17:27Z

Lovely... looks like this is ready to merge, assuming you're happy, @msalsbery.

msalsbery · 2014-08-07T16:38:19Z

@iangilman Go for it, thanks!

MouseTracker - Pointer over/out event fixes

msalsbery added 4 commits August 4, 2014 21:41

Mousetracker Over/Out Tracking Fixes

e110959

Merge branch 'master' into IE9-MouseTracker-Clicks

c6bea68

Trailing whitespace fixes

4152b8b

Fixed pointer event capture, Cleaned up debug code

3521bac

gasi reviewed Aug 5, 2014
View reviewed changes

msalsbery added 3 commits August 5, 2014 16:01

touch-action fixup

bf19e73

Removed debug code

be13d25

Added $.setElementTouchActionNone function

e9b7634

gasi reviewed Aug 6, 2014
View reviewed changes

msalsbery added 2 commits August 6, 2014 10:49

Combined redundant captureMouse()/capturePointer() and releaseMouse()…

ee3ab70

…/releasePointer() functions

Call Viewport.applyConstraints() on drag-end when flick gesture is en…

1d5b059

…abled

Changelog update

94f7ca3

iangilman added a commit that referenced this pull request Aug 7, 2014

Merge pull request #448 from msalsbery/IE9-MouseTracker-Clicks

df5f3cf

MouseTracker - Pointer over/out event fixes

iangilman merged commit df5f3cf into openseadragon:master Aug 7, 2014

iangilman mentioned this pull request Sep 11, 2014

Mouse events not reaching overlay elements #431

Closed

iangilman mentioned this pull request Nov 18, 2019

After browser zooming canvas takes up the entire screen and not able to zoom out #1534

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MouseTracker - Pointer over/out event fixes #448

MouseTracker - Pointer over/out event fixes #448

msalsbery commented Aug 5, 2014

gasi Aug 5, 2014

msalsbery Aug 5, 2014

gasi Aug 5, 2014

msalsbery Aug 5, 2014

gasi Aug 5, 2014

msalsbery Aug 5, 2014

gasi Aug 5, 2014

msalsbery Aug 5, 2014

msalsbery Aug 5, 2014

iangilman Aug 6, 2014

gasi Aug 6, 2014

dom96 commented Aug 6, 2014

msalsbery commented Aug 6, 2014

dom96 commented Aug 6, 2014

msalsbery commented Aug 6, 2014

dom96 commented Aug 6, 2014

msalsbery commented Aug 6, 2014

iangilman commented Aug 6, 2014

msalsbery commented Aug 6, 2014

msalsbery commented Aug 6, 2014

dom96 commented Aug 6, 2014

msalsbery commented Aug 6, 2014

msalsbery commented Aug 6, 2014

dom96 commented Aug 7, 2014

msalsbery commented Aug 7, 2014

msalsbery commented Aug 7, 2014

iangilman commented Aug 7, 2014

msalsbery commented Aug 7, 2014

MouseTracker - Pointer over/out event fixes #448

MouseTracker - Pointer over/out event fixes #448

Conversation

msalsbery commented Aug 5, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dom96 commented Aug 6, 2014

msalsbery commented Aug 6, 2014

dom96 commented Aug 6, 2014

msalsbery commented Aug 6, 2014

dom96 commented Aug 6, 2014

msalsbery commented Aug 6, 2014

iangilman commented Aug 6, 2014

msalsbery commented Aug 6, 2014

msalsbery commented Aug 6, 2014

dom96 commented Aug 6, 2014

msalsbery commented Aug 6, 2014

msalsbery commented Aug 6, 2014

dom96 commented Aug 7, 2014

msalsbery commented Aug 7, 2014

msalsbery commented Aug 7, 2014

iangilman commented Aug 7, 2014

msalsbery commented Aug 7, 2014