mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.27k stars 32.12k forks source link

[Drawer] Support gestures #8127

Closed emidelgo closed 6 years ago

emidelgo commented 7 years ago

As asserted in the subject, the drawer in v1 appears to not be able anymore to interact with gestures emulating the native Android App drawer, as opening and closing by swipes...

Is it a lack of docs or is a missed feature port?

Regards

oliviertassinari commented 7 years ago

We haven't implemented the feature in order to simplify the implementation. I agree it would be better if we support it. But I don't think that it's a priority. On mobile, I personally find myself not using the feature as swiping is requiring me more effort than a tap.

emidelgo commented 7 years ago

I think you're right on small screens. Generally the drawer toggle button is placed on top left corner. On large touch screen devices (> 4.7") is really more comfortable to swipe than to get the toggle by thumb.

tomscholz commented 6 years ago

Hey, I just came across this issue and wanted to say that I think that it is a great idea to bring back gestures, but we should also add a note to the docs, that the material design guidelines state not to use drag-out drawers on the web. See the sections about gestures for reference: https://material.io/guidelines/platforms/platform-adaptation.html#platform-adaptation-platform-recommendations

oliviertassinari commented 6 years ago

Browsers often use edge gestures for their feature set. Edge swipes should not be used on the web. Edge swipes can not be relied on across browsers.

@tomscholz This is an excellent reference. So this issue is definitely not a priority. Still, I think that it can always be interesting to have this feature under a flag (disabled by default) so people writing web-view inside apps can take advantage of it. Or even for people having iOS (disabled) / Android (enabled) detection logic. Of course at the condition that the implementation doesn't harm the 80% use cases.

emidelgo commented 6 years ago

Hey, yes, of course the drawer gesture is useful when in standalone mode (the user add the website to the home screen), or for webapps. And I think these are consistent use cases for a React/materialUI project. Of course I agree with you this isn't a urgent feature, but it was a really useful feature in older versions. IMHO.

cherniavskii commented 6 years ago

@tomscholz, @oliviertassinari Despite guidelines, Google implements gestures in Maps' web version

verekia commented 6 years ago

From my understanding, the point that the material design guidelines is really making is that relying solely on swipe to open a menu is a design flaw because of those edge gestures that some browsers support. If there was no other way to open the drawer (like in their screenshot), then it would be impossible to open the drawer.

I believe that if there is a button (like the AppBar's burger for instance), it completely solves that issue:

Either way, the closing gesture (as seen in the gif above) does not conflict with those guidelines.

Now as a personal note here: I must say that this swipe gesture to open the drawer is the only thing that I really miss in v1. It was in my opinion a killer feature of Material UI and I really hope it comes back :) Thanks for all your work either way.

naman03malhotra commented 6 years ago

@emidelgo I have used https://github.com/leonaves/react-touch to enable gesture support for the drawer. @oliviertassinari can we use this natively in Material UI to enable this feature.

leMaik commented 6 years ago

I'd really love to have this feature in our Material-UI-based mobile webapp. :+1: So I looked into how this is implemented in v0.x and how it could be done in v1.0

Unfortunately, in v1, the temporary Drawer uses a Modal (which is fine) with a Slide transition for the drawer. The transition doesn't support being half open, so opening the drawer partially while touching wouldn't work. Also, the specs suggest opening the drawer for some pixels when touching the edge of the screen, which wouldn't be possible either.

Now the question @oliviertassinari: Do you have any idea on how we could make Slide (or any Transition, actually) support intermediate states beside in={true} and in={false} :thinking: Maybe by making in a number between 0 and 1? Edit: This is probably a baaaad idea, since this would require modifying react-transition-group. Let's not do that. :sweat_smile:

Edit: Maybe I could also set in to true when the drawer is semi-opened (while touching) and manually adjust its position. I'll try this out.

oliviertassinari commented 6 years ago

with a Slide transition for the drawer.

@leMaik If I had to work on such issue. I would start by investigating how we can implement an enhanced version of the Slide component with touch support.

