This repository was archived by the owner on Feb 25, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 6k
Only reject gestures to embedded UIViews when the framework says so. #7307
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously the framework could only tell the engine to forward a touch sequence to an embeded UIView between the time touches has started and the time touches ended. This couldn't support gesture arena setups where the gesture is recognized after the touch sequence is complete (e.g a tap competing with a scroll). This change makes it so that a touch gesture is only finally rejected by a platform view when the framework invokes the `rejectGesture` method. This allows the framework to resolve a gesture conflict after the touch sequence was ended.
amirh
added a commit
to amirh/flutter
that referenced
this pull request
Dec 26, 2018
flutter/engine#7307 changes the engine side of embedded UIView to only reject gestures when the framework sends a `rejectGesture` message, so that gesture resolution can done after a touch sequence has ended (see PR description for flutter/engine#7307 for more details). This change makes the framework send a `rejectGesture` message to the engine when a UiKitView rejects a gesture. I'm planning to land this PR before the engine side change, so right now it swallows the exception thrown if there is no engine implementation for `rejectGesture` (which keeps us with the current behavior). After this change lands I'll land the engine PR, and then clean up the part that swallows the exception.
liyuqian
approved these changes
Dec 26, 2018
Contributor
liyuqian
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RSLGTM.
I'm really unfamiliar with Objective C... Is there a test that fails the old code and passes the new code? That would make my rubber stamp much stronger...
Contributor
Author
|
I wish we had a test, we don't have the framework for integration tests that are doing gestures on iOS right now 😞 . There is a "manual" test, if you run the google_maps_flutter example app right now and tap on the marker in the "scrolling map" sample, the tap won't register (as it is a delayed gesture resolution), with this change and flutter/flutter#25792 it works. |
Contributor
Author
|
Going to land with the RSLGTM, TBR @chinmaygarde. |
amirh
added a commit
to flutter/flutter
that referenced
this pull request
Dec 27, 2018
flutter/engine#7307 changes the engine side of embedded UIView to only reject gestures when the framework sends a `rejectGesture` message, so that gesture resolution can done after a touch sequence has ended (see PR description for flutter/engine#7307 for more details). This change makes the framework send a `rejectGesture` message to the engine when a UiKitView rejects a gesture. I'm planning to land this PR before the engine side change, so right now it swallows the exception thrown if there is no engine implementation for `rejectGesture` (which keeps us with the current behavior). After this change lands I'll land the engine PR, and then clean up the part that swallows the exception.
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Dec 27, 2018
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Dec 27, 2018
amirh
added a commit
to amirh/engine
that referenced
this pull request
Dec 27, 2018
…ays so. (flutter#7307)" This reverts commit cc9c670.
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Dec 27, 2018
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Dec 27, 2018
amirh
added a commit
to amirh/engine
that referenced
this pull request
Dec 27, 2018
…ys so. (flutter#7307)" This reverts commit 20ee4e3.
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Dec 27, 2018
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Dec 27, 2018
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Dec 27, 2018
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Dec 28, 2018
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Dec 28, 2018
This was referenced Dec 28, 2018
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Dec 28, 2018
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Dec 28, 2018
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Dec 28, 2018
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously the framework could only tell the engine to forward a touch
sequence to an embeded UIView between the time touches has started and
the time touches ended. This couldn't support gesture arena setups where
the gesture is recognized after the touch sequence is complete (e.g a
tap competing with a scroll).
This change makes it so that a touch gesture is only finally rejected by
a platform view when the framework invokes the
rejectGesturemethod.This allows the framework to resolve a gesture conflict after the touch
sequence was ended.
Will land this after flutter/flutter#25792
fixes flutter/flutter#24076
fixes flutter/flutter#24207