Skip to content

fix(drawer): drawer container animating when open by default #7129

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/lib/sidenav/drawer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@ describe('MatDrawerContainer', () => {
declarations: [
DrawerContainerTwoDrawerTestApp,
DrawerDelayed,
DrawerSetToOpenedTrue,
DrawerContainerStateChangesTestApp,
],
});
Expand Down Expand Up @@ -417,6 +418,17 @@ describe('MatDrawerContainer', () => {
expect(parseInt(contentElement.style.marginLeft)).toBeLessThan(initialMargin);
}));

it('should not animate when the sidenav is open on load ', fakeAsync(() => {
const fixture = TestBed.createComponent(DrawerSetToOpenedTrue);

fixture.detectChanges();
tick();

const container = fixture.debugElement.nativeElement.querySelector('.mat-drawer-container');

expect(container.classList).not.toContain('mat-drawer-transition');
}));

});


Expand Down
36 changes: 20 additions & 16 deletions src/lib/sidenav/drawer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,8 @@ import {
} from '@angular/core';
import {DOCUMENT} from '@angular/platform-browser';
import {merge} from 'rxjs/observable/merge';
import {first} from 'rxjs/operator/first';
import {startWith} from 'rxjs/operator/startWith';
import {takeUntil} from 'rxjs/operator/takeUntil';
import {Subject} from 'rxjs/Subject';
import {RxChain, filter, first, startWith, takeUntil} from '@angular/cdk/rxjs';


/** Throws an exception when two MatDrawer are matching the same position. */
Expand Down Expand Up @@ -117,7 +115,7 @@ export class MatDrawerContent implements AfterContentInit {
host: {
'class': 'mat-drawer',
'[@transform]': '_animationState',
'(@transform.start)': '_onAnimationStart()',
'(@transform.start)': '_onAnimationStart($event)',
'(@transform.done)': '_onAnimationEnd($event)',
'(keydown)': 'handleKeydown($event)',
// must prevent the browser from aligning text based on value
Expand Down Expand Up @@ -177,7 +175,7 @@ export class MatDrawer implements AfterContentInit, OnDestroy {
private _opened: boolean = false;

/** Emits whenever the drawer has started animating. */
_animationStarted = new EventEmitter<void>();
_animationStarted = new EventEmitter<AnimationEvent>();

/** Whether the drawer is animating. Used to prevent overlapping animations. */
_isAnimating = false;
Expand Down Expand Up @@ -320,9 +318,9 @@ export class MatDrawer implements AfterContentInit, OnDestroy {
}
}

_onAnimationStart() {
_onAnimationStart(event: AnimationEvent) {
this._isAnimating = true;
this._animationStarted.emit();
this._animationStarted.emit(event);
}

_onAnimationEnd(event: AnimationEvent) {
Expand Down Expand Up @@ -453,13 +451,19 @@ export class MatDrawerContainer implements AfterContentInit, OnDestroy {
* is properly hidden.
*/
private _watchDrawerToggle(drawer: MatDrawer): void {
takeUntil.call(drawer._animationStarted, this._drawers.changes).subscribe(() => {
// Set the transition class on the container so that the animations occur. This should not
// be set initially because animations should only be triggered via a change in state.
this._renderer.addClass(this._element.nativeElement, 'mat-drawer-transition');
this._updateContentMargins();
this._changeDetectorRef.markForCheck();
});
RxChain.from(drawer._animationStarted)
.call(takeUntil, this._drawers.changes)
.call(filter, (event: AnimationEvent) => event.fromState !== event.toState)
.subscribe((event: AnimationEvent) => {
// Set the transition class on the container so that the animations occur. This should not
// be set initially because animations should only be triggered via a change in state.
if (event.toState !== 'open-instant') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this._element.nativeElement.classList.toggle('mat-drawer-transition', event.toState != 'open-instant');

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second param isn’t supported on IE.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, in that case can we still at least use classList.add instead of the Renderer

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The renderer is SSR-friendly. AFAIK we can stop using it in Angular 5.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't animation events not fire on the server?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK they could fire if the sidenav starts off opened.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm ok, just needs rebase then

this._renderer.addClass(this._element.nativeElement, 'mat-drawer-transition');
}

this._updateContentMargins();
this._changeDetectorRef.markForCheck();
});

if (drawer.mode !== 'side') {
takeUntil.call(merge(drawer.onOpen, drawer.onClose), this._drawers.changes).subscribe(() =>
Expand All @@ -468,8 +472,8 @@ export class MatDrawerContainer implements AfterContentInit, OnDestroy {
}

/**
* Subscribes to drawer onPositionChanged event in order to re-validate drawers when the position
* changes.
* Subscribes to drawer onPositionChanged event in order to
* re-validate drawers when the position changes.
*/
private _watchDrawerPosition(drawer: MatDrawer): void {
if (!drawer) {
Expand Down
2 changes: 1 addition & 1 deletion src/lib/sidenav/sidenav.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export class MatSidenavContent extends MatDrawerContent {
'class': 'mat-drawer mat-sidenav',
'tabIndex': '-1',
'[@transform]': '_animationState',
'(@transform.start)': '_onAnimationStart()',
'(@transform.start)': '_onAnimationStart($event)',
'(@transform.done)': '_onAnimationEnd($event)',
'(keydown)': 'handleKeydown($event)',
// must prevent the browser from aligning text based on value
Expand Down