fix(sidenav): container not picking up indirect descendant sid… (#17453)

crisbeto · mmalerba · commit 993347977167 · 2019-10-21T16:26:13.000-07:00
Fixes the sidenav container not picking up indirect descendant sidenavs.
diff --git a/src/material/sidenav/BUILD.bazel b/src/material/sidenav/BUILD.bazel
@@ -65,6 +65,7 @@ ng_test_library(
         "//src/cdk/platform",
         "//src/cdk/scrolling",
         "//src/cdk/testing",
+        "@npm//@angular/common",
         "@npm//@angular/platform-browser",
     ],
 )
diff --git a/src/material/sidenav/drawer.spec.ts b/src/material/sidenav/drawer.spec.ts
@@ -18,12 +18,13 @@ import {PlatformModule, Platform} from '@angular/cdk/platform';
 import {ESCAPE} from '@angular/cdk/keycodes';
 import {dispatchKeyboardEvent, createKeyboardEvent, dispatchEvent} from '@angular/cdk/testing';
 import {CdkScrollable} from '@angular/cdk/scrolling';
+import {CommonModule} from '@angular/common';
 
 
 describe('MatDrawer', () => {
   beforeEach(async(() => {
     TestBed.configureTestingModule({
-      imports: [MatSidenavModule, A11yModule, PlatformModule, NoopAnimationsModule],
+      imports: [MatSidenavModule, A11yModule, PlatformModule, NoopAnimationsModule, CommonModule],
       declarations: [
         BasicTestApp,
         DrawerContainerNoDrawerTestApp,
@@ -33,6 +34,8 @@ describe('MatDrawer', () => {
         DrawerWithFocusableElements,
         DrawerOpenBinding,
         DrawerWithoutFocusableElements,
+        IndirectDescendantDrawer,
+        NestedDrawerContainers,
       ],
     });
 
@@ -323,6 +326,46 @@ describe('MatDrawer', () => {
       expect(document.activeElement)
           .toBe(closeButton, 'Expected focus not to be restored to the open button on close.');
     }));
+
+    it('should pick up drawers that are not direct descendants', fakeAsync(() => {
+      const fixture = TestBed.createComponent(IndirectDescendantDrawer);
+      fixture.detectChanges();
+
+      expect(fixture.componentInstance.drawer.opened).toBe(false);
+
+      fixture.componentInstance.container.open();
+      fixture.detectChanges();
+      tick();
+      fixture.detectChanges();
+
+      expect(fixture.componentInstance.drawer.opened).toBe(true);
+    }));
+
+    it('should not pick up drawers from nested containers', fakeAsync(() => {
+      const fixture = TestBed.createComponent(NestedDrawerContainers);
+      const instance = fixture.componentInstance;
+      fixture.detectChanges();
+
+      expect(instance.outerDrawer.opened).toBe(false);
+      expect(instance.innerDrawer.opened).toBe(false);
+
+      instance.outerContainer.open();
+      fixture.detectChanges();
+      tick();
+      fixture.detectChanges();
+
+      expect(instance.outerDrawer.opened).toBe(true);
+      expect(instance.innerDrawer.opened).toBe(false);
+
+      instance.innerContainer.open();
+      fixture.detectChanges();
+      tick();
+      fixture.detectChanges();
+
+      expect(instance.outerDrawer.opened).toBe(true);
+      expect(instance.innerDrawer.opened).toBe(true);
+    }));
+
   });
 
   describe('attributes', () => {
@@ -998,3 +1041,38 @@ class AutosizeDrawer {
 class DrawerContainerWithContent {
   @ViewChild(MatDrawerContainer, {static: false}) drawerContainer: MatDrawerContainer;
 }
+
+
+@Component({
+  // Note that we need the `ng-container` with the `ngSwitch` so that
+  // there's a directive between the container and the drawer.
+  template: `
+    <mat-drawer-container #container>
+      <ng-container [ngSwitch]="true">
+        <mat-drawer #drawer>Drawer</mat-drawer>
+      </ng-container>
+    </mat-drawer-container>`,
+})
+class IndirectDescendantDrawer {
+  @ViewChild('container', {static: false}) container: MatDrawerContainer;
+  @ViewChild('drawer', {static: false}) drawer: MatDrawer;
+}
+
+@Component({
+  template: `
+    <mat-drawer-container #outerContainer>
+      <mat-drawer #outerDrawer>Drawer</mat-drawer>
+      <mat-drawer-content>
+        <mat-drawer-container #innerContainer>
+          <mat-drawer #innerDrawer>Drawer</mat-drawer>
+        </mat-drawer-container>
+      </mat-drawer-content>
+    </mat-drawer-container>
+  `,
+})
+class NestedDrawerContainers {
+  @ViewChild('outerContainer', {static: false}) outerContainer: MatDrawerContainer;
+  @ViewChild('outerDrawer', {static: false}) outerDrawer: MatDrawer;
+  @ViewChild('innerContainer', {static: false}) innerContainer: MatDrawerContainer;
+  @ViewChild('innerDrawer', {static: false}) innerDrawer: MatDrawer;
+}
diff --git a/src/material/sidenav/drawer.ts b/src/material/sidenav/drawer.ts
@@ -71,6 +71,13 @@ export const MAT_DRAWER_DEFAULT_AUTOSIZE =
       factory: MAT_DRAWER_DEFAULT_AUTOSIZE_FACTORY,
     });
 
+
+/**
+ * Used to provide a drawer container to a drawer while avoiding circular references.
+ * @docs-private
+ */
+export const MAT_DRAWER_CONTAINER = new InjectionToken('MAT_DRAWER_CONTAINER');
+
 /** @docs-private */
 export function MAT_DRAWER_DEFAULT_AUTOSIZE_FACTORY(): boolean {
   return false;
@@ -246,7 +253,12 @@ export class MatDrawer implements AfterContentInit, AfterContentChecked, OnDestr
               private _focusMonitor: FocusMonitor,
               private _platform: Platform,
               private _ngZone: NgZone,
-              @Optional() @Inject(DOCUMENT) private _doc: any) {
+              @Optional() @Inject(DOCUMENT) private _doc: any,
+              /**
+               * @deprecated `_container` parameter to be made required.
+               * @breaking-change 10.0.0
+               */
+              @Optional() @Inject(MAT_DRAWER_CONTAINER) public _container?: MatDrawerContainer) {
 
     this.openedChange.subscribe((opened: boolean) => {
       if (opened) {
@@ -459,9 +471,23 @@ export class MatDrawer implements AfterContentInit, AfterContentChecked, OnDestr
   },
   changeDetection: ChangeDetectionStrategy.OnPush,
   encapsulation: ViewEncapsulation.None,
+  providers: [{
+    provide: MAT_DRAWER_CONTAINER,
+    useExisting: MatDrawerContainer
+  }]
 })
 export class MatDrawerContainer implements AfterContentInit, DoCheck, OnDestroy {
-  @ContentChildren(MatDrawer) _drawers: QueryList<MatDrawer>;
+  /** All drawers in the container. Includes drawers from inside nested containers. */
+  @ContentChildren(MatDrawer, {
+    // We need to use `descendants: true`, because Ivy will no longer match
+    // indirect descendants if it's left as false.
+    descendants: true
+  })
+  _allDrawers: QueryList<MatDrawer>;
+
+  /** Drawers that belong to this container. */
+  _drawers = new QueryList<MatDrawer>();
+
   @ContentChild(MatDrawerContent, {static: false}) _content: MatDrawerContent;
   @ViewChild(MatDrawerContent, {static: false}) _userContent: MatDrawerContent;
 
@@ -565,6 +591,14 @@ export class MatDrawerContainer implements AfterContentInit, DoCheck, OnDestroy
   }
 
   ngAfterContentInit() {
+    this._allDrawers.changes
+      .pipe(startWith(this._allDrawers), takeUntil(this._destroyed))
+      .subscribe((drawer: QueryList<MatDrawer>) => {
+        // @breaking-change 10.0.0 Remove `_container` check once container parameter is required.
+        this._drawers.reset(drawer.filter(item => !item._container || item._container === this));
+        this._drawers.notifyOnChanges();
+      });
+
     this._drawers.changes.pipe(startWith(null)).subscribe(() => {
       this._validateDrawers();
 
diff --git a/src/material/sidenav/public-api.ts b/src/material/sidenav/public-api.ts
@@ -7,6 +7,14 @@
  */
 
 export * from './sidenav-module';
-export * from './drawer';
+export {
+  throwMatDuplicatedDrawerError,
+  MatDrawerToggleResult,
+  MAT_DRAWER_DEFAULT_AUTOSIZE,
+  MAT_DRAWER_DEFAULT_AUTOSIZE_FACTORY,
+  MatDrawerContent,
+  MatDrawer,
+  MatDrawerContainer,
+} from './drawer';
 export * from './sidenav';
 export * from './drawer-animations';
diff --git a/src/material/sidenav/sidenav.spec.ts b/src/material/sidenav/sidenav.spec.ts
@@ -1,29 +1,26 @@
-import {Component} from '@angular/core';
-import {async, TestBed, ComponentFixture} from '@angular/core/testing';
-import {MatSidenav, MatSidenavModule} from './index';
+import {Component, ViewChild} from '@angular/core';
+import {async, TestBed, fakeAsync, tick} from '@angular/core/testing';
+import {MatSidenav, MatSidenavModule, MatSidenavContainer} from './index';
 import {NoopAnimationsModule} from '@angular/platform-browser/animations';
 import {By} from '@angular/platform-browser';
+import {CommonModule} from '@angular/common';
 
 
 describe('MatSidenav', () => {
-  let fixture: ComponentFixture<SidenavWithFixedPosition>;
-  let sidenavEl: HTMLElement;
-
   beforeEach(async(() => {
     TestBed.configureTestingModule({
-      imports: [MatSidenavModule, NoopAnimationsModule],
-      declarations: [SidenavWithFixedPosition],
+      imports: [MatSidenavModule, NoopAnimationsModule, CommonModule],
+      declarations: [SidenavWithFixedPosition, IndirectDescendantSidenav, NestedSidenavContainers],
     });
 
     TestBed.compileComponents();
-
-    fixture = TestBed.createComponent(SidenavWithFixedPosition);
-    fixture.detectChanges();
-
-    sidenavEl = fixture.debugElement.query(By.directive(MatSidenav))!.nativeElement;
   }));
 
   it('should be fixed position when in fixed mode', () => {
+    const fixture = TestBed.createComponent(SidenavWithFixedPosition);
+    fixture.detectChanges();
+    const sidenavEl = fixture.debugElement.query(By.directive(MatSidenav))!.nativeElement;
+
     expect(sidenavEl.classList).toContain('mat-sidenav-fixed');
 
     fixture.componentInstance.fixed = false;
@@ -33,6 +30,10 @@ describe('MatSidenav', () => {
   });
 
   it('should set fixed bottom and top when in fixed mode', () => {
+    const fixture = TestBed.createComponent(SidenavWithFixedPosition);
+    fixture.detectChanges();
+    const sidenavEl = fixture.debugElement.query(By.directive(MatSidenav))!.nativeElement;
+
     expect(sidenavEl.style.top).toBe('20px');
     expect(sidenavEl.style.bottom).toBe('30px');
 
@@ -42,6 +43,46 @@ describe('MatSidenav', () => {
     expect(sidenavEl.style.top).toBeFalsy();
     expect(sidenavEl.style.bottom).toBeFalsy();
   });
+
+  it('should pick up sidenavs that are not direct descendants', fakeAsync(() => {
+    const fixture = TestBed.createComponent(IndirectDescendantSidenav);
+    fixture.detectChanges();
+
+    expect(fixture.componentInstance.sidenav.opened).toBe(false);
+
+    fixture.componentInstance.container.open();
+    fixture.detectChanges();
+    tick();
+    fixture.detectChanges();
+
+    expect(fixture.componentInstance.sidenav.opened).toBe(true);
+  }));
+
+  it('should not pick up sidenavs from nested containers', fakeAsync(() => {
+    const fixture = TestBed.createComponent(NestedSidenavContainers);
+    const instance = fixture.componentInstance;
+    fixture.detectChanges();
+
+    expect(instance.outerSidenav.opened).toBe(false);
+    expect(instance.innerSidenav.opened).toBe(false);
+
+    instance.outerContainer.open();
+    fixture.detectChanges();
+    tick();
+    fixture.detectChanges();
+
+    expect(instance.outerSidenav.opened).toBe(true);
+    expect(instance.innerSidenav.opened).toBe(false);
+
+    instance.innerContainer.open();
+    fixture.detectChanges();
+    tick();
+    fixture.detectChanges();
+
+    expect(instance.outerSidenav.opened).toBe(true);
+    expect(instance.innerSidenav.opened).toBe(true);
+  }));
+
 });
 
 
@@ -65,3 +106,39 @@ class SidenavWithFixedPosition {
   fixedTop = 20;
   fixedBottom = 30;
 }
+
+
+@Component({
+  // Note that we need the `ng-container` with the `ngSwitch` so that
+  // there's a directive between the container and the sidenav.
+  template: `
+    <mat-sidenav-container #container>
+      <ng-container [ngSwitch]="true">
+        <mat-sidenav #sidenav>Sidenav.</mat-sidenav>
+      </ng-container>
+      <mat-sidenav-content>Some content.</mat-sidenav-content>
+    </mat-sidenav-container>`,
+})
+class IndirectDescendantSidenav {
+  @ViewChild('container', {static: false}) container: MatSidenavContainer;
+  @ViewChild('sidenav', {static: false}) sidenav: MatSidenav;
+}
+
+@Component({
+  template: `
+    <mat-sidenav-container #outerContainer>
+      <mat-sidenav #outerSidenav>Sidenav</mat-sidenav>
+      <mat-sidenav-content>
+        <mat-sidenav-container #innerContainer>
+          <mat-sidenav #innerSidenav>Sidenav</mat-sidenav>
+        </mat-sidenav-container>
+      </mat-sidenav-content>
+    </mat-sidenav-container>
+  `,
+})
+class NestedSidenavContainers {
+  @ViewChild('outerContainer', {static: false}) outerContainer: MatSidenavContainer;
+  @ViewChild('outerSidenav', {static: false}) outerSidenav: MatSidenav;
+  @ViewChild('innerContainer', {static: false}) innerContainer: MatSidenavContainer;
+  @ViewChild('innerSidenav', {static: false}) innerSidenav: MatSidenav;
+}
diff --git a/src/material/sidenav/sidenav.ts b/src/material/sidenav/sidenav.ts
@@ -20,7 +20,7 @@ import {
   ElementRef,
   NgZone,
 } from '@angular/core';
-import {MatDrawer, MatDrawerContainer, MatDrawerContent} from './drawer';
+import {MatDrawer, MatDrawerContainer, MatDrawerContent, MAT_DRAWER_CONTAINER} from './drawer';
 import {matDrawerAnimations} from './drawer-animations';
 import {coerceBooleanProperty, coerceNumberProperty} from '@angular/cdk/coercion';
 import {ScrollDispatcher} from '@angular/cdk/scrolling';
@@ -112,8 +112,19 @@ export class MatSidenav extends MatDrawer {
   },
   changeDetection: ChangeDetectionStrategy.OnPush,
   encapsulation: ViewEncapsulation.None,
+  providers: [{
+    provide: MAT_DRAWER_CONTAINER,
+    useExisting: MatSidenavContainer
+  }]
+
 })
 export class MatSidenavContainer extends MatDrawerContainer {
-  @ContentChildren(MatSidenav) _drawers: QueryList<MatSidenav>;
+  @ContentChildren(MatSidenav, {
+    // We need to use `descendants: true`, because Ivy will no longer match
+    // indirect descendants if it's left as false.
+    descendants: true
+  })
+  _allDrawers: QueryList<MatSidenav>;
+
   @ContentChild(MatSidenavContent, {static: false}) _content: MatSidenavContent;
 }
diff --git a/tools/public_api_guard/material/sidenav.d.ts b/tools/public_api_guard/material/sidenav.d.ts

Original file line number	Diff line number	Diff line change
`@@ -65,6 +65,7 @@ ng_test_library(`
`65`	`65`	`"//src/cdk/platform",`
`66`	`66`	`"//src/cdk/scrolling",`
`67`	`67`	`"//src/cdk/testing",`
	`68`	`+ "@npm//@angular/common",`
`68`	`69`	`"@npm//@angular/platform-browser",`
`69`	`70`	`],`
`70`	`71`	`)`