Skip to content
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

[PopUpMenuButton] When the menu is open, scroll gesture doesn't dismiss the modal barrier #90223

Open
nosmirck opened this issue Sep 16, 2021 · 16 comments · Fixed by #98191
Open
Labels
a: fidelity Matching the OEM platforms better customer: samehere f: material design flutter/packages/flutter/material repository. f: scrolling Viewports, list views, slivers, etc. found in release: 2.5 Found to occur in 2.5 found in release: 2.6 Found to occur in 2.6 framework flutter/packages/flutter repository. See also f: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on P2 Important issues not at the top of the work list team-design Owned by Design Languages team triaged-design Triaged by Design Languages team
Projects

Comments

@nosmirck
Copy link

nosmirck commented Sep 16, 2021

Steps to Reproduce

This is the minimal code required to reproduce it, open the popup menu from the AppBar and attempt to scroll the ListView behind it. Only possible actions are either tap anywhere outside the popup menu to dismiss it or select an item. Scrolling the background ListView should be possible. Optionally, we should be able to set the behaviour for this, we could have this default behavior (only possible actions are to tap to close or select item), close on any action (not just tap, any gesture, like drag) or pass through the gesture (let the gesture be passed down if not captured by the pop-up menu).

import 'package:flutter/material.dart';

void main() => runApp(MyApp());

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      debugShowCheckedModeBanner: false,
      theme: ThemeData(
        primarySwatch: Colors.blue,
      ),
      home: const MyHomePage(),
    );
  }
}

class MyHomePage extends StatefulWidget {
  const MyHomePage({
    Key? key,
  }) : super(key: key);

  @override
  _MyHomePageState createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: const Text('Flutter Example'),
        actions: [
          PopupMenuButton<int>(
            icon: const Icon(Icons.view_list_rounded),
            onSelected: (_) {},
            itemBuilder: (context) => List.generate(
              100,
              (i) => PopupMenuItem(
                child: Text('Pop Up Menu Item $i'),
                value: i,
              ),
            ),
          ),
        ],
      ),
      body: ListView(
        children: List.generate(
          100,
          (i) => ListTile(
            title: Text('Sample item $i'),
          ),
        ),
      ),
    );
  }
}
Flutter Doctor
[√] Flutter (Channel stable, 2.5.0, on Microsoft Windows [Version 10.0.19043.1237],
    locale en-CA)
    • Flutter version 2.5.0 at C:\flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 4cc385b4b8 (9 days ago), 2021-09-07 23:01:49 -0700        
    • Engine revision f0826da7ef
    • Dart version 2.14.0

[√] Android toolchain - develop for Android devices (Android SDK version 30.0.0-rc2)
    • Android SDK at D:\Android
    • Platform android-30, build-tools 30.0.0-rc2
    • ANDROID_HOME = D:\Android
    • Java binary at: D:\Program Files\Android\Android Studio\jre\bin\java
    • Java version OpenJDK Runtime Environment (build 1.8.0_242-release-1644-b01)    
    • All Android licenses accepted.

[√] Chrome - develop for the web
    • Chrome at C:\Program Files (x86)\Google\Chrome\Application\chrome.exe

[√] Visual Studio - develop for Windows (Visual Studio Professional 2019 16.6.2)
    • Visual Studio at C:\Program Files (x86)\Microsoft Visual
      Studio\2019\Professional
    • Visual Studio Professional 2019 version 16.6.30204.135
    • Windows 10 SDK version 10.0.18362.0

[√] Android Studio (version 4.0)
    • Android Studio at D:\Program Files\Android\Android Studio
    • Flutter plugin version 49.0.2
    • Dart plugin version 193.7547
    • Java version OpenJDK Runtime Environment (build 1.8.0_242-release-1644-b01)    

[√] VS Code (version 1.60.1)
    • VS Code at C:\Users\nosmi\AppData\Local\Programs\Microsoft VS Code
    • Flutter extension version 3.26.0

[√] Connected device (3 available)
    • Windows (desktop) • windows • windows-x64    • Microsoft Windows [Version      
      10.0.19043.1237]
    • Chrome (web)      • chrome  • web-javascript • Google Chrome 93.0.4577.82      
    • Edge (web)        • edge    • web-javascript • Microsoft Edge 93.0.961.47      

