Navigation recenter, pinch and bubbles options #1628
Conversation
Update to origin flot
…propagation; Improve test.
source/jquery.flot.touch.js
Outdated
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| if (!plot.getOptions().propagateOriginTouch) { |
There was a problem hiding this comment.
I think this condition can become:
if ((e.touches.length === 1 && options.pan.enableTouch)
|| (e.touches.length === 2 && options.zoom.enableTouch))
This should equate to if a user has pan enabled and does a pan gesture we will stop the propagation of the event, and will similarly do so when a user has zoom enabled and performs a pinch gesture.
The extra propagateOriginTouch option can present a confusing state where a user can still propagate the event despite it being handled. I don't see a good reason to allow this.
There was a problem hiding this comment.
I am afraid that this way would not meet our expectation. We'd like to enable pan, but disable zoom(pinch). In this case, the first touchstart event and the last touchend of a pinch interaction will be stopped from propagation, because these events have only one touch (the second touch does not start or already ends). However, the gesture recognizer hammerjs we used outside need all of those events to recognize the actual gesture. I have tried and found out the outside hammer fails.
By the way, completely or partially stopping propagation of the origin events by a gesture recognizer seems not very common (while preventing default is common). I tested hammerjs that it does not stop any origin events even the events are handled as a gesture. So I would say propagate handled events is not a confusing behavior, actually, it is the expected behavior for most cases.
Again, my suggestion is, at least, to have an option to completely disable the stopping-event-propagation behavior. I even agree to completely remove it.
There was a problem hiding this comment.
That's fine. Can we rename the option to propagateSupportedGestures?
There was a problem hiding this comment.
I feel it a little misleading to name this option with gesture. It controls the behavior of the origin touch events like touchstart and touchmove, while I will understand gesture as pinchmove or doubletap. propagateSupportedGesture sounds like it will allow the gesture event (e.g. pinchmove) to bubble.
Anyway, I can rename it as your comments if you insist.
| @@ -32,7 +32,7 @@ The plugin supports these options: | |||
| enableTouch: true | |||
| } | |||
|
|
|||
| propagateOriginTouch: false, | |||
| propagateSupportedGesture: false, | |||
There was a problem hiding this comment.
I renamed it now since I have not got your agreement to keep it propagateOriginTouch. I hope we can have the version bump ASAP then I will make changes into webcharts.
This PR is a combination of #1621, #1622, and #1625.
That PRs have some conflict with each other. I fix the conflicts to make it easier to pull. @atmgrifter00