Skip to content

Commit 34f27f5

Browse files
crisbetocrisbeto
committed
fix(sidenav): container not picking up indirect descendant sidenavs
Fixes the sidenav container not picking up indirect descendant sidenavs.
1 parent ef92091 commit 34f27f5

File tree

6 files changed

+211
-19
lines changed

6 files changed

+211
-19
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: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,12 @@ export class MatDrawer implements AfterContentInit, AfterContentChecked, OnDestr
246246
private _focusMonitor: FocusMonitor,
247247
private _platform: Platform,
248248
private _ngZone: NgZone,
249-
@Optional() @Inject(DOCUMENT) private _doc: any) {
249+
@Optional() @Inject(DOCUMENT) private _doc: any,
250+
/**
251+
* @deprecated `_container` parameter to be made required.
252+
* @breaking-change 10.0.0
253+
*/
254+
@Optional() public _container?: MatDrawerContainer) {
250255

251256
this.openedChange.subscribe((opened: boolean) => {
252257
if (opened) {
@@ -461,7 +466,17 @@ export class MatDrawer implements AfterContentInit, AfterContentChecked, OnDestr
461466
encapsulation: ViewEncapsulation.None,
462467
})
463468
export class MatDrawerContainer implements AfterContentInit, DoCheck, OnDestroy {
464-
@ContentChildren(MatDrawer) _drawers: QueryList<MatDrawer>;
469+
/** All drawers in the container. Includes drawers from inside nested containers. */
470+
@ContentChildren(MatDrawer, {
471+
// We need to use `descendants: true`, because Ivy will no longer match
472+
// indirect descendants if it's left as false.
473+
descendants: true
474+
})
475+
_allDrawers: QueryList<MatDrawer>;
476+
477+
/** Drawers that belong to this container. */
478+
_drawers = new QueryList<MatDrawer>();
479+
465480
@ContentChild(MatDrawerContent, {static: false}) _content: MatDrawerContent;
466481
@ViewChild(MatDrawerContent, {static: false}) _userContent: MatDrawerContent;
467482

@@ -565,6 +580,14 @@ export class MatDrawerContainer implements AfterContentInit, DoCheck, OnDestroy
565580
}
566581

567582
ngAfterContentInit() {
583+
this._allDrawers.changes
584+
.pipe(startWith(this._allDrawers), takeUntil(this._destroyed))
585+
.subscribe((drawer: QueryList<MatDrawer>) => {
586+
// @breaking-change 10.0.0 Remove `_container` check once container parameter is required.
587+
this._drawers.reset(drawer.filter(item => !item._container || item._container === this));
588+
this._drawers.notifyOnChanges();
589+
});
590+
568591
this._drawers.changes.pipe(startWith(null)).subscribe(() => {
569592
this._validateDrawers();
570593

‎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: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,18 @@ export class MatSidenav extends MatDrawer {
112112
},
113113
changeDetection: ChangeDetectionStrategy.OnPush,
114114
encapsulation: ViewEncapsulation.None,
115+
providers: [{
116+
provide: MatDrawerContainer,
117+
useExisting: MatSidenavContainer
118+
}]
115119
})
116120
export class MatSidenavContainer extends MatDrawerContainer {
117-
@ContentChildren(MatSidenav) _drawers: QueryList<MatSidenav>;
121+
@ContentChildren(MatSidenav, {
122+
// We need to use `descendants: true`, because Ivy will no longer match
123+
// indirect descendants if it's left as false.
124+
descendants: true
125+
})
126+
_allDrawers: QueryList<MatSidenav>;
127+
118128
@ContentChild(MatSidenavContent, {static: false}) _content: MatSidenavContent;
119129
}

‎tools/public_api_guard/material/sidenav.d.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ export declare class MatDrawer implements AfterContentInit, AfterContentChecked,
77
_animationStarted: Subject<AnimationEvent>;
88
_animationState: 'open-instant' | 'open' | 'void';
99
readonly _closedStream: Observable<void>;
10+
_container?: MatDrawerContainer | undefined;
1011
readonly _isFocusTrapEnabled: boolean;
1112
readonly _modeChanged: Subject<void>;
1213
readonly _openedStream: Observable<void>;
@@ -20,7 +21,8 @@ export declare class MatDrawer implements AfterContentInit, AfterContentChecked,
2021
readonly openedChange: EventEmitter<boolean>;
2122
readonly openedStart: Observable<void>;
2223
position: 'start' | 'end';
23-
constructor(_elementRef: ElementRef<HTMLElement>, _focusTrapFactory: FocusTrapFactory, _focusMonitor: FocusMonitor, _platform: Platform, _ngZone: NgZone, _doc: any);
24+
constructor(_elementRef: ElementRef<HTMLElement>, _focusTrapFactory: FocusTrapFactory, _focusMonitor: FocusMonitor, _platform: Platform, _ngZone: NgZone, _doc: any,
25+
_container?: MatDrawerContainer | undefined);
2426
_animationDoneListener(event: AnimationEvent): void;
2527
_animationStartListener(event: AnimationEvent): void;
2628
close(): Promise<MatDrawerToggleResult>;
@@ -36,6 +38,7 @@ export declare const matDrawerAnimations: {
3638
};
3739

3840
export declare class MatDrawerContainer implements AfterContentInit, DoCheck, OnDestroy {
41+
_allDrawers: QueryList<MatDrawer>;
3942
_backdropOverride: boolean | null;
4043
_content: MatDrawerContent;
4144
readonly _contentMarginChanges: Subject<{
@@ -81,8 +84,8 @@ export declare class MatSidenav extends MatDrawer {
8184
}
8285

8386
export declare class MatSidenavContainer extends MatDrawerContainer {
87+
_allDrawers: QueryList<MatSidenav>;
8488
_content: MatSidenavContent;
85-
_drawers: QueryList<MatSidenav>;
8689
}
8790

8891
export declare class MatSidenavContent extends MatDrawerContent {

0 commit comments

Comments
 (0)