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

[SwipeableDrawer] Only listen on touchStart events of the swipe area for opening #11362

Closed
wants to merge 4 commits into from

Conversation

leMaik
Copy link
Member

@leMaik leMaik commented May 13, 2018

This PR does not only simplify the detection logic of opening swipes but also fixes a bug where you would swipe open (or discover) a drawer even if there are elements on top of it (e.g. Dialogs) because the open swipe used to listen to the body's events.

It requires re-testing on Safari, I don't have any 🍎 devices at hand.

@oliviertassinari oliviertassinari added this to the post v1.0.0 milestone May 13, 2018
changeTransition: false,
});
}
setImmediate(() => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Meh… I'll change it to setTimeout then.

@mbrookes
Copy link
Member

The current deploy preview is here: https://deploy-preview-11362--material-ui-ci.netlify.com

@mbrookes
Copy link
Member

mbrookes commented May 13, 2018

Seems fine to me (for what this PR intended to do).

The only issues I have are that the page behind the drawer scrolls as you swipe. This HN app which was my go-to before the scraper stopped working reliably) seems to have solved it by disabling scrolling while swiping.

(It also supports something resembling momentum swipes.)

@leMaik
Copy link
Member Author

leMaik commented May 13, 2018

@mbrookes Doesn't seem fine to me, though, after some more testing.

It flickers now, even with setTimeout/setImmediate.
All I want is that swiping only starts when the touch is from the swipeArea, I might achieve that by checking which element is at the touched position

Regarding momentum swipe: That's on my TODO list, but not in the near future.

@mbrookes
Copy link
Member

Hmm, yeah, now that I look at it again out of the bright 🌞 it does seem a bit juddery.

@leMaik
Copy link
Member Author

leMaik commented May 27, 2018

Swiping now works like a charm, but toggling open to open/close the drawer doesn't work anymore. 😞 I'll look into that soon…

@oliviertassinari
Copy link
Member

@leMaik I'm closing for now

@zannager zannager added package: styles Specific to @mui/styles. Legacy package, @material-ui/styled-engine is taking over in v5. component: SwipeableDrawer The React component. and removed package: styles Specific to @mui/styles. Legacy package, @material-ui/styled-engine is taking over in v5. labels Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: SwipeableDrawer The React component.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants