-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Dismiss Modal Barrier on handleTapCancel
#98191
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
Dismiss Modal Barrier on handleTapCancel
#98191
Conversation
fix test title
handleTapCancel
| if (onAnyTapUp == null) | ||
| if (onAnyTapUp == null) { | ||
| return false; | ||
| } else if (onAnyTapCancel == null) { |
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.
The current logic return false if one of onAnyTapUp and onAnyTapCancel is null, maybe they should be &&?
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.
That makes sense, thanks!
chunhtai
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.
LGTM
|
This change causes a regression.
expect: alert dialog is dismissed and page stays at /abc Preparing for revert. |
This reverts commit 58ad6e1.
|
Since the original issue #90233 is locked, I'm adding a few things. If I'm not mistaken, the last time I saw the code for the popup it's using a ModalBarrier which is used on other types of popups (like dialogues). I believe it should have its own barrier, one that closes as soon as an interaction outside the popu menu is done. Take for example the Gmail App on Android. Here is an example of the drawer menu: If you hold the barrier or drag up/down it stays open. It closes by a right to left gesture (even initiated from the barrier) or a single tap on the barrier. This pop up to switch accounts. Stats open with hold or drags on the barrier but a single tap closes it. Finally this pop-up menu that opens when tapping the meatballs menu on the top right of the email. Any gesture outside of it closes it, as soon as the screen is touched (tap down) it closes immediately. There's no way to keep it open when interacting with anything outside of the menu. Now, the main reason I opened this issue is because I have had users complain that the app is frozen on a screen and sent me screenshots of a screen very similar to the last screenshot, where they open a pop-up menu that allows them to have multiple selections while open and then they try to scroll the screen behind it, the popup doesn't close and the screen behind doesn't scroll, which gives them the feeling that the app is frozen. Also, it's unbelievable the amount of people that think that to close a popup menu like the one in the third screenshot they have to "swipe up" instead of just a tap. I have had reports of people trying to get back to the screen for several minutes before filing the support ticket and even sent recorded videos of them picking options from the menu (which doesn't close since it's just picking on/off options) and then swiping outside the menu several times, fast and slow, in every direction but never tapping the screen with a single tap, believing somehow that the app is frozen and only the menu is interactive until they have to force close the app and reopen. My issue is temporarily resolved because I implemented a custom popup menu that closes as soon as a gesture is detected on the custom barrier, but it's a mess of a code, only works with my data. Then I tried the same thing on web and it's even worse. You'd expect that when you have a popup menu open and scroll with the mousewheel, that at least the content behind scrolls freely, but no. (Like it does here in GitHub if you click the meatballs menu on the top right of this comment and then scroll up/down). On some pages, this behaves differently. Some retain the menu in place relative to the initial position where it opened, others keep it attached to the button that originated the menu and others close it immediately. O have a couple of examples, like using Google docs and opening one of the context menu options (file, edit) then scrolling the page below, or clicking the options menu in chrome and scrolling the page while open (it closes). I believe that the Material Design guidelines states that a popup menu should close with any gesture, but dialogs can remain open unless dismissable with a tap on the barrier, but I'm not sure about this, it's just the behaviour I've see on apps like Google Play Store and Gmail, while other apps have alight modifications (like the steam app, where some popup menus on the screen remain open and attached to the button that originated it while allowing for scrolling of the whole content). In any case, the current implementation doesn't allow anything but the same behavior than dialogs, which is dismiss with a tap where it should, at least, provide a few options (like close any gesture or passthrough, maybe with options to remain open relative to screen or attached to the button that opened it). |
|
I didn't mean the current behavior is correct. The reason the change is reverted is that it introduced a different issue. We should find a find a fix that does not cause other regression. |



Fixes #90223
This PR adds dismiss callback to onTapCancel modal barrier.
Native
22-02-10-13-09-39.mp4
Flutter (with this PR)
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.