• No issues found!
@darshankawar darshankawar added the in triage Presently being triaged by the triage team label Sep 17, 2021
@darshankawar
Copy link
Member

@nosmirck
Thanks for the report. Wondering if it's possible to check how's the native behavior of this case is ? Does it allow to scroll when in background ?

@darshankawar darshankawar added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Sep 17, 2021
@nosmirck
Copy link
Author

nosmirck commented Sep 17, 2021

@darshankawar I was looking at some apps and this is what I've found:

Mobile:

  • it's hard to find diversity, all native apps I've tested close the menu with any gesture outside of it (taps, drags) it looks like as soon as tap down starts. Example: in Google PlayStore, on any app details page, open the pop-up menu from the top right, try to scroll, it will close the menu.
  • Only apps that are using some kind of WebView to display the content (I tested the Steam app) let you scroll with a pop up menu open, and it sticks to the button that opened it when scrolling. Example, open a game page on steam mobile, right below the game's videos and screenshots you have buttons to add to favorites, ignore, share, etc. There's a button with a down arrow that opens a menu, this menu closes when you tap outside of it but scrolls up and down with the button if you scroll the page.

Web/Desktop:

  • Right here on GitHub, open any source file and on the top right of the source code, there's a pop-up menu where you can select "view raw" or "view blame". On mobile and desktop it scrolls with the screen.
  • Desktop apps like Microsoft Word let you scroll the main content even when a pop-up menu is open (like scrolling pages with font size selection menu open).

At first, we'd like to at least have consistency with native mobile (close the menu with tap down instead of tap) with the option to pass through the gesture.

@github-actions github-actions bot removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Sep 17, 2021
@darshankawar
Copy link
Member

Thanks for the detailed analysis. Although I see the same behavior on latest stable and master, I am a bit unsure if this is a limitation or a bug.

90223.mov

I am going ahead and keeping it open for further insights from the team.

stable, master flutter doctor -v
[✓] Flutter (Channel stable, 2.5.1, on Mac OS X 10.15.4 19E2269 darwin-x64,
    locale en-GB)
    • Flutter version 2.5.1 at /Users/dhs/documents/fluttersdk/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision ffb2ecea52 (2 days ago), 2021-09-17 15:26:33 -0400
    • Engine revision b3af521a05
    • Dart version 2.14.2

[✓] Android toolchain - develop for Android devices (Android SDK version 30)
    • Android SDK at /Users/dhs/Library/Android/sdk
    • Platform android-30, build-tools 30.0.3
    • ANDROID_HOME = /Users/dhs/Library/Android/sdk
    • Java binary at: /Users/dhs/Library/Application Support/JetBrains/Toolbox/apps/AndroidStudio/ch-0/202.7486908/Android
      Studio.app/Contents/jre/jdk/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 1.8.0_242-release-1644-b3-6915495)
    • All Android licenses accepted.    

[✓] Xcode - develop for iOS and macOS
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Xcode 12.3, Build version 12C33
    • CocoaPods version 1.10.1

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 4.1)
    • Android Studio at /Users/dhs/Library/Application Support/JetBrains/Toolbox/apps/AndroidStudio/ch-0/202.7486908/Android
      Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build
      1.8.0_242-release-1644-b3-6915495)        

[✓] VS Code (version 1.57.1)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.21.0

[✓] Connected device (4 available)
    • iPhone 12 Pro Max (mobile)                     •
      A5473606-0213-4FD8-BA16-553433949729 • ios            •
      com.apple.CoreSimulator.SimRuntime.iOS-14-3 (simulator)
    • iPad Pro (12.9-inch) (4th generation) (mobile) •
      AD9F1B85-B622-485E-97E8-E4AC866D5770 • ios            •
      com.apple.CoreSimulator.SimRuntime.iOS-14-3 (simulator)
    • macOS (desktop)                                • macos
      • darwin-x64     • Mac OS X 10.15.4 19E2269 darwin-x64
    • Chrome (web)                                   • chrome
      • web-javascript • Google Chrome 92.0.4515.159


• No issues found!

[✓] Flutter (Channel master, 2.6.0-6.0.pre.147, on Mac OS X 10.15.4 19E2269
    darwin-x64, locale en-GB)
    • Flutter version 2.6.0-6.0.pre.147 at
      /Users/dhs/documents/fluttersdk/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 83dfb2237a (2 days ago), 2021-09-17 23:48:04 -0400
    • Engine revision 31792e0340
    • Dart version 2.15.0 (build 2.15.0-120.0.dev)

