Add a function to remove a registered gesture recognizer in gestures #12271
Conversation
|
Merging master onto this branch for test fix in #12272. |
|
/to @cvializ @aghassemi |
| */ | ||
| removeGesture(recognizerConstr) { | ||
| const type = new recognizerConstr(this).getType(); | ||
| const overserver = this.overservers_[type]; |
There was a problem hiding this comment.
It'd be nice if we could get the type of the constructor without creating an instance, but it doesn't look like there's another way to do it right now without refactoring, maybe adding an inherited static method on the subclasses to replace the hardcoded string in the constructor. Not necessary to fix, just thinking out loud.
There was a problem hiding this comment.
Can investigate feasibility and add a GFI task to refactor.
| * @param {function(new:GestureRecognizer<DATA>, !Gestures)} recognizerConstr | ||
| * @return {boolean} | ||
| */ | ||
| removeGesture(recognizerConstr) { |
There was a problem hiding this comment.
Could this become a remove (or unlisten) on GestureRecognizer class and hence on every recognizer instance?
My concern with current approach is that it gives one part of the code ability to unregister handlers for everyone which breaks code isolation.
Is there a use-case that unregistering handlers individually is not enough?
There was a problem hiding this comment.
chatted offline, gesture instance is tied to the element so this is fine.
There was a problem hiding this comment.
Anymore changes to address or are we good for merge? =)
…ognizer in gestures (ampproject#12271)
Closes #12112.
This is a small PR. Please review the API for this new function and give feedback on whether it seems reasonable. The reason I'm proposing this function is because using
SwipeXYto pan onimage-viewerconflicts with usingSwipeYto swipe-to-close onamp-lightbox-viewer. We want to deregister theSwipeXYonimage-viewerwhen the image isn't scaled.This PR enables swipe up to close lightbox, prototype on feature branch, deployment in progress on heroku for iOS testing. The actual lightbox swipe to close feature will be in a separate PR that is based off of #12256.