Refactor: Base tap gesture recognizer - #41329
Conversation
goderbauer
left a comment
There was a problem hiding this comment.
Th looks fine with a couple of nits.
Just to re-confirm: This does not address the issue that different operating systems handle Modal dismissal differently, right? E.g. on iOS and Android 10 a tapUp is required to dismiss, on Android prior to 10 a tapDown is enough?
| /// | ||
| /// A tap is defined as a sequence of events that starts with a down, followed | ||
| /// by optional moves, then ends with an up. All move events must contain the | ||
| /// same `buttons` with the down event, and must not be too far from the initial |
There was a problem hiding this comment.
| /// same `buttons` with the down event, and must not be too far from the initial | |
| /// same `buttons` as the down event, and must not be too far from the initial |
There was a problem hiding this comment.
Buttons for the up event don't matter? Do all buttons have to come up at the same time?
There was a problem hiding this comment.
- Up events always have
buttons: 0. - If the pointer's
buttonschanges to a different non-zero value, it's considered a Move event. (for example, ifbuttonschanges from2 | 1to1, then there will be a Move event withbuttons: 1)
Therefore, since multiple buttons will almost always be pressed or released one at a time, BaseTapGestureRecognizer will not accept multi-button taps (which was an intentional design when we added button constraint to gestures)
No (but it does make it a little easier to do). |
|
@goderbauer All comments have been taken care of. PTAL |
|
is this something MultiTapGestureRecognizer can achieve? |
|
@chunhtai Which goal are you suggesting to achieve using Can you further illustrate? |
now i think of it, i think multitap might not be a good idea. nvm |
* Extracts the logic of TapGestureRecognizer into an abstract class BaseTapGestureRecognizer * Fixes ModalBarrier unable to dismiss when competing
Description
Currently
TapGestureRecognizeronly defines taps of primary button and taps of secondary button, and leaves no customization for other configurations. For example,ModalBarrierneeds a tap recognizer that recognizes the tap down of any button, therefore it has to copy the code and remove unnecessary parts. Not only is it redundant, but it has also caused bugs, such as in #41178 (comment).This PR extracts the logic of
TapGestureRecognizerinto an abstract classBaseTapGestureRecognizer, so that it is much easier to customize.BaseTapGestureRecognizerstill requires that the button is consistent throughout the gesture, but it's not opinionated on which buttons to accept, nor does it know what to on each event. In this way,TapGestureRecognizerand_AnyTapGestureRecognizercan be easily defined, and the logic of tap is also clearer.Related Issues
Tests
I added the following tests:
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Does your PR require Flutter developers to manually update their apps to accommodate your change?