fix(drawer): allow drawer below top app bar #4028

moog16 · 2018-10-31T20:19:09Z

related material-components/material-components-web-react#377

Add screenshot tests for dismissible and permanent drawers to appear below top app bar
removed top css property from dismissible drawer class

abhiomkar · 2018-10-31T20:51:31Z

packages/mdc-drawer/README.md

@@ -160,6 +160,62 @@ Drawers can contain a header element which will not scroll with the rest of the
 </aside>
 ```

+### Drawer Below Top App Bar


Can we remove this section and keep the one in Usage with Top App Bar section?

This is the permanent below top app bar...it is slightly different. Do you still wanna remove it?

Let's keep just one example for Dismissible drawer variant to avoid maintaining two different variations of this version.

abhiomkar · 2018-10-31T22:00:21Z

test/screenshot/golden.json

@@ -556,6 +556,14 @@
      "desktop_windows_ie@11": "https://storage.googleapis.com/mdc-web-screenshot-tests/advorak/2018/09/21/16_50_45_748/spec/mdc-dialog/mixins/title-ink-color.html.windows_ie_11.png"
    }
  },
+  "spec/mdc-drawer/classes/dismissible-below-top-app-bar.html": {
+    "public_url": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/10/31/20_39_25_363/spec/mdc-drawer/classes/dismissible-below-top-app-bar.html?utm_source=golden_json",


Seems like the main content is not scrollable in this demo. Also the drawer is being clipped at the bottom when I resize the browser small viewport.

We need to update the docs too to layout main content and drawer correctly.

abhiomkar · 2018-10-31T22:01:50Z

packages/mdc-drawer/README.md

@@ -160,6 +160,62 @@ Drawers can contain a header element which will not scroll with the rest of the
 </aside>
 ```

+### Drawer Below Top App Bar


Let's keep just one example for Dismissible drawer variant to avoid maintaining two different variations of this version.

abhiomkar · 2018-10-31T22:02:37Z

packages/mdc-drawer/README.md

@@ -263,7 +365,6 @@ body {
 .app-bar {
  position: absolute;
 }
-```


Revert this line.

I don't see this change updated. This breaks the formatting.

abhiomkar · 2018-10-31T22:07:12Z

packages/mdc-drawer/README.md

+```scss
+// Note: these styles do not account for any paddings/margins that you may need.
+
+.below-top-app-bar-container {


I'm sure we need more additional styles than these to get correct layout for this variant. The z-index of app-bar need to be updated to stay on top of drawer. --fixed-adjust needs to be applied to drawer root and app-content individually. Also the main content should be scrollable.

Please take a look at this example:
https://stackblitz.com/edit/mdc-drawer-demo-fiqbfg?file=index.scss

Not sure what you mean. It looks like both areas independently scroll from each other.

abhiomkar · 2018-10-31T22:07:54Z

test/screenshot/golden.json

@@ -556,6 +556,14 @@
      "desktop_windows_ie@11": "https://storage.googleapis.com/mdc-web-screenshot-tests/advorak/2018/09/21/16_50_45_748/spec/mdc-dialog/mixins/title-ink-color.html.windows_ie_11.png"
    }
  },
