chore(sidenav): switch to OnPush change detection

Switches the `MdSidenavContainer` to `OnPush` change detection and fixes some changes not being picked up when its sidenav is opened. Relates to #5035.
angular · Jul 14, 2017 · 482e9cc · 482e9cc
1 parent 1e41b04
commit 482e9cc
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 10 deletions.
diff --git a/e2e/components/sidenav-e2e.spec.ts b/e2e/components/sidenav-e2e.spec.ts
@@ -1,5 +1,4 @@
 import {browser, by, element, ExpectedConditions} from 'protractor';
-const EC = ExpectedConditions;
 
 describe('sidenav', () => {
   describe('opening and closing', () => {
@@ -20,7 +19,7 @@ describe('sidenav', () => {
     it('should close again', () => {
       element(by.buttonText('Open sidenav')).click();
       element(by.buttonText('Open sidenav')).click();
-      browser.wait(EC.presenceOf(element(by.className('mat-sidenav-closed'))), 1000);
+      browser.wait(ExpectedConditions.presenceOf(element(by.className('mat-sidenav-closed'))), 999);
       expect(input.isDisplayed()).toBeFalsy();
     });
   });

diff --git a/src/lib/sidenav/sidenav.ts b/src/lib/sidenav/sidenav.ts
@@ -22,12 +22,14 @@ import {
   NgZone,
   OnDestroy,
   Inject,
+  ChangeDetectorRef,
 } from '@angular/core';
 import {Directionality, coerceBooleanProperty} from '../core';
 import {FocusTrapFactory, FocusTrap} from '../core/a11y/focus-trap';
 import {ESCAPE} from '../core/keyboard/keycodes';
 import {first} from '../core/rxjs/index';
 import {DOCUMENT} from '@angular/platform-browser';
+import {merge} from 'rxjs/observable/merge';
 
 
 /** Throws an exception when two MdSidenav are matching the same side. */
@@ -52,7 +54,6 @@ export class MdSidenavToggleResult {
 @Component({
   moduleId: module.id,
   selector: 'md-sidenav, mat-sidenav',
-  // TODO(mmalerba): move template to separate file.
   templateUrl: 'sidenav.html',
   host: {
     'class': 'mat-sidenav',
@@ -324,9 +325,6 @@ export class MdSidenav implements AfterContentInit, OnDestroy {
 @Component({
   moduleId: module.id,
   selector: 'md-sidenav-container, mat-sidenav-container',
-  // Do not use ChangeDetectionStrategy.OnPush. It does not work for this component because
-  // technically it is a sibling of MdSidenav (on the content tree) and isn't updated when MdSidenav
-  // changes its state.
   templateUrl: 'sidenav-container.html',
   styleUrls: [
     'sidenav.css',
@@ -336,6 +334,7 @@ export class MdSidenav implements AfterContentInit, OnDestroy {
     'class': 'mat-sidenav-container',
     '[class.mat-sidenav-transition]': '_enableTransitions',
   },
+  changeDetection: ChangeDetectionStrategy.OnPush,
   encapsulation: ViewEncapsulation.None,
 })
 export class MdSidenavContainer implements AfterContentInit {
@@ -367,7 +366,8 @@ export class MdSidenavContainer implements AfterContentInit {
   _enableTransitions = false;
 
   constructor(@Optional() private _dir: Directionality, private _element: ElementRef,
-              private _renderer: Renderer2, private _ngZone: NgZone) {
+              private _renderer: Renderer2, private _ngZone: NgZone,
+              private _changeDetectorRef: ChangeDetectorRef) {
     // If a `Dir` directive exists up the tree, listen direction changes and update the left/right
     // properties to point to the proper start/end.
     if (_dir != null) {
@@ -408,9 +408,14 @@ export class MdSidenavContainer implements AfterContentInit {
    * is properly hidden.
    */
   private _watchSidenavToggle(sidenav: MdSidenav): void {
-    if (!sidenav || sidenav.mode === 'side') { return; }
-    sidenav.onOpen.subscribe(() => this._setContainerClass(true));
-    sidenav.onClose.subscribe(() => this._setContainerClass(false));
+    merge(sidenav.onOpenStart, sidenav.onCloseStart).subscribe(() => {
+      this._changeDetectorRef.markForCheck();
+    });
+
+    if (sidenav.mode !== 'side') {
+      sidenav.onOpen.subscribe(() => this._setContainerClass(true));
+      sidenav.onClose.subscribe(() => this._setContainerClass(false));
+    }
   }
 
   /**