Effectively, this would require changing the API of the component. For performance concerns, I would keep as much state logic into this enhanced slider. You don't want to rerender the whole drawer 60 times per second. two-way data binding is the way to go. It's who I have been implemented react-swipeable-views. So, I would keep in as a boolean. I'm encouraging anyone interesting in this issue to have a look at the API & implementation. It has proven to work well, for instance, it's used by mastodon on low-end devices with a good FPS rate.

However, I would add a onOpen and onClose callback properties so users can keep full control on the state of the enhanced slider. I hope that help.

I'm adding the issue to the Future milestone as we should be able to release v1 without the feature.

leMaik commented 6 years ago

@oliviertassinari The thing is that tweaking Slide alone still wouldn't be able to make this spec-compliant. I just tried out the approach from my edit and it seems to work fine. I used the (kind-of-ugly) approach from v0.x where style.transform is set manually so that the drawer doesn't need to be re-rendered by react whenever it moves.

oliviertassinari commented 6 years ago

kind-of-ugly

@leMaik No, it's 👌. I don't understand the specification argument. What's the blocker?

leMaik commented 6 years ago

There's no blocker. :wink: I'm currently experimenting, it almost works for me.

Spec argument: When touching the left … maybe 30 px of the screen, the left drawer should slide in a bit to make discovery easier. How would that be possible with the touch-enabled Slide?

leMaik commented 6 years ago

@oliviertassinari With the Slide approach, the Slide component would need to be modified, but I have no idea how that should be done. Also, the Modal would need to be updated to change the overlay opacity while swiping. I feel like those new components just block me from implementing the ~animation~ swiping. :disappointed: On the other hand, I like how the drawer's code is easy to understand by using them.

oliviertassinari commented 6 years ago

I like how the drawer's code is easy to understand by using them.

@leMaik We need to use the Modal for accessibility concerns, it's not just about simplicity. I think that we can fix the Modal opacity problem by moving the logic up, from the Slide to the Drawer.

leMaik commented 6 years ago

@oliviertassinari Modal opacity now works fine after moving that logic partially into the Drawer. Turns out that I just needed to do what v0.x did, i.e. setting the opacity of the backdrop according to the open-percentage of the drawer.

leMaik commented 6 years ago

Just a small update on this:

mbrookes commented 6 years ago

@leMaik Sorry if this as covered in the discussion, but will the gesture support be available for potential use in other components or by the user, or is it tightly coupled to the Drawer?

What would anchor='top' and anchor='bottom' support do?

leMaik commented 6 years ago

@mbrookes It's tightly covered to the Drawer, as it interacts with its backdrop and position. It would be pretty hard to make it universal while maintaining the native-like integration with the drawer. I tried to start a touch-enabled Slide, but after trying, I gave up and started to just solve the problem at hand.

What would anchor='top' and anchor='bottom' support do?

This would allow to swipe a drawer from top/bottom if it has anchor='top' or bottom

oliviertassinari commented 6 years ago

@leMaik Great! Also, be carefull with the bundle size implication. I have noticed that Google, on his search page for mobile, has a drawer that doesn't respond to touch move events. I can think of three different reasons:

Maybe I'm missing something else. I'm just trying to understand why.

mbrookes commented 6 years ago

This would allow to swipe a drawer from top/bottom if it has anchor='top' or bottom

I don't think that's needed then.

oliviertassinari commented 6 years ago

On the other hand, Google Map Web drawer responds to touch events :).

oliviertassinari commented 6 years ago

What would anchor='top' and anchor='bottom' support do?

The Material specification has a component that need this feature. Let me find it. This one https://material.io/guidelines/components/bottom-sheets.html

leMaik commented 6 years ago

@oliviertassinari This would mean that bottom sheets could be implemented using <Drawer anchor='bottom'>. Awesome! :tada:

Performance on my (pretty) old Nexus 4 is great and performance on 6-times-slowed-down "low end" Chrome debugger is also fine. Regarding the bundle size implication: Maybe that matters at Google-scale, but it won't be that big.

On the other hand, Google Map Web drawer responds to touch events :).

Especially when Google started to adapt the Material Design patterns, I got a feeling that the various teams don't talk that much to each other. :sweat_smile: Seems to have changed, though, meanwhile at least the apps are consistent (except for Google Now, which just looks disgusting after the update that brought rounded corners to Android :sob:).