+  "spec/mdc-drawer/classes/dismissible-below-top-app-bar.html": {


Thanks for adding screenshot test 🎉

abhiomkar · 2018-11-05T18:42:49Z

packages/mdc-drawer/README.md

-The CSS to match it looks like this:
+##### Dismissible Drawer Below Top App Bar
+
+In cases where the drawer appears below the top app bar you will want to follow the markup shown below. The `mdc-drawer__content` and `main-content` elements will also scroll independently of each other. The `mdc-top-app-bar` and `mdc-top-app-bar--fixed-adjust` will be sit sibling to each other. The `mdc-top-app-bar--fixed-adjust` element will contain the `mdc-drawer--dismissible` and `mdc-drawer-app-content` elements.


"The mdc-top-app-bar, mdc-drawer and mdc-drawer-app-content will be sibling to each other. The mdc-top-app-bar--fixed-adjust helper class will be applied to mdc-drawer and mdc-drawer-app-content elements to adjust the position with top app bar."

abhiomkar · 2018-11-05T18:43:43Z

packages/mdc-drawer/README.md

@@ -263,7 +365,6 @@ body {
 .app-bar {
  position: absolute;
 }
-```


I don't see this change updated. This breaks the formatting.

abhiomkar · 2018-11-05T18:45:35Z

test/screenshot/spec/mdc-drawer/classes/permanant-below-top-app-bar.html

@@ -0,0 +1,233 @@
+<!DOCTYPE html>


We might not need to screenshot test this. Dismissible & permanent drawers have exact same styles except the JS logic.

true - i'll remove

abhiomkar

LGTM.

Please update golden.json file.

abhiomkar · 2018-11-06T18:39:46Z

test/screenshot/golden.json

@@ -574,6 +582,14 @@
      "desktop_windows_ie@11": "https://storage.googleapis.com/mdc-web-screenshot-tests/advorak/2018/09/26/06_08_27_877/spec/mdc-drawer/classes/modal.html.windows_ie_11.png"
    }
  },
+  "spec/mdc-drawer/classes/permanant-below-top-app-bar.html": {


Since you've removed this from screenshot tests, remove these from golden.json too.

abhiomkar · 2018-11-06T18:40:31Z

test/screenshot/golden.json

@@ -556,6 +556,14 @@
      "desktop_windows_ie@11": "https://storage.googleapis.com/mdc-web-screenshot-tests/advorak/2018/09/21/16_50_45_748/spec/mdc-dialog/mixins/title-ink-color.html.windows_ie_11.png"
    }
  },
+  "spec/mdc-drawer/classes/dismissible-below-top-app-bar.html": {
+    "public_url": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/10/31/20_39_25_363/spec/mdc-drawer/classes/dismissible-below-top-app-bar.html?utm_source=golden_json",


Update golden.json with latest changes.

…mponents/material-components-web into fix/drawer/below-top-app-bar

…mponents/material-components-web into fix/drawer/below-top-app-bart

mdc-web-bot · 2018-11-07T19:45:46Z

🤖 Beep boop!

Screenshot test report 🚦

6 screenshots changed from master on commit 92d7a5e:

https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2018/11/07/19_43_10_261/report/report.html?utm_source=github&utm_medium=pr_comment

Details

3 Changed:

spec/mdc-dialog/classes/baseline-alert-above-drawer.html

3 Added:

spec/mdc-drawer/classes/dismissible-below-top-app-bar.html

(cherry picked from commit ebdb084)

fix(drawer): allow drawer below top app bar

0589754

googlebot added the cla: yes label Oct 31, 2018

docs: update readme with examples

6d6e4ba

kfranqueiro requested a review from abhiomkar October 31, 2018 20:48

abhiomkar reviewed Oct 31, 2018

View reviewed changes

fix: screenshots

21f0287

abhiomkar requested changes Oct 31, 2018

View reviewed changes

Matt Goo added 2 commits October 31, 2018 15:43

docs: update

bb1a730

fix: top app bar above drawer dismissible

2e6f765

moog16 mentioned this pull request Oct 31, 2018

fix(drawer): update readme and screenshots to support below top app bar material-components/material-components-web-react#397

Merged

abhiomkar requested changes Nov 5, 2018

View reviewed changes

Matt Goo added 2 commits November 5, 2018 13:53

Update README.md

dac44bd

WIP: fix description to docs

d75ba04

abhiomkar requested changes Nov 6, 2018

View reviewed changes

Matt Goo added 4 commits November 6, 2018 10:45

Update golden.json

cab30cf

Merge branch 'master' into fix/drawer/below-top-app-bar

31b2562

WIP: golden update

efebdd8

Merge branch 'fix/drawer/below-top-app-bar' of github.com:material-co…

316b838

…mponents/material-components-web into fix/drawer/below-top-app-bar

abhiomkar approved these changes Nov 7, 2018

View reviewed changes

abhiomkar and others added 5 commits November 7, 2018 13:05

Merge branch 'master' into fix/drawer/below-top-app-bar

0dfb150

fix: update dialog screenshot using drawer

384d0f0

Merge branch 'fix/drawer/below-top-app-bar' of github.com:material-co…

e240e79

…mponents/material-components-web into fix/drawer/below-top-app-bart

fix: lint

e9f04dc

fix: screenshot

92d7a5e

moog16 merged commit ebdb084 into master Nov 7, 2018

moog16 deleted the fix/drawer/below-top-app-bar branch November 7, 2018 22:53

abhiomkar mentioned this pull request Nov 13, 2018

MDC Drawer: Document clipped variant and update demos #3456

Closed

trimox mentioned this pull request Nov 17, 2018

Upgrade to material-components-web v0.41.1 trimox/angular-mdc-web#1542

Closed

adrianschmidt pushed a commit to Lundalogik/material-components-web that referenced this pull request Jan 4, 2019

fix(drawer): allow drawer below top app bar (material-components#4028)

1eff602

(cherry picked from commit ebdb084)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(drawer): allow drawer below top app bar #4028

fix(drawer): allow drawer below top app bar #4028

moog16 commented Oct 31, 2018

abhiomkar Oct 31, 2018

moog16 Oct 31, 2018 •

edited

Loading

abhiomkar Oct 31, 2018

abhiomkar Oct 31, 2018

abhiomkar Oct 31, 2018

abhiomkar Oct 31, 2018

abhiomkar Nov 5, 2018

abhiomkar Oct 31, 2018

moog16 Oct 31, 2018

abhiomkar Oct 31, 2018

abhiomkar Nov 5, 2018

abhiomkar Nov 5, 2018

abhiomkar Nov 5, 2018

moog16 Nov 5, 2018

abhiomkar left a comment

abhiomkar Nov 6, 2018

abhiomkar Nov 6, 2018

mdc-web-bot commented Nov 7, 2018

3 Changed:

3 Added:

fix(drawer): allow drawer below top app bar #4028

fix(drawer): allow drawer below top app bar #4028

Conversation

moog16 commented Oct 31, 2018

Choose a reason for hiding this comment

moog16 Oct 31, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abhiomkar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mdc-web-bot commented Nov 7, 2018

Screenshot test report 🚦

3 Changed:

3 Added:

moog16 Oct 31, 2018 •

edited

Loading