[✓] Android toolchain - develop for Android devices (Android SDK version 30)
    • Android SDK at /Users/dhs/Library/Android/sdk
    • Platform android-30, build-tools 30.0.3
    • ANDROID_HOME = /Users/dhs/Library/Android/sdk
    • Java binary at: /Users/dhs/Library/Application Support/JetBrains/Toolbox/apps/AndroidStudio/ch-0/202.7486908/Android
      Studio.app/Contents/jre/jdk/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 1.8.0_242-release-1644-b3-6915495)
    • All Android licenses accepted.    

[✓] Xcode - develop for iOS and macOS
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Xcode 12.5.1, Build version 12E507
    • CocoaPods version 1.10.1

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 4.1)
    • Android Studio at /Users/dhs/Library/Application Support/JetBrains/Toolbox/apps/AndroidStudio/ch-0/202.7486908/Android
      Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build
      1.8.0_242-release-1644-b3-6915495)        

[✓] VS Code (version 1.57.1)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.21.0

[✓] Connected device (4 available)
    • iPhone 12 Pro Max (mobile)                     •
      A5473606-0213-4FD8-BA16-553433949729 • ios            •
      com.apple.CoreSimulator.SimRuntime.iOS-14-3 (simulator)
    • iPad Pro (12.9-inch) (4th generation) (mobile) •
      AD9F1B85-B622-485E-97E8-E4AC866D5770 • ios            •
      com.apple.CoreSimulator.SimRuntime.iOS-14-3 (simulator)
    • macOS (desktop)                                • macos
      • darwin-x64     • Mac OS X 10.15.4 19E2269 darwin-x64
    • Chrome (web)                                   • chrome
      • web-javascript • Google Chrome 92.0.4515.159


• No issues found!




@darshankawar darshankawar added f: material design flutter/packages/flutter/material repository. f: scrolling Viewports, list views, slivers, etc. found in release: 2.5 Found to occur in 2.5 found in release: 2.6 Found to occur in 2.6 framework flutter/packages/flutter repository. See also f: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on and removed in triage Presently being triaged by the triage team labels Sep 20, 2021
@nosmirck
Copy link
Author

nosmirck commented Sep 20, 2021

So, digging a bit I found in the source of PopupMenu that it uses a ModalBarrier with _AnyTapGestureRecognizer

The comment above this Gesture Recognizer reads as follows:

// Recognizes tap down by any pointer button.
//
// It is similar to [TapGestureRecognizer.onTapDown], but accepts any single
// button, which means the gesture also takes parts in gesture arenas.

That description would suggest that it would accept any gesture, however, it only accept tapUps that are not a cancel. A cancel happens when the gesture is a drag, which makes sense for inputs that accept taps (we see this on "native" implementations of similar popups for desktop and mobile as well as any web framework).

This, at least for mobile, seems like a bug. The _AnyTapGestureRecognizer should also override the cancelTap and trigger the callback, however, it wouldn't happen until the gesture has finished (end drag with tap up).

It could also happen that for some reason the gesture recognizer is not calling tapUp when detects a tapCancel.

Ultimately, the gesture, at least to mimic a native mobile popupMenu should be tapDown, to close as soon as a tap is detected. This would fix the issue on mobile and make it behave exactly as native.

For Web and Desktop, the default should be a passTrough of the gesture.

A good solution I propose would be to add some kind of enum property to the popupMenu to decide the behaviour for the popupMenu's modal barrier: closeOnAnyTap, passThrough. (maybe more?) and use by default passThrough on Web/Desktop and closeOnAnyTap for mobile.

@TahaTesser
Copy link
Member

TahaTesser commented Feb 10, 2022

Ths should be split into two issues

  1. The original issue when scroll gesture or drag on the modal barrier doesn't dismiss the modal barrier (only tap does)
    This doesn't match material design behaviour (@nosmirck Thanks for investigation on this)
    I have a PR ready - Dismiss Modal Barrier on handleTapCancel  #98191.
  2. Proposal for pass through gesture. This should be a separate issue not a bug but current design. As this is not related to the issue above, mainly for targeting desktop/web.

