Responsive drawer #6535

oliver-ni · 2017-08-17T22:17:10Z

New version of #6525

Fixes #1130

Pull

* Add breakpointWidth * Breaks automatically Fix #1130

oliver-ni · 2017-08-17T22:25:09Z

Fixing code style in progress..

* Strip trailing whitespace * Fix long lines * Use element width instead of window width

* Change 'rxjs/subscription' to 'rxjs/Subscription'

rafaelss95 · 2017-08-17T22:46:51Z

src/lib/sidenav/drawer.ts

+        }
+        this.close();
+      }
+      if (this._element.nativeElement.offsetWidth > this.breakpointWidth) {


else here sounds better.

Didn't notice that, fixing.

oliver-ni · 2017-08-20T20:29:48Z

@willshowell

willshowell

Can you update your original post to mark it as fixing the issue?
The team will require unit tests before merging
I looked over the drawer/sidenav PR again, and am now not so sure it's a public facing change... Maybe @mmalerba can comment on the correct usage going forward?

willshowell · 2017-08-21T12:54:32Z

src/lib/sidenav/drawer.ts

@@ -325,6 +328,27 @@ export class MdDrawerContainer implements AfterContentInit {
  /** Event emitted when the drawer backdrop is clicked. */
  @Output() backdropClick = new EventEmitter<void>();

+  /**
+   * Width of the container at which it breaks
+   * e.g., breakpointWidth="500"


Maybe just,

/** Breakpoint width (in px) at which the sidenav collapses. */

willshowell · 2017-08-21T13:11:59Z

src/lib/sidenav/drawer.ts

+   * Width of the container at which it breaks
+   * e.g., breakpointWidth="500"
+   */
+  @Input() breakpointWidth: number;


Coerce this? See https://github.com/angular/material2/blob/master/src/lib/slider/slider.ts#L149-L164

willshowell · 2017-08-21T13:14:10Z

src/lib/sidenav/drawer.ts

+   * Note that this only works when the window is rezied.
+   * If you resize it some other way, call _updateDrawer().
+   */
+  _updateDrawer: Subscription | null = null;


Maybe rename this to _updateDrawerSubscription and then pull out a method named _updateDrawer that can still manually be used? Or just rename it and update the comment.

willshowell · 2017-08-21T13:17:04Z

src/lib/sidenav/drawer.ts

+  ngOnDestroy() {
+    if (this._updateDrawer) {
+      this._updateDrawer.unsubscribe();
+      this._updateDrawer = null;


I don't think setting to null is necessary since the component is being destroyed

willshowell · 2017-08-21T13:20:41Z

src/lib/sidenav/drawer.ts

+  set breakpointChangeMode(value: boolean) {
+    this._breakpointChangeMode = coerceBooleanProperty(value);
+  }
+  private _breakpointChangeMode: boolean = true;


Type should be implicit

private _breakpointChangeMode = true;

willshowell · 2017-08-21T13:22:44Z

src/lib/sidenav/drawer.ts

+      observableOf(null);
+
+    this._updateDrawer = startWith.call(resize, null).subscribe(() => {
+      if (this._element.nativeElement.offsetWidth < this.breakpointWidth) {


I haven't checked, but this needs to be consistent with any other responsive behavior in this lib or in flex-layout (i.e. < vs <=)

* coerce width * change comments * change _updateDrawer to _updateDrawerSubscription and add separate method

…rial2 into responsive-drawer

oliver-ni · 2017-08-22T02:37:58Z

comments addressed

rafaelss95 · 2017-08-22T02:41:20Z

src/lib/sidenav/drawer.ts


  /** Whether the drawer changes modes when collapsing. */
  @Input()
-  get breakpointChangeMode(): boolean { return this._breakpointChangeMode; }
+  get breakpointChangeMode() { return this._breakpointChangeMode; }


Just a quick question... why did you remove the return type here?

It wasn't on the number one, so I removed it here

mmalerba · 2017-08-22T16:28:54Z

src/lib/sidenav/drawer.ts

+  /** Breakpoint width (in px) at which the sidenav collapses. */
+
+  @Input()
+  get breakpointWidth() { return this._breakpointWidth; }


@jelbourn have you added anything to cdk yet for dealing with breakpoints?

mmalerba · 2017-08-22T16:36:46Z

src/demo-app/sidenav/sidenav-demo.html

@@ -48,6 +48,18 @@
  </div>
 </md-sidenav-container>

+<h2>Responsive Drawer</h2>
+
+<md-sidenav-container class="demo-sidenav-container" breakpointWidth="600" breakpointChangeMode>


I'm not so sure about this API, it doesn't seem to give much flexibility. You can't control what modes it switches between and you can't disable the auto-close/open feature

mmalerba · 2017-08-22T16:41:58Z

@willshowell the split between sidenav and drawer is to allow us to add features that make sense for a true global sidenav to the md-sidenav component (e.g. some fixed positioning features) and leave the md-drawer as something that can be used to create subsections of your app that have their own side "drawers". Currently they're pretty much the same, but they will diverge. This change seems like it belongs in the sidenav only since I don't think the feature makes sense for the drawer-style usage

jelbourn · 2017-08-22T16:50:40Z

@mmalerba not yet; that will be @josephperrott's first task next week

In general I'd like to have one consistent API pattern for defining responsive behaviors for all components

josephperrott · 2017-10-06T22:17:42Z

@mcparadip With the merge of #6772, an API is now available to manage watching breakpoints. You should be able to rebase and use this to work on making the drawer responsive.

oliver-ni · 2017-10-07T00:26:56Z

Got it. Will work on it in a while

angular-automatic-lock-bot · 2019-09-07T09:01:28Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

oliver-ni added 3 commits August 17, 2017 14:51

Merge pull request #1 from angular/master

2ad5cd2

Pull

feat(drawer): add responsive drawer

560e60d

* Add breakpointWidth * Breaks automatically Fix #1130

fix(drawer): typo "the the"

267389b

oliver-ni requested a review from mmalerba as a code owner August 17, 2017 22:17

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Aug 17, 2017

oliver-ni mentioned this pull request Aug 17, 2017

feat(sidenav): add responsive sidenav #6525

Closed

oliver-ni added 2 commits August 17, 2017 15:30

fix(drawer): fix code style and accidental bug

d7d1214

* Strip trailing whitespace * Fix long lines * Use element width instead of window width

fix(drawer): fix subscription import

759b31d

* Change 'rxjs/subscription' to 'rxjs/Subscription'

rafaelss95 reviewed Aug 17, 2017

View reviewed changes

refactor(drawer): change double if to if-else

806bd8d

willshowell reviewed Aug 21, 2017

View reviewed changes

oliver-ni added 2 commits August 21, 2017 19:29

fix(drawer): fixed responsive drawer code

2b69f7b

* coerce width * change comments * change _updateDrawer to _updateDrawerSubscription and add separate method

Merge branch 'responsive-drawer' of https://github.com/mcparadip/mate…

c1f705f

…rial2 into responsive-drawer

rafaelss95 reviewed Aug 22, 2017

View reviewed changes

refactor(drawer): fix lint issues

47e698e

mmalerba reviewed Aug 22, 2017

View reviewed changes

mmalerba assigned josephperrott Sep 26, 2017

josephperrott removed their assignment Oct 6, 2017

oliver-ni closed this Oct 7, 2017

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Responsive drawer #6535

Responsive drawer #6535

oliver-ni commented Aug 17, 2017 •

edited

Loading

oliver-ni commented Aug 17, 2017

rafaelss95 Aug 17, 2017

oliver-ni Aug 18, 2017

oliver-ni commented Aug 20, 2017

willshowell left a comment

willshowell Aug 21, 2017

willshowell Aug 21, 2017

willshowell Aug 21, 2017

willshowell Aug 21, 2017

willshowell Aug 21, 2017

willshowell Aug 21, 2017

oliver-ni commented Aug 22, 2017

rafaelss95 Aug 22, 2017

oliver-ni Aug 22, 2017

mmalerba Aug 22, 2017

mmalerba Aug 22, 2017

mmalerba commented Aug 22, 2017

jelbourn commented Aug 22, 2017

josephperrott commented Oct 6, 2017

oliver-ni commented Oct 7, 2017

angular-automatic-lock-bot bot commented Sep 7, 2019

Responsive drawer #6535

Responsive drawer #6535

Conversation

oliver-ni commented Aug 17, 2017 • edited Loading

oliver-ni commented Aug 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oliver-ni commented Aug 20, 2017

willshowell left a 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

oliver-ni commented Aug 22, 2017

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

mmalerba commented Aug 22, 2017

jelbourn commented Aug 22, 2017

josephperrott commented Oct 6, 2017

oliver-ni commented Oct 7, 2017

angular-automatic-lock-bot bot commented Sep 7, 2019

oliver-ni commented Aug 17, 2017 •

edited

Loading