[Drawer] Support all types of navigation drawers in the Material Design Guidelines #6878

Zuurkern · 2017-05-16T14:46:43Z

has tests
has docs demos
is linted
Commit and PR titles begin with [ComponentName], and are in imperative form: "[Component] Fix leaky abstraction".
Description explains the issue / use-case resolved, and auto-closes the related issue(s) (http://tr.im/vFqem).

Description

The Material Design Guidelines specify different navigation drawer types/behaviors under Patterns. This PR aims to support all of those types, true to the spec, while retaining or hopefully expanding the flexibility of the Drawer component. It also adds demos to show how you can create all of these behaviors in your app, as some of these patterns require the Drawer, AppBar and a content element to work together in harmony.

Should close #4773 and possibly #4752 and #6451? (the latter two are for master though I think)

I apologize in advance for potentially bad JS code, I have only been coding in React (or JS for that matter) for a couple of months. But we have to start somewhere, right?

I also haven't written tests as I haven't figured out the testing tools yet :/ Sorry about that.

About changes to [Slide]

For the persistent navigation drawer demo to work, I had to modify Slide and give it a prop to transition its child element from outside its own position in the DOM instead of from outside the viewport. Without this, the transitions on the AppBar and content wouldn't sync up nicely with the Drawer's. In a real scenario where your app fills the entire viewport, you won't need this. Still, I think this opens up possibilities to get creative with the Drawer component (which passes this optional prop on to Slide). Also, I think it's a nice addition to Slide. I'm having a hard time coming up with a succinct and descriptive prop name for it though. Would love to discuss all of this further.

Hardcoding of the width transition for the mini variant

Currently, the width transition for the mini variant is hardcoded into the stylesheet. I was thinking maybe we could add support for width to the Collapse component and use that so it's more configurable. What do you think about this?

…next v1.0.0-alpha.13

…e/drawer-update

…e layout (overflow is on body instead of AppContent)

… box and dimensions instead of the viewport

…next" This reverts commit 41d08cf, reversing changes made to a1c248e.

mbrookes · 2017-05-29T11:47:05Z

src/transitions/Slide.js

+     * If `false`, let the child element slide in from outside the viewport.
+     * Set to `true` to let the child element slide in from outside its own
+     * bounding box and position.
+     */


We are using the pattern: "If true the child element slides in from outside its own bounding box and position"

Not sure how best to address the default case - perhaps follow the above with: "; by default, the child element slide in from outside the viewport."?

That makes sense! Good call on scrapping the "let"s too.

mbrookes · 2017-05-29T11:51:14Z

@Zuurkern Thanks for your hard work on this - it looks great! Please could you rebase - there have been some conflicting changes since.

@oliviertassinari any thoughts on the name for the useChildBounds prop in Slide, and @Zuurkern's question about adding support for width to Collapse?

Zuurkern · 2017-05-31T23:41:47Z

@mbrookes thanks for picking this back up and sorry for not following up on this sooner, I'm working on another, older project right now (and miss material-ui sooo bad) and I'm short on time.

I did some battle testing last week, trying to get creative with the Drawer to see how well it holds up flexibility wise. I made some small adjustments and took notes for things I would like to improve and/or discuss. I will try to make time this weekend to write those out and post here.

In its current state though, getting a basic layout down as described in the guidelines (drawer, app bar, content synergy) is really easy. You can basically just copy paste the demos. I tested the demos in latest Chrome, Firefox, Safari, Edge and IE11 (I think) as well. It just works.

I'll rebase too when I dive back into the code.

mbrookes · 2017-06-01T14:35:12Z

Fantastic, thanks for sticking with it - it'll be here when you get time. 👍

bai · 2017-06-22T04:17:33Z

Thanks for implementing this one! I'd be happy to fork & rebase — is that something I could help with to get this shipped?

Zuurkern · 2017-06-27T21:40:53Z

Hey guys, sorry about the radio silence. I was going to post my findings like a month ago but some things have changed in the meantime.

There have been some changes to the API in general and other components that Drawer is composed of:

classes prop, which got rid of paperClassName in @oliviertassinari 's latest iteration
Slide now has an offset prop - although not implemented yet, that's a great idea and gets rid of my awkwardly named useChildBounds and slideFromOutsideSelf props

paperClassName prop is gone now and I've tried to get rid of it as well because the mini variant implementation makes the stylesheet too specific (you have to use !important to do non-MD-spec stuff when trying to override CSS via this prop) - this was one of my gripes with my current implementation.

I still think we should add support for width to Collapse first (for the mini variant) and then revisit Drawer to make it leaner in the render function and be a good example of how to compose more complex components from other components (Paper, Slide, Collapse).

wtgtybhertgeghgtwtg · 2017-07-03T18:01:51Z

What about clipped drawers?

Zuurkern · 2017-07-03T18:22:40Z

Clipped drawers are just a matter of leaving the AppBar styling as is. It's not really something Drawer controls, except for maybe adding a top padding the same height as the AppBar.

I considered adding seperate demos for clipped drawers but decided not to because I just wanted some proof of concept for the changes I made to the Drawer component.

jaesung2061 · 2017-07-19T05:02:56Z

@Zuurkern what is the status on the PR?

acmitch · 2017-07-24T21:16:34Z

@mbrookes @Zuurkern @oliviertassinari When is this expected to be released? Our team is needing a mini-variant navigation bar and this looks extremely promising.

mbrookes · 2017-07-25T12:52:22Z

@acmitch I think everyone would welcome it if you or your team were to pick up this PR and continue the good work that @Zuurkern has started.

cr101 · 2017-07-30T03:53:17Z

It would be great to have a navigation drawer clipped under the AppBar so it doesn't cover the site brand and burger icon when open on mobile.

kgregory · 2017-07-31T18:26:15Z

@cr101 I'm not familiar with that material design standard for navigation drawer, can you share a link? I've seen similar behavior documented for permanent drawers.

wtgtybhertgeghgtwtg · 2017-07-31T18:29:47Z

Should it matter? Clipping and permanence are independent.

kgregory · 2017-07-31T18:34:19Z

@wtgtybhertgeghgtwtg is there a standard defined for that behavior?

cr101 · 2017-07-31T21:45:48Z

@kgregory This is the Material Design standard for navigation drawer

kgregory · 2017-07-31T21:51:35Z

@cr101 I'm familiar with this document, but unfortunately it doesn't support your request:

It would be great to have a navigation drawer clipped under the AppBar so it doesn't cover the site brand and burger icon when open on mobile.

The only thing that comes close is a permanent navigation drawer:

Types of permanent navigation drawers:
Navigation drawer clipped under the app bar: Apps focused on productivity that require balance across the screen

Or perhaps the mini-variant of the persistent navigation drawer:

Its resting state is as a mini-drawer at the same elevation as the content, clipped by the app bar. When expanded, it appears as the standard persistent navigation drawer.

However, it is not suitable for mobile:

Persistent navigation drawers are acceptable for all sizes larger than mobile.

The standard requires a temporary navigation drawer for mobile:

Temporary navigation drawers can toggle open or closed. Closed by default, the drawer opens temporarily above all other content until a section is selected.
Required for: Mobile

cr101 · 2017-07-31T22:43:52Z

@kgregory I'm after a permanent, clipped drawer like the example below:

Types of permanent navigation drawers:
Navigation drawer clipped under the app bar: Apps focused on productivity that require balance across the screen

kgregory · 2017-07-31T22:53:56Z

@cr101 right..

oliviertassinari · 2017-08-25T20:30:57Z

I'm going to resume this work as the oldest PR we have open, will move to the other after.

Zuurkern added 30 commits May 8, 2017 14:25

Merge tag 'v1.0.0-alpha.13' of github.com:callemall/material-ui into …

a1c248e

…next v1.0.0-alpha.13

reinstate bottom anchor in getSlideDirection

aa78cc0

improve comment for anchor propType

fdb562a

Merge branch 'next' of git.solarapps.be:solar/material-ui into featur…

52a03c5

…e/drawer-update

revert getSlideDirection code (left is now default by propTypes)

c0ca21e

add type prop

35ddc06

add classes and rendering logic for all types

fea0849

add comment to stylesheet explaining the chosen approach

ec3f601

docs: refactor AppDrawer and AppFrame to use new Drawer

ca29b68

remove position: relative from docked class as it breaks the docs sit…

163b8f8

…e layout (overflow is on body instead of AppContent)

add permanent navigation drawer demo to docs

22e41ea

move overflowX: hidden to Paper instead of root

cfc6de3

add persistent navigation drawer demo to docs

0c85a97

add mini navigation drawer demo to docs

253a3c2

clean up unnecessary code

e3a1841

restructure demos markup so AppBar comes first in the DOM

854f424

commit WIP for tomorrow at home

2127c0d

commit Slide and margin transitions approach for reference

7289f19

Slide: add support for sliding from outside an element's own bounding…

e70dd2c

… box and dimensions instead of the viewport

commit progress

474f9fc

invert new Slide prop, modify demos

257d555

improve comments in MiniDrawer

ad20988

set fixed height on drawer demo containers

0d5ce83

update api docs

184665c

fix wrong transitions

d0f9973

restructure Drawer render function

4faf5c2

add spec-compliant default width settings to Drawer and update demos

ab70d65

update code comments a bit

67f666f

Merge branch 'next' of github.com:callemall/material-ui into next

41d08cf

Revert "Merge branch 'next' of github.com:callemall/material-ui into …

c29f438

…next" This reverts commit 41d08cf, reversing changes made to a1c248e.

Sewdn mentioned this pull request May 17, 2017

[Drawer] Is there support for a pinned / stacked sidebar? #4752

Closed

muibot added the PR: out-of-date The pull request has merge conflicts and can't be merged label May 18, 2017

mbrookes reviewed May 29, 2017

View reviewed changes

oliviertassinari closed this Jun 27, 2017

oliviertassinari changed the base branch from next to v1-alpha June 27, 2017 20:37

oliviertassinari reopened this Jun 27, 2017

oliviertassinari changed the base branch from v1-alpha to v1-beta July 23, 2017 17:09

nathanmarks mentioned this pull request Jul 30, 2017

[Drawer] Port component #4773

Closed

5 tasks

oliviertassinari mentioned this pull request Aug 5, 2017

[Popover] Fixed bug where Popover renders relative to screen #7663

Merged

3 tasks

oliviertassinari mentioned this pull request Aug 26, 2017

[Drawer] New improvements #7925

Merged

3 tasks

oliviertassinari closed this in #7925 Aug 26, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Drawer] Support all types of navigation drawers in the Material Design Guidelines #6878

[Drawer] Support all types of navigation drawers in the Material Design Guidelines #6878

Zuurkern commented May 16, 2017 •

edited

Loading

mbrookes May 29, 2017

Zuurkern May 31, 2017

mbrookes commented May 29, 2017

Zuurkern commented May 31, 2017 •

edited

Loading

mbrookes commented Jun 1, 2017

bai commented Jun 22, 2017

Zuurkern commented Jun 27, 2017 •

edited

Loading

wtgtybhertgeghgtwtg commented Jul 3, 2017

Zuurkern commented Jul 3, 2017 •

edited

Loading

jaesung2061 commented Jul 19, 2017

acmitch commented Jul 24, 2017 •

edited

Loading

mbrookes commented Jul 25, 2017

cr101 commented Jul 30, 2017

kgregory commented Jul 31, 2017 •

edited

Loading

wtgtybhertgeghgtwtg commented Jul 31, 2017

kgregory commented Jul 31, 2017

cr101 commented Jul 31, 2017

kgregory commented Jul 31, 2017 •

edited

Loading

cr101 commented Jul 31, 2017 •

edited

Loading

kgregory commented Jul 31, 2017

oliviertassinari commented Aug 25, 2017

[Drawer] Support all types of navigation drawers in the Material Design Guidelines #6878

[Drawer] Support all types of navigation drawers in the Material Design Guidelines #6878

Conversation

Zuurkern commented May 16, 2017 • edited Loading

Description

About changes to [Slide]

Hardcoding of the width transition for the mini variant

mbrookes May 29, 2017

Choose a reason for hiding this comment

Zuurkern May 31, 2017

Choose a reason for hiding this comment

mbrookes commented May 29, 2017

Zuurkern commented May 31, 2017 • edited Loading

mbrookes commented Jun 1, 2017

bai commented Jun 22, 2017

Zuurkern commented Jun 27, 2017 • edited Loading

wtgtybhertgeghgtwtg commented Jul 3, 2017

Zuurkern commented Jul 3, 2017 • edited Loading

jaesung2061 commented Jul 19, 2017

acmitch commented Jul 24, 2017 • edited Loading

mbrookes commented Jul 25, 2017

cr101 commented Jul 30, 2017

kgregory commented Jul 31, 2017 • edited Loading

wtgtybhertgeghgtwtg commented Jul 31, 2017

kgregory commented Jul 31, 2017

cr101 commented Jul 31, 2017

kgregory commented Jul 31, 2017 • edited Loading

cr101 commented Jul 31, 2017 • edited Loading

kgregory commented Jul 31, 2017

oliviertassinari commented Aug 25, 2017

Zuurkern commented May 16, 2017 •

edited

Loading

Zuurkern commented May 31, 2017 •

edited

Loading

Zuurkern commented Jun 27, 2017 •

edited

Loading

Zuurkern commented Jul 3, 2017 •

edited

Loading

acmitch commented Jul 24, 2017 •

edited

Loading

kgregory commented Jul 31, 2017 •

edited

Loading

kgregory commented Jul 31, 2017 •

edited

Loading

cr101 commented Jul 31, 2017 •

edited

Loading