Skip to content

Commit 9933479

Browse files
crisbetommalerba
crisbeto
authored and
mmalerba
committed
fix(sidenav): container not picking up indirect descendant sid… (#17453)
Fixes the sidenav container not picking up indirect descendant sidenavs.
1 parent d773cce commit 9933479

File tree

7 files changed

+233
-21
lines changed

7 files changed

+233
-21
lines changed

‎src/material/sidenav/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ ng_test_library(
6565
"//src/cdk/platform",
6666
"//src/cdk/scrolling",
6767
"//src/cdk/testing",
68+
"@npm//@angular/common",
6869
"@npm//@angular/platform-browser",
6970
],
7071
)

‎src/material/sidenav/drawer.spec.ts

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,13 @@ import {PlatformModule, Platform} from '@angular/cdk/platform';
1818
import {ESCAPE} from '@angular/cdk/keycodes';
1919
import {dispatchKeyboardEvent, createKeyboardEvent, dispatchEvent} from '@angular/cdk/testing';
2020
import {CdkScrollable} from '@angular/cdk/scrolling';
21+
import {CommonModule} from '@angular/common';
2122

2223

2324
describe('MatDrawer', () => {
2425
beforeEach(async(() => {
2526
TestBed.configureTestingModule({
26-
imports: [MatSidenavModule, A11yModule, PlatformModule, NoopAnimationsModule],
27+
imports: [MatSidenavModule, A11yModule, PlatformModule, NoopAnimationsModule, CommonModule],
2728
declarations: [
2829
BasicTestApp,
2930
DrawerContainerNoDrawerTestApp,
@@ -33,6 +34,8 @@ describe('MatDrawer', () => {
3334
DrawerWithFocusableElements,
3435
DrawerOpenBinding,
3536
DrawerWithoutFocusableElements,
37+
IndirectDescendantDrawer,
38+
NestedDrawerContainers,
3639
],
3740
});
3841

@@ -323,6 +326,46 @@ describe('MatDrawer', () => {
323326
expect(document.activeElement)
324327
.toBe(closeButton, 'Expected focus not to be restored to the open button on close.');
325328
}));
329+
330+
it('should pick up drawers that are not direct descendants', fakeAsync(() => {
331+
const fixture = TestBed.createComponent(IndirectDescendantDrawer);
332+
fixture.detectChanges();
333+
334+
expect(fixture.componentInstance.drawer.opened).toBe(false);
335+
336+
fixture.componentInstance.container.open();
337+
fixture.detectChanges();
338+
tick();
339+
fixture.detectChanges();
340+
341+
expect(fixture.componentInstance.drawer.opened).toBe(true);
342+
}));
343+
344+
it('should not pick up drawers from nested containers', fakeAsync(() => {
345+
const fixture = TestBed.createComponent(NestedDrawerContainers);
346+
const instance = fixture.componentInstance;
347+
fixture.detectChanges();
348+
349+
expect(instance.outerDrawer.opened).toBe(false);
350+
expect(instance.innerDrawer.opened).toBe(false);
351+
352+
instance.outerContainer.open();
353+
fixture.detectChanges();
354+
tick();
355+
fixture.detectChanges();
356+
357+
expect(instance.outerDrawer.opened).toBe(true);
358+
expect(instance.innerDrawer.opened).toBe(false);
359+
360+
instance.innerContainer.open();
361+
fixture.detectChanges();
362+
tick();
363+
fixture.detectChanges();
364+
365+
expect(instance.outerDrawer.opened).toBe(true);
366+
expect(instance.innerDrawer.opened).toBe(true);
367+
}));
368+
326369
});
327370

328371
describe('attributes', () => {
@@ -998,3 +1041,38 @@ class AutosizeDrawer {
9981041
class DrawerContainerWithContent {
9991042
@ViewChild(MatDrawerContainer, {static: false}) drawerContainer: MatDrawerContainer;
10001043
}
1044+
1045+
1046+
@Component({
1047+
// Note that we need the `ng-container` with the `ngSwitch` so that
1048+
// there's a directive between the container and the drawer.
1049+
template: `
1050+
<mat-drawer-container #container>
1051+
<ng-container [ngSwitch]="true">
1052+
<mat-drawer #drawer>Drawer</mat-drawer>
1053+
</ng-container>
1054+
</mat-drawer-container>`,
1055+
})
1056+
class IndirectDescendantDrawer {
1057+
@ViewChild('container', {static: false}) container: MatDrawerContainer;
1058+
@ViewChild('drawer', {static: false}) drawer: MatDrawer;
1059+
}
1060+
1061+
@Component({
1062+
template: `
1063+
<mat-drawer-container #outerContainer>
1064+
<mat-drawer #outerDrawer>Drawer</mat-drawer>
1065+
<mat-drawer-content>
1066+
<mat-drawer-container #innerContainer>
1067+
<mat-drawer #innerDrawer>Drawer</mat-drawer>
1068+
</mat-drawer-container>
1069+
</mat-drawer-content>
1070+
</mat-drawer-container>
1071+
`,
1072+
})
1073+
class NestedDrawerContainers {
1074+
@ViewChild('outerContainer', {static: false}) outerContainer: MatDrawerContainer;
1075+
@ViewChild('outerDrawer', {static: false}) outerDrawer: MatDrawer;
1076+
@ViewChild('innerContainer', {static: false}) innerContainer: MatDrawerContainer;
1077+
@ViewChild('innerDrawer', {static: false}) innerDrawer: MatDrawer;
1078+
}

‎src/material/sidenav/drawer.ts

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,13 @@ export const MAT_DRAWER_DEFAULT_AUTOSIZE =
7171
factory: MAT_DRAWER_DEFAULT_AUTOSIZE_FACTORY,
7272
});
7373

74+
75+
/**
76+
* Used to provide a drawer container to a drawer while avoiding circular references.
77+
* @docs-private
78+
*/
79+
export const MAT_DRAWER_CONTAINER = new InjectionToken('MAT_DRAWER_CONTAINER');
80+
7481
/** @docs-private */
7582
export function MAT_DRAWER_DEFAULT_AUTOSIZE_FACTORY(): boolean {
7683
return false;
@@ -246,7 +253,12 @@ export class MatDrawer implements AfterContentInit, AfterContentChecked, OnDestr
246253
private _focusMonitor: FocusMonitor,
247254
private _platform: Platform,
248255
private _ngZone: NgZone,
249-
@Optional() @Inject(DOCUMENT) private _doc: any) {
256+
@Optional() @Inject(DOCUMENT) private _doc: any,
257+
/**
258+
* @deprecated `_container` parameter to be made required.
259+
* @breaking-change 10.0.0
260+
*/
261+
@Optional() @Inject(MAT_DRAWER_CONTAINER) public _container?: MatDrawerContainer) {
250262

251263
this.openedChange.subscribe((opened: boolean) => {
252264
if (opened) {
@@ -459,9 +471,23 @@ export class MatDrawer implements AfterContentInit, AfterContentChecked, OnDestr
459471
},
460472
changeDetection: ChangeDetectionStrategy.OnPush,
461473
encapsulation: ViewEncapsulation.None,
474+
providers: [{
475+
provide: MAT_DRAWER_CONTAINER,
476+
useExisting: MatDrawerContainer
477+
}]
462478
})
463479
export class MatDrawerContainer implements AfterContentInit, DoCheck, OnDestroy {
464-
@ContentChildren(MatDrawer) _drawers: QueryList<MatDrawer>;
480+
/** All drawers in the container. Includes drawers from inside nested containers. */
481+
@ContentChildren(MatDrawer, {
482+
// We need to use `descendants: true`, because Ivy will no longer match
483+
// indirect descendants if it's left as false.
484+
descendants: true
485+
})
486+
_allDrawers: QueryList<MatDrawer>;
487+
488+
/** Drawers that belong to this container. */
489+
_drawers = new QueryList<MatDrawer>();
490+
465491
@ContentChild(MatDrawerContent, {static: false}) _content: MatDrawerContent;
466492
@ViewChild(MatDrawerContent, {static: false}) _userContent: MatDrawerContent;
467493

@@ -565,6 +591,14 @@ export class MatDrawerContainer implements AfterContentInit, DoCheck, OnDestroy
565591
}
566592

567593
ngAfterContentInit() {
594+
this._allDrawers.changes
595+
.pipe(startWith(this._allDrawers), takeUntil(this._destroyed))
596+
.subscribe((drawer: QueryList<MatDrawer>) => {
597+
// @breaking-change 10.0.0 Remove `_container` check once container parameter is required.
598+
this._drawers.reset(drawer.filter(item => !item._container || item._container === this));
599+
this._drawers.notifyOnChanges();
600+
});
601+
568602
this._drawers.changes.pipe(startWith(null)).subscribe(() => {
569603
this._validateDrawers();
570604

‎src/material/sidenav/public-api.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,14 @@
77
*/
88

99
export * from './sidenav-module';
10-
export * from './drawer';
10+
export {
11+
throwMatDuplicatedDrawerError,
12+
MatDrawerToggleResult,
13+
MAT_DRAWER_DEFAULT_AUTOSIZE,
14+
MAT_DRAWER_DEFAULT_AUTOSIZE_FACTORY,
15+
MatDrawerContent,
16+
MatDrawer,
17+
MatDrawerContainer,
18+
} from './drawer';
1119
export * from './sidenav';
1220
export * from './drawer-animations';

‎src/material/sidenav/sidenav.spec.ts

Lines changed: 90 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,26 @@
1-
import {Component} from '@angular/core';
2-
import {async, TestBed, ComponentFixture} from '@angular/core/testing';
3-
import {MatSidenav, MatSidenavModule} from './index';
1+
import {Component, ViewChild} from '@angular/core';
2+
import {async, TestBed, fakeAsync, tick} from '@angular/core/testing';
3+
import {MatSidenav, MatSidenavModule, MatSidenavContainer} from './index';
44
import {NoopAnimationsModule} from '@angular/platform-browser/animations';
55
import {By} from '@angular/platform-browser';
6+
import {CommonModule} from '@angular/common';
67

78

89
describe('MatSidenav', () => {
9-
let fixture: ComponentFixture<SidenavWithFixedPosition>;
10-
let sidenavEl: HTMLElement;
11-
1210
beforeEach(async(() => {
1311
TestBed.configureTestingModule({
14-
imports: [MatSidenavModule, NoopAnimationsModule],
15-
declarations: [SidenavWithFixedPosition],
12+
imports: [MatSidenavModule, NoopAnimationsModule, CommonModule],
13+
declarations: [SidenavWithFixedPosition, IndirectDescendantSidenav, NestedSidenavContainers],
1614
});
1715

1816
TestBed.compileComponents();
19-
20-
fixture = TestBed.createComponent(SidenavWithFixedPosition);
21-
fixture.detectChanges();
22-
23-
sidenavEl = fixture.debugElement.query(By.directive(MatSidenav))!.nativeElement;
2417
}));
2518

2619
it('should be fixed position when in fixed mode', () => {
20+
const fixture = TestBed.createComponent(SidenavWithFixedPosition);
21+
fixture.detectChanges();
22+
const sidenavEl = fixture.debugElement.query(By.directive(MatSidenav))!.nativeElement;
23+
2724
expect(sidenavEl.classList).toContain('mat-sidenav-fixed');
2825

2926
fixture.componentInstance.fixed = false;
@@ -33,6 +30,10 @@ describe('MatSidenav', () => {
3330
});
3431

3532
it('should set fixed bottom and top when in fixed mode', () => {
33+
const fixture = TestBed.createComponent(SidenavWithFixedPosition);
34+
fixture.detectChanges();
35+
const sidenavEl = fixture.debugElement.query(By.directive(MatSidenav))!.nativeElement;
36+
3637
expect(sidenavEl.style.top).toBe('20px');
3738
expect(sidenavEl.style.bottom).toBe('30px');
3839

@@ -42,6 +43,46 @@ describe('MatSidenav', () => {
4243
expect(sidenavEl.style.top).toBeFalsy();
4344
expect(sidenavEl.style.bottom).toBeFalsy();
4445
});
46+
47+
it('should pick up sidenavs that are not direct descendants', fakeAsync(() => {
48+
const fixture = TestBed.createComponent(IndirectDescendantSidenav);
49+
fixture.detectChanges();
50+
51+
expect(fixture.componentInstance.sidenav.opened).toBe(false);
52+
53+
fixture.componentInstance.container.open();
54+
fixture.detectChanges();
55+
tick();
56+
fixture.detectChanges();
57+
58+
expect(fixture.componentInstance.sidenav.opened).toBe(true);
59+
}));
60+
61+
it('should not pick up sidenavs from nested containers', fakeAsync(() => {
62+
const fixture = TestBed.createComponent(NestedSidenavContainers);
63+
const instance = fixture.componentInstance;
64+
fixture.detectChanges();
65+
66+
expect(instance.outerSidenav.opened).toBe(false);
67+
expect(instance.innerSidenav.opened).toBe(false);
68+
69+
instance.outerContainer.open();
70+
fixture.detectChanges();
71+
tick();
72+
fixture.detectChanges();
73+
74+
expect(instance.outerSidenav.opened).toBe(true);
75+
expect(instance.innerSidenav.opened).toBe(false);
76+
77+
instance.innerContainer.open();
78+
fixture.detectChanges();
79+
tick();
80+
fixture.detectChanges();
81+
82+
expect(instance.outerSidenav.opened).toBe(true);
83+
expect(instance.innerSidenav.opened).toBe(true);
84+
}));
85+
4586
});
4687

4788

@@ -65,3 +106,39 @@ class SidenavWithFixedPosition {
65106
fixedTop = 20;
66107
fixedBottom = 30;
67108
}
109+
110+
111+
@Component({
112+
// Note that we need the `ng-container` with the `ngSwitch` so that
113+
// there's a directive between the container and the sidenav.
114+
template: `
115+
<mat-sidenav-container #container>
116+
<ng-container [ngSwitch]="true">
117+
<mat-sidenav #sidenav>Sidenav.</mat-sidenav>
118+
</ng-container>
119+
<mat-sidenav-content>Some content.</mat-sidenav-content>
120+
</mat-sidenav-container>`,
121+
})
122+
class IndirectDescendantSidenav {
123+
@ViewChild('container', {static: false}) container: MatSidenavContainer;
124+
@ViewChild('sidenav', {static: false}) sidenav: MatSidenav;
125+
}
126+
127+
@Component({
128+
template: `
129+
<mat-sidenav-container #outerContainer>
130+
<mat-sidenav #outerSidenav>Sidenav</mat-sidenav>
131+
<mat-sidenav-content>
132+
<mat-sidenav-container #innerContainer>
133+
<mat-sidenav #innerSidenav>Sidenav</mat-sidenav>
134+
</mat-sidenav-container>
135+
</mat-sidenav-content>
136+
</mat-sidenav-container>
137+
`,
138+
})
139+
class NestedSidenavContainers {
140+
@ViewChild('outerContainer', {static: false}) outerContainer: MatSidenavContainer;
141+
@ViewChild('outerSidenav', {static: false}) outerSidenav: MatSidenav;
142+
@ViewChild('innerContainer', {static: false}) innerContainer: MatSidenavContainer;
143+
@ViewChild('innerSidenav', {static: false}) innerSidenav: MatSidenav;
144+
}

‎src/material/sidenav/sidenav.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import {
2020
ElementRef,
2121
NgZone,
2222
} from '@angular/core';
23-
import {MatDrawer, MatDrawerContainer, MatDrawerContent} from './drawer';
23+
import {MatDrawer, MatDrawerContainer, MatDrawerContent, MAT_DRAWER_CONTAINER} from './drawer';
2424
import {matDrawerAnimations} from './drawer-animations';
2525
import {coerceBooleanProperty, coerceNumberProperty} from '@angular/cdk/coercion';
2626
import {ScrollDispatcher} from '@angular/cdk/scrolling';
@@ -112,8 +112,19 @@ export class MatSidenav extends MatDrawer {
112112
},
113113
changeDetection: ChangeDetectionStrategy.OnPush,
114114
encapsulation: ViewEncapsulation.None,
115+
providers: [{
116+
provide: MAT_DRAWER_CONTAINER,
117+
useExisting: MatSidenavContainer
118+
}]
119+
115120
})
116121
export class MatSidenavContainer extends MatDrawerContainer {
117-
@ContentChildren(MatSidenav) _drawers: QueryList<MatSidenav>;
122+
@ContentChildren(MatSidenav, {
123+
// We need to use `descendants: true`, because Ivy will no longer match
124+
// indirect descendants if it's left as false.
125+
descendants: true
126+
})
127+
_allDrawers: QueryList<MatSidenav>;
128+
118129
@ContentChild(MatSidenavContent, {static: false}) _content: MatSidenavContent;
119130
}

0 commit comments

Comments
 (0)