@TahaTesser TahaTesser added this to To do in Nevercode via automation Feb 10, 2022
@TahaTesser TahaTesser moved this from To do to In progress in Nevercode Feb 10, 2022
@TahaTesser
Copy link
Member

@nosmirck
Would you please file a new issue for the proposal? Thanks

@TahaTesser TahaTesser moved this from In progress to PR submitted in Nevercode Feb 10, 2022
@TahaTesser TahaTesser changed the title [PopUpMenuButton] When the menu is open, it prevents scrolling on background scroll widgets [PopUpMenuButton] When the menu is open, scroll gesture doesn't dismiss the modal barrier Feb 10, 2022
@TahaTesser TahaTesser added the a: fidelity Matching the OEM platforms better label Feb 10, 2022
@nosmirck
Copy link
Author

@nosmirck
Would you please file a new issue for the proposal? Thanks

Sure thing! Should I tag you there?

@TahaTesser
Copy link
Member

TahaTesser commented Feb 10, 2022

Sure thing! Should I tag you there?

You can simply file the proposal and mention this issue for context, rest should be taken care of :)

Nevercode automation moved this from PR submitted to Done (PR merged) Feb 16, 2022
@darshankawar darshankawar added the r: fixed Issue is closed as already fixed in a newer version label Feb 17, 2022
@github-actions
Copy link

github-actions bot commented Mar 3, 2022

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 3, 2022
@chunhtai chunhtai reopened this Mar 25, 2022
Nevercode automation moved this from Done (PR merged) to In progress Mar 25, 2022
@chunhtai
Copy link
Contributor

chunhtai commented Mar 25, 2022

cc @TahaTesser

This change causes a regression.
repro:
https://gist.github.com/chunhtai/db68a074f63f96fe2b997597599ebbbb

tap show dialog
tap and hold one finger on modal barrier without lifting it.
use other finger to tap ok
expect: alert dialog is dismissed and page stays at /abc
actual: alert dialog is dismissed, page /abc is popped

preparing revert

@chunhtai
Copy link
Contributor

This is caused by modal barrier will cancel any ongoing gesture when it is disposed by clicking ok, which result in another pop is called. I think the on cancel need to be smart about when it should send the pop vs not.

@darshankawar darshankawar removed the r: fixed Issue is closed as already fixed in a newer version label Mar 28, 2022
@TahaTesser TahaTesser self-assigned this Mar 28, 2022
@TahaTesser TahaTesser moved this from In progress to PR submitted in Nevercode Jul 14, 2022
@TahaTesser
Copy link
Member

TahaTesser commented Jul 14, 2022

submitted #107284

Closed.

@gspencergoog
Copy link
Contributor

FYI, we had a report that this is not fixed when you scroll with the scroll wheel:

"[this issue] is not fixed for scroll-wheel scrolling. If a pop-up menu is open (anywhere in the application), scrolling the scroll-wheel doesn't doesn't do anything until the pop-up menu is closed."

@Piinks Piinks added the P2 Important issues not at the top of the work list label Mar 17, 2023
@flutter-triage-bot flutter-triage-bot bot unlocked this conversation Jun 16, 2023
@flutter-triage-bot flutter-triage-bot bot added multiteam-retriage-candidate team-design Owned by Design Languages team triaged-design Triaged by Design Languages team labels Jul 8, 2023
@Daydreamer-riri
Copy link

Hi, is there any progress on this? I found this problem in the flutter app I use, and I thought it was weird that PopUpMenu would block page scrolling.

@Reza-Babakhani
Copy link

Reza-Babakhani commented Nov 4, 2023

After 2 years, this bug has not been fixed yet.

@ledjoncili
Copy link

FYI, we had a report that this is not fixed when you scroll with the scroll wheel:

"[this issue] is not fixed for scroll-wheel scrolling. If a pop-up menu is open (anywhere in the application), scrolling the scroll-wheel doesn't doesn't do anything until the pop-up menu is closed."

Same issue here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: fidelity Matching the OEM platforms better customer: samehere f: material design flutter/packages/flutter/material repository. f: scrolling Viewports, list views, slivers, etc. found in release: 2.5 Found to occur in 2.5 found in release: 2.6 Found to occur in 2.6 framework flutter/packages/flutter repository. See also f: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on P2 Important issues not at the top of the work list team-design Owned by Design Languages team triaged-design Triaged by Design Languages team
Projects
No open projects
Nevercode
  
PR submitted