Skip to content

Commit

Permalink
fix(tabs): crashing on chrome under certain conditions (#2411)
Browse files Browse the repository at this point in the history
* fix(tabs): crashing on chrome under certain conditions

Prevents the tabs from either crashing Chrome (in Angular < 2.3) or throwing an animation error (in Angular >= 2.3). This was happening when the animations get triggered before the element is inserted into the DOM and thus doesn't have a computed `transform` value.

Fixes #2151.

* Fix IE errors.

* Add a TODO.

* Revert.

* Add TODO for removing the workaround.
  • Loading branch information
crisbeto authored and mmalerba committed Jan 18, 2017
1 parent 9e99374 commit 727ce53
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 5 deletions.
2 changes: 1 addition & 1 deletion src/lib/tabs/tab-body.html
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<div class="md-tab-body-content" #content
[@translateTab]="_position"
[@translateTab]="_canBeAnimated ? _position : null"
(@translateTab.start)="_onTranslateTabStarted($event)"
(@translateTab.done)="_onTranslateTabComplete($event)">
<template cdkPortalHost></template>
Expand Down
14 changes: 14 additions & 0 deletions src/lib/tabs/tab-body.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,20 @@ describe('MdTabBody', () => {
expect(fixture.componentInstance.mdTabBody._portalHost.hasAttached()).toBe(false);
}));
});

it('it should toggle the canBeAnimated flag', () => {
let fixture: ComponentFixture<SimpleTabBodyApp>;
let tabBody: MdTabBody;

fixture = TestBed.createComponent(SimpleTabBodyApp);
tabBody = fixture.componentInstance.mdTabBody;

expect(tabBody._canBeAnimated).toBe(false);

fixture.detectChanges();

expect(tabBody._canBeAnimated).toBe(true);
});
});


Expand Down
39 changes: 35 additions & 4 deletions src/lib/tabs/tab-body.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ import {
transition,
AnimationTransitionEvent,
ElementRef,
Optional
Optional,
ChangeDetectorRef,
AfterViewChecked,
AfterContentChecked,
} from '@angular/core';
import {TemplatePortal, PortalHostDirective, Dir, LayoutDirection} from '../core';
import 'rxjs/add/operator/map';
Expand Down Expand Up @@ -65,7 +68,7 @@ export type MdTabBodyOriginState = 'left' | 'right';
])
]
})
export class MdTabBody implements OnInit {
export class MdTabBody implements OnInit, AfterViewChecked, AfterContentChecked {
/** The portal host inside of this container into which the tab body content will be loaded. */
@ViewChild(PortalHostDirective) _portalHost: PortalHostDirective;

Expand All @@ -92,6 +95,10 @@ export class MdTabBody implements OnInit {
}
}

/** Whether the element is allowed to be animated. */
_canBeAnimated: boolean = false;

/** The origin position from which this tab should appear when it is centered into view. */
_origin: MdTabBodyOriginState;

/** The origin position from which this tab should appear when it is centered into view. */
Expand All @@ -106,7 +113,10 @@ export class MdTabBody implements OnInit {
}
}

constructor(private _elementRef: ElementRef, @Optional() private _dir: Dir) {}
constructor(
@Optional() private _dir: Dir,
private _elementRef: ElementRef,
private _changeDetectorRef: ChangeDetectorRef) { }

/**
* After initialized, check if the content is centered and has an origin. If so, set the
Expand All @@ -128,6 +138,28 @@ export class MdTabBody implements OnInit {
}
}

/**
* After the content has been checked, determines whether the element should be allowed to
* animate. This has to be limited, because under a specific set of circumstances (see #2151),
* the animations can be triggered too early, which either crashes Chrome by putting it into an
* infinite loop (with Angular < 2.3.0) or throws an error because the element doesn't have a
* computed style (with Angular > 2.3.0). This can alternatively be determined by checking the
* transform: canBeAnimated = getComputedStyle(element) !== '', however document.contains should
* be faster since it doesn't cause a reflow.
*
* TODO: This can safely be removed after we stop supporting Angular < 2.4.2. The fix landed via
* https://github.com/angular/angular/commit/21030e9a1cf30e8101399d8535ed72d847a23ba6
*/
ngAfterContentChecked() {
if (!this._canBeAnimated) {
this._canBeAnimated = document.body.contains(this._elementRef.nativeElement);

if (this._canBeAnimated) {
this._changeDetectorRef.markForCheck();
}
}
}

_onTranslateTabStarted(e: AnimationTransitionEvent) {
if (this._isCenterPosition(e.toState)) {
this.onCentering.emit(this._elementRef.nativeElement.clientHeight);
Expand All @@ -151,7 +183,6 @@ export class MdTabBody implements OnInit {
return this._dir && this._dir.value === 'rtl' ? 'rtl' : 'ltr';
}


/** Whether the provided position state is considered center, regardless of origin. */
private _isCenterPosition(position: MdTabBodyPositionState|string): boolean {
return position == 'center' ||
Expand Down

0 comments on commit 727ce53

Please sign in to comment.