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

Only set interacting hint when modifying the view #1398

Closed
wants to merge 1 commit into from
Closed

Only set interacting hint when modifying the view #1398

wants to merge 1 commit into from

Conversation

elemoine
Copy link
Member

This PR follows up on #1277 by applying the same changes to the touch interaction.

This should also fix a bug in the vector-api branch where vector layers are not redrawn after pinch-zooming. @oterral, you can use my vector-api-interaction-hint if you want to test my commit with the vector-api branch.

Please test and review.

@twpayne
Copy link
Contributor

twpayne commented Dec 17, 2013

This should also fix a bug in the vector-api branch where vector layers are not redrawn after pinch-zooming.

Hmmm, on my Nexus 7 with this branch I still have this bug. Not sure why yet.

@elemoine
Copy link
Member Author

Hmmm, on my Nexus 7 with this branch I still have this bug. Not sure why yet.

Thanks a lot for testing @twpayne. There certainly are other bugs.

@elemoine
Copy link
Member Author

Here's one: in ol.MapBrowserEventHandler#handleTouchEnd_ we deregister the handleTouchMove_ and handleTouchEnd_ listeners.

This means that in the sequence "touchstart (finger 1), touchstart (finger 2), touchend (finger 1), touchend (finger 2)" we won't see the last touchend event because we deregistered the touchend listener on the first touchend event.

Similarly if we pan the pan with one finger, then do touchstart touchend with a second finger, then you can no longer with the first finger - the touchmove listener was deregistered when the second finger caused a touchend.

@elemoine
Copy link
Member Author

I added a commit to conditionally register/deregister the touchend listener.

@oterral
Copy link
Contributor

oterral commented Dec 17, 2013

I'm testing

@oterral
Copy link
Contributor

oterral commented Dec 17, 2013

It works if you release the 2 fingers in the same time. If you pinch then release a finger then release the 2nd finger, the rerender of the vector layer is not done then it doesn't work forever :) . Probably the event is deregister after the 1st finger is released.

@elemoine
Copy link
Member Author

Thanks for testing @oterral. What browser have you used?

@oterral
Copy link
Contributor

oterral commented Dec 18, 2013

Safari and chrome on iOS 7

@elemoine
Copy link
Member Author

Thanks. I'm trying to reproduce it.

@@ -135,6 +139,7 @@ ol.interaction.TouchPan.prototype.handleTouchStart =
// No kinetic as soon as more than one fingers on the screen is
// detected. This is to prevent nasty pans after pinch.
this.noKinetic_ = this.targetTouches.length > 1;
view.setHint(ol.ViewHint.INTERACTING, 1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible issue here: ol.ViewHint.INTERACTING is incremented on every new touch:
this.targetTouches.length > 0

but decremented only when the last touch leaves the screen:
this.targetTouches.length === 0

@elemoine
Copy link
Member Author

Thanks for the comments @fredj. I'm closing this PR in favor of a new one.

@elemoine elemoine closed this Dec 18, 2013
@elemoine elemoine deleted the interaction-hint branch December 18, 2013 14:56
@elemoine elemoine restored the interaction-hint branch December 18, 2013 14:56
@elemoine elemoine deleted the interaction-hint branch December 18, 2013 16:37
afabiani pushed a commit to geosolutions-it/openlayers that referenced this pull request Nov 7, 2017
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.

None yet

4 participants