Skip to content

Commit 89e9621

Browse files
crisbetoandrewseguin
authored andcommitted
chore(sidenav): switch to OnPush change detection (#5731)
Switches the `MdSidenavContainer` to `OnPush` change detection and fixes some changes not being picked up when its sidenav is opened. Relates to #5035.
1 parent 76cc6f0 commit 89e9621

File tree

2 files changed

+14
-10
lines changed

2 files changed

+14
-10
lines changed

e2e/components/sidenav-e2e.spec.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import {browser, by, element, ExpectedConditions} from 'protractor';
2-
const EC = ExpectedConditions;
32

43
describe('sidenav', () => {
54
describe('opening and closing', () => {
@@ -20,7 +19,7 @@ describe('sidenav', () => {
2019
it('should close again', () => {
2120
element(by.buttonText('Open sidenav')).click();
2221
element(by.buttonText('Open sidenav')).click();
23-
browser.wait(EC.presenceOf(element(by.className('mat-sidenav-closed'))), 1000);
22+
browser.wait(ExpectedConditions.presenceOf(element(by.className('mat-sidenav-closed'))), 999);
2423
expect(input.isDisplayed()).toBeFalsy();
2524
});
2625
});

src/lib/sidenav/sidenav.ts

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,14 @@ import {
2222
NgZone,
2323
OnDestroy,
2424
Inject,
25+
ChangeDetectorRef,
2526
} from '@angular/core';
2627
import {Directionality, coerceBooleanProperty} from '../core';
2728
import {FocusTrapFactory, FocusTrap} from '../core/a11y/focus-trap';
2829
import {ESCAPE} from '../core/keyboard/keycodes';
2930
import {first} from '../core/rxjs/index';
3031
import {DOCUMENT} from '@angular/platform-browser';
32+
import {merge} from 'rxjs/observable/merge';
3133

3234

3335
/** Throws an exception when two MdSidenav are matching the same side. */
@@ -52,7 +54,6 @@ export class MdSidenavToggleResult {
5254
@Component({
5355
moduleId: module.id,
5456
selector: 'md-sidenav, mat-sidenav',
55-
// TODO(mmalerba): move template to separate file.
5657
templateUrl: 'sidenav.html',
5758
host: {
5859
'class': 'mat-sidenav',
@@ -324,9 +325,6 @@ export class MdSidenav implements AfterContentInit, OnDestroy {
324325
@Component({
325326
moduleId: module.id,
326327
selector: 'md-sidenav-container, mat-sidenav-container',
327-
// Do not use ChangeDetectionStrategy.OnPush. It does not work for this component because
328-
// technically it is a sibling of MdSidenav (on the content tree) and isn't updated when MdSidenav
329-
// changes its state.
330328
templateUrl: 'sidenav-container.html',
331329
styleUrls: [
332330
'sidenav.css',
@@ -335,6 +333,7 @@ export class MdSidenav implements AfterContentInit, OnDestroy {
335333
host: {
336334
'class': 'mat-sidenav-container',
337335
},
336+
changeDetection: ChangeDetectionStrategy.OnPush,
338337
encapsulation: ViewEncapsulation.None,
339338
})
340339
export class MdSidenavContainer implements AfterContentInit {
@@ -363,7 +362,8 @@ export class MdSidenavContainer implements AfterContentInit {
363362
private _right: MdSidenav | null;
364363

365364
constructor(@Optional() private _dir: Directionality, private _element: ElementRef,
366-
private _renderer: Renderer2, private _ngZone: NgZone) {
365+
private _renderer: Renderer2, private _ngZone: NgZone,
366+
private _changeDetectorRef: ChangeDetectorRef) {
367367
// If a `Dir` directive exists up the tree, listen direction changes and update the left/right
368368
// properties to point to the proper start/end.
369369
if (_dir != null) {
@@ -408,9 +408,14 @@ export class MdSidenavContainer implements AfterContentInit {
408408
* is properly hidden.
409409
*/
410410
private _watchSidenavToggle(sidenav: MdSidenav): void {
411-
if (!sidenav || sidenav.mode === 'side') { return; }
412-
sidenav.onOpen.subscribe(() => this._setContainerClass(true));
413-
sidenav.onClose.subscribe(() => this._setContainerClass(false));
411+
merge(sidenav.onOpenStart, sidenav.onCloseStart).subscribe(() => {
412+
this._changeDetectorRef.markForCheck();
413+
});
414+
415+
if (sidenav.mode !== 'side') {
416+
sidenav.onOpen.subscribe(() => this._setContainerClass(true));
417+
sidenav.onClose.subscribe(() => this._setContainerClass(false));
418+
}
414419
}
415420

416421
/**

0 commit comments

Comments
 (0)