Material-ui: SwipeableDrawer get triggered when scrolling on a Dialog that appears on top of it

[Bouncue]

I have SwipeableDrawer open, then I have a Dialog open/appears on top of the SwipeableDrawer covering it.

When I scrolling on the Dialog horizontally it makes the SwipeableDrawer close.

I have tried setting zIndex of Dialog higher than the SwipeableDrawer, but still no help.

I don't know if this is a bug or something as expected. If not a bug, is it possible to add an API to disable swipe to close?

There are already two questions In stackoverflow asked for disableSwipeToClose that have yet answered.

• [x] The issue is present in the latest release.
• [x] I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

Scrolling horizontally on Dialog component which appears on top of SwipeableDrawer also trigger SwipeableDrawer to slide which often causes it to close

Expected Behavior 🤔

Scrolling on Dialog should not trigger SwipeableDrawer to slide

Steps to Reproduce 🕹

CodeSanbox demo: https://codesandbox.io/s/material-demo-eubn5?fontsize=14

Steps:

1. Open the demo in mobile view
2. Swipe from left to the right to open the drawer
3. Click Open Dialog button, a dialog will appear on top of drawer covering it
4. On the dialog, swipe from right to left will close the drawer under it

Context 🔦

I want the SwipeableDraewer still there when users close the Dialog.

Your Environment 🌎

| Tech | Version |
| ----------- | ------- |
| Material-UI | v4.4.2 |
| React | 16.8 |
| Browser | Brave |
| TypeScript | no |
| etc. | |

[oliviertassinari]

Interesting, it's basically another side of the same core issue. We have a few other manifestions of an unoptimal trigger of the drawer behavior: #16942, #16565 and #17190. We probably have enough reports now to consider a correct solution.

[oliviertassinari]

In this manifestation of the problem, we might be able to solve the problem by testing that the drawer contains the event.target that triggers the swipe event.

[Bouncue]

Thanks I'll try it

[oliviertassinari]

I propose the following solution:

diff --git a/packages/material-ui/src/SwipeableDrawer/SwipeableDrawer.js b/packages/material-ui/src/SwipeableDrawer/SwipeableDrawer.js
index 862aaf47e..422ff469f 100644
--- a/packages/material-ui/src/SwipeableDrawer/SwipeableDrawer.js
+++ b/packages/material-ui/src/SwipeableDrawer/SwipeableDrawer.js
@@ -302,6 +302,15 @@ const SwipeableDrawer = React.forwardRef(function SwipeableDrawer(props, ref) {

   const handleBodyTouchStart = React.useCallback(
     event => {
+      // At least one element clogs the drawer interaction zone.
+      if (
+        openRef.current &&
+        !backdropRef.current.contains(event.target) &&
+        !paperRef.current.contains(event.target)
+      ) {
+        return;
+      }
+
       // We are not supposed to handle this touch move.
       if (nodeThatClaimedTheSwipe !== null && nodeThatClaimedTheSwipe !== swipeInstance.current) {
         return;

Does anyone want to work on it? :)

We have a reproduction example in https://github.com/mui-org/material-ui/issues/16565#issuecomment-542105521 to try it out.

[oliviertassinari]

Regarding problem No 2: We should add the move listener to the paper only. That would solve the issue and also I just compared MUI's swipeable drawer with Android's native one:
MUI: swipes while swiping on the backdrop
Android: swipes only when touching the paper, not while swiping on the backdrop, but the swipe can start on the backdrop, though

@leMaik I confirm your observation, on Android, the drawer waits for the touch move to be over the paper to handle it. I think that it would be great to reproduce it. Regarding this issue, I imagine it wouldn't impact it as if we have two swipeable drawers, we would need, right from the touch start event, decide which of the two swipeable drawers to consider?

[leMaik]

@oliviertassinari The start event can stay as it is, we just need to change the move event. You only have one drawer per screen edge, so the "target drawer" can be determined right away.

[oliviertassinari]

Agree about handling it in the move event. I hope it will have a small overhead.

[leMaik]

Regarding the start event: We already use that one to determine what is being swiped: https://github.com/mui-org/material-ui/blob/master/packages/material-ui/src/SwipeableDrawer/SwipeableDrawer.js#L329

[oliviertassinari]

I propose to replace nodeThatClaimedTheSwipe in #16565 with event.muiHandled. Do we need to keep it?

[leMaik]

Yes, we need nodeThatClaimedTheSwipe, don't remove it! :scream: Otherwise, having multiple swipeable drawers won't work anymore (e.g. the demo in the docs).

Edit: Hm... Since we claim that in the touch start event, it might not be needed. Swiping a different edge would mean to start a new touch start event, so... Oh, that means that we _do_ need it.

[oliviertassinari]

Otherwise, having multiple swipeable drawers won't work anymore (e.g. the demo in the docs).

Could you confirm that https://github.com/mui-org/material-ui/issues/16565#issuecomment-544137595 doesn't introduce a regression of multiple swipeable drawers? I have tested it, but better be sure :).

[leMaik]

@oliviertassinari The current behavior is that only one swipeable drawer can be open at the same time. I don't see how that could be done without checking if a swipeable drawer is already open.

A git diff in a comment really isn't the best way to share code or evaluate possible solutions. I'm lazy, ~I won't copy that into my editor and merge it by hand to get your code to work.~ Could you point me to your branch next time? :wink:

---

Edit: There are actually no regressions. :tada: Saves some bytes and drops that lock. Nice!

[oliviertassinari]

Thanks for the confirmation! I'm sorry, I haven't created a branch. I have erased the change from my env.

[leMaik]

@oliviertassinari

I confirm your observation, on Android, the drawer waits for the touch move to be over the paper to handle it. I think that it would be great to reproduce it.

This is pretty easy to do. But it goes on: When leaving the paper while swiping, the start position gets set to the touch position if the direction isn't closing the menu. I'm still struggling with that. :sweat_smile: