Skip to content

Commit

Permalink
fix(sidenav): scrollable instance not exposed when explicitly specify…
Browse files Browse the repository at this point in the history
…ing content element

* Fixes the `MatSidenavContainer.scrollable` being undefined if the consumer has set the `mat-sidenav-content` themselves. The issue comes from the fact that we only query for scrollables inside the drawer's own view, but not inside the projected content.
* Fixes the example in the sidenav docs accessing the scrollable too early.

Fixes angular#10884.

BREAKING CHANGE: the constructor signature of the `MatDrawerContent` and `MatSidenavContent` has changed.
  • Loading branch information
crisbeto committed Jun 7, 2018
1 parent b349de3 commit a5eef4a
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 11 deletions.
2 changes: 1 addition & 1 deletion src/lib/sidenav/drawer-container.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@

<ng-content select="mat-drawer-content">
</ng-content>
<mat-drawer-content *ngIf="!_content" cdkScrollable>
<mat-drawer-content *ngIf="!_content">
<ng-content></ng-content>
</mat-drawer-content>
36 changes: 36 additions & 0 deletions src/lib/sidenav/drawer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {A11yModule} from '@angular/cdk/a11y';
import {PlatformModule} from '@angular/cdk/platform';
import {ESCAPE} from '@angular/cdk/keycodes';
import {dispatchKeyboardEvent} from '@angular/cdk/testing';
import {CdkScrollable} from '@angular/cdk/scrolling';


describe('MatDrawer', () => {
Expand Down Expand Up @@ -496,6 +497,7 @@ describe('MatDrawerContainer', () => {
DrawerContainerStateChangesTestApp,
AutosizeDrawer,
BasicTestApp,
DrawerContainerWithContent,
],
});

Expand Down Expand Up @@ -692,6 +694,27 @@ describe('MatDrawerContainer', () => {
expect(fixture.componentInstance.drawer.opened).toBe(false);
}));

it('should expose a scrollable when the consumer has not specified drawer content',
fakeAsync(() => {
const fixture = TestBed.createComponent(DrawerContainerTwoDrawerTestApp);

fixture.detectChanges();

expect(fixture.componentInstance.drawerContainer.scrollable instanceof CdkScrollable)
.toBe(true);
}));

it('should expose a scrollable when the consumer has specified drawer content',
fakeAsync(() => {
const fixture = TestBed.createComponent(DrawerContainerWithContent);

fixture.detectChanges();

expect(fixture.componentInstance.drawerContainer.scrollable instanceof CdkScrollable)
.toBe(true);
}));


});


Expand Down Expand Up @@ -874,3 +897,16 @@ class AutosizeDrawer {
@ViewChild(MatDrawer) drawer: MatDrawer;
fillerWidth = 0;
}


@Component({
template: `
<mat-drawer-container>
<mat-drawer>Drawer</mat-drawer>
<mat-drawer-content>Content</mat-drawer-content>
</mat-drawer-container>
`,
})
class DrawerContainerWithContent {
@ViewChild(MatDrawerContainer) drawerContainer: MatDrawerContainer;
}
15 changes: 11 additions & 4 deletions src/lib/sidenav/drawer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {Directionality} from '@angular/cdk/bidi';
import {coerceBooleanProperty} from '@angular/cdk/coercion';
import {ESCAPE} from '@angular/cdk/keycodes';
import {Platform} from '@angular/cdk/platform';
import {CdkScrollable} from '@angular/cdk/scrolling';
import {CdkScrollable, ScrollDispatcher} from '@angular/cdk/scrolling';
import {DOCUMENT} from '@angular/common';
import {
AfterContentChecked,
Expand Down Expand Up @@ -75,10 +75,14 @@ export function MAT_DRAWER_DEFAULT_AUTOSIZE_FACTORY(): boolean {
changeDetection: ChangeDetectionStrategy.OnPush,
encapsulation: ViewEncapsulation.None,
})
export class MatDrawerContent implements AfterContentInit {
export class MatDrawerContent extends CdkScrollable implements AfterContentInit {
constructor(
private _changeDetectorRef: ChangeDetectorRef,
@Inject(forwardRef(() => MatDrawerContainer)) public _container: MatDrawerContainer) {
@Inject(forwardRef(() => MatDrawerContainer)) public _container: MatDrawerContainer,
elementRef: ElementRef<HTMLElement>,
scrollDispatcher: ScrollDispatcher,
ngZone: NgZone) {
super(elementRef, scrollDispatcher, ngZone);
}

ngAfterContentInit() {
Expand Down Expand Up @@ -401,6 +405,7 @@ export class MatDrawer implements AfterContentInit, AfterContentChecked, OnDestr
export class MatDrawerContainer implements AfterContentInit, DoCheck, OnDestroy {
@ContentChildren(MatDrawer) _drawers: QueryList<MatDrawer>;
@ContentChild(MatDrawerContent) _content: MatDrawerContent;
@ViewChild(MatDrawerContent) _userContent: MatDrawerContent;

/** The drawer child with the `start` position. */
get start(): MatDrawer | null { return this._start; }
Expand Down Expand Up @@ -471,7 +476,9 @@ export class MatDrawerContainer implements AfterContentInit, DoCheck, OnDestroy
readonly _contentMarginChanges = new Subject<{left: number|null, right: number|null}>();

/** Reference to the CdkScrollable instance that wraps the scrollable content. */
@ViewChild(CdkScrollable) scrollable: CdkScrollable;
get scrollable(): CdkScrollable {
return this._userContent || this._content;
}

constructor(@Optional() private _dir: Directionality,
private _element: ElementRef,
Expand Down
8 changes: 4 additions & 4 deletions src/lib/sidenav/sidenav.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ The `<mat-sidenav>` can render in one of three different ways based on the `mode

| Mode | Description |
|--------|-----------------------------------------------------------------------------------------|
| `over` | Sidenav floats over the primary content, which is covered by a backdrop |
| `push` | Sidenav pushes the primary content out of its way, also covering it with a backdrop |
| `over` | Sidenav floats over the primary content, which is covered by a backdrop |
| `push` | Sidenav pushes the primary content out of its way, also covering it with a backdrop |
| `side` | Sidenav appears side-by-side with the main content, shrinking the main content's width to make space for the sidenav. |

If no `mode` is specified, `over` is used by default.
Expand Down Expand Up @@ -186,10 +186,10 @@ To react to scrolling inside the `<mat-sidenav-container>`, you can get a hold o
`CdkScrollable` instance through the `MatSidenavContainer`.

```ts
class YourComponent {
class YourComponent implements AfterViewInit {
@ViewChild(MatSidenavContainer) sidenavContainer: MatSidenavContainer;

constructor() {
ngAfterViewInit() {
this.sidenavContainer.scrollable.elementScrolled().subscribe(() => /* react to scrolling */);
}
}
Expand Down
10 changes: 8 additions & 2 deletions src/lib/sidenav/sidenav.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,13 @@ import {
Input,
ViewEncapsulation,
QueryList,
ElementRef,
NgZone,
} from '@angular/core';
import {MatDrawer, MatDrawerContainer, MatDrawerContent} from './drawer';
import {matDrawerAnimations} from './drawer-animations';
import {coerceBooleanProperty, coerceNumberProperty} from '@angular/cdk/coercion';
import {ScrollDispatcher} from '@angular/cdk/scrolling';


@Component({
Expand All @@ -38,8 +41,11 @@ import {coerceBooleanProperty, coerceNumberProperty} from '@angular/cdk/coercion
export class MatSidenavContent extends MatDrawerContent {
constructor(
changeDetectorRef: ChangeDetectorRef,
@Inject(forwardRef(() => MatSidenavContainer)) container: MatSidenavContainer) {
super(changeDetectorRef, container);
@Inject(forwardRef(() => MatSidenavContainer)) container: MatSidenavContainer,
elementRef: ElementRef<HTMLElement>,
scrollDispatcher: ScrollDispatcher,
ngZone: NgZone) {
super(changeDetectorRef, container, elementRef, scrollDispatcher, ngZone);
}
}

Expand Down

0 comments on commit a5eef4a

Please sign in to comment.