Skip to content

Commit

Permalink
fix(material/tabs): avoid timeouts in background tabs
Browse files Browse the repository at this point in the history
The tabs have a couple of `requestAnimationFrame` calls when checking for the size/position of elements which can cause tests to hang when they're in a background tab. The problem is that browsers block `requestAnimationFrame` when the browser is out of focus and test harnesses will wait for the call to resolve.

These changes switch to `NgZone.onStable` in an attempt to resolve the issue.

Fixes angular#23964.
  • Loading branch information
crisbeto committed Mar 9, 2022
1 parent f91b98f commit 6ec3e48
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 55 deletions.
4 changes: 3 additions & 1 deletion src/material-experimental/mdc-tabs/ink-bar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,9 @@ export class MatInkBarFoundation {
}
},
setContentStyleProperty: (propName, value) => {
this._inkBarContentElement.style.setProperty(propName, value);
if (!this._destroyed) {
this._inkBarContentElement.style.setProperty(propName, value);
}
},
computeContentClientRect: () => {
// `getBoundingClientRect` isn't available on the server.
Expand Down
22 changes: 6 additions & 16 deletions src/material-experimental/mdc-tabs/tab-header.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {Direction, Directionality} from '@angular/cdk/bidi';
import {Direction} from '@angular/cdk/bidi';
import {END, ENTER, HOME, LEFT_ARROW, RIGHT_ARROW, SPACE} from '@angular/cdk/keycodes';
import {PortalModule} from '@angular/cdk/portal';
import {ScrollingModule, ViewportRuler} from '@angular/cdk/scrolling';
Expand All @@ -23,25 +23,18 @@ import {MatRippleModule} from '@angular/material-experimental/mdc-core';
import {By} from '@angular/platform-browser';
import {MatTabHeader} from './tab-header';
import {MatTabLabelWrapper} from './tab-label-wrapper';
import {Subject} from 'rxjs';
import {ObserversModule, MutationObserverFactory} from '@angular/cdk/observers';

describe('MDC-based MatTabHeader', () => {
let dir: Direction = 'ltr';
let change = new Subject();
let fixture: ComponentFixture<SimpleTabHeaderApp>;
let appComponent: SimpleTabHeaderApp;

beforeEach(
waitForAsync(() => {
dir = 'ltr';
TestBed.configureTestingModule({
imports: [CommonModule, PortalModule, MatRippleModule, ScrollingModule, ObserversModule],
declarations: [MatTabHeader, MatTabLabelWrapper, SimpleTabHeaderApp],
providers: [
ViewportRuler,
{provide: Directionality, useFactory: () => ({value: dir, change: change})},
],
providers: [ViewportRuler],
});

TestBed.compileComponents();
Expand Down Expand Up @@ -238,11 +231,10 @@ describe('MDC-based MatTabHeader', () => {
describe('pagination', () => {
describe('ltr', () => {
beforeEach(() => {
dir = 'ltr';
fixture = TestBed.createComponent(SimpleTabHeaderApp);
fixture.detectChanges();

appComponent = fixture.componentInstance;
appComponent.dir = 'ltr';
fixture.detectChanges();
});

it('should show width when tab list width exceeds container', () => {
Expand Down Expand Up @@ -322,11 +314,9 @@ describe('MDC-based MatTabHeader', () => {

describe('rtl', () => {
beforeEach(() => {
dir = 'rtl';
fixture = TestBed.createComponent(SimpleTabHeaderApp);
appComponent = fixture.componentInstance;
appComponent.dir = 'rtl';

fixture.detectChanges();
});

Expand Down Expand Up @@ -607,9 +597,9 @@ describe('MDC-based MatTabHeader', () => {

fixture.detectChanges();

change.next();
fixture.componentInstance.dir = 'rtl';
fixture.detectChanges();
tick(20); // Angular turns rAF calls into 16.6ms timeouts in tests.
tick();

expect(inkBar.alignToElement).toHaveBeenCalled();
}));
Expand Down
27 changes: 7 additions & 20 deletions src/material/tabs/ink-bar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import {Directive, ElementRef, Inject, InjectionToken, NgZone, Optional} from '@angular/core';
import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations';
import {take} from 'rxjs/operators';

/**
* Interface for a a MatInkBar positioner method, defining the positioning and width of the ink
Expand Down Expand Up @@ -65,14 +66,12 @@ export class MatInkBar {
*/
alignToElement(element: HTMLElement) {
this.show();

if (typeof requestAnimationFrame !== 'undefined') {
this._ngZone.runOutsideAngular(() => {
requestAnimationFrame(() => this._setStyles(element));
});
} else {
this._setStyles(element);
}
this._ngZone.onStable.pipe(take(1)).subscribe(() => {
const positions = this._inkBarPositioner(element);
const inkBar: HTMLElement = this._elementRef.nativeElement;
inkBar.style.left = positions.left;
inkBar.style.width = positions.width;
});
}

/** Shows the ink bar. */
Expand All @@ -84,16 +83,4 @@ export class MatInkBar {
hide(): void {
this._elementRef.nativeElement.style.visibility = 'hidden';
}

/**
* Sets the proper styles to the ink bar element.
* @param element
*/
private _setStyles(element: HTMLElement) {
const positions = this._inkBarPositioner(element);
const inkBar: HTMLElement = this._elementRef.nativeElement;

inkBar.style.left = positions.left;
inkBar.style.width = positions.width;
}
}
6 changes: 4 additions & 2 deletions src/material/tabs/paginated-tab-header.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import {ViewportRuler} from '@angular/cdk/scrolling';
import {FocusKeyManager, FocusableOption} from '@angular/cdk/a11y';
import {ENTER, SPACE, hasModifierKey} from '@angular/cdk/keycodes';
import {merge, of as observableOf, Subject, timer, fromEvent} from 'rxjs';
import {takeUntil} from 'rxjs/operators';
import {take, takeUntil} from 'rxjs/operators';
import {Platform, normalizePassiveListenerOptions} from '@angular/cdk/platform';
import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations';

Expand Down Expand Up @@ -212,7 +212,9 @@ export abstract class MatPaginatedTabHeader

// Defer the first call in order to allow for slower browsers to lay out the elements.
// This helps in cases where the user lands directly on a page with paginated tabs.
typeof requestAnimationFrame !== 'undefined' ? requestAnimationFrame(realign) : realign();
// Note that we use `onStable` instead of `requestAnimationFrame`, because the latter
// can hold up tests that are in a background tab.
this._ngZone.onStable.pipe(take(1)).subscribe(realign);

// On dir change or window resize, realign the ink bar and update the orientation of
// the key manager if the direction has changed.
Expand Down
22 changes: 6 additions & 16 deletions src/material/tabs/tab-header.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {Direction, Directionality} from '@angular/cdk/bidi';
import {Direction} from '@angular/cdk/bidi';
import {END, ENTER, HOME, LEFT_ARROW, RIGHT_ARROW, SPACE} from '@angular/cdk/keycodes';
import {PortalModule} from '@angular/cdk/portal';
import {ScrollingModule, ViewportRuler} from '@angular/cdk/scrolling';
Expand All @@ -24,25 +24,18 @@ import {By} from '@angular/platform-browser';
import {MatInkBar} from './ink-bar';
import {MatTabHeader} from './tab-header';
import {MatTabLabelWrapper} from './tab-label-wrapper';
import {Subject} from 'rxjs';
import {ObserversModule, MutationObserverFactory} from '@angular/cdk/observers';

describe('MatTabHeader', () => {
let dir: Direction = 'ltr';
let change = new Subject();
let fixture: ComponentFixture<SimpleTabHeaderApp>;
let appComponent: SimpleTabHeaderApp;

beforeEach(
waitForAsync(() => {
dir = 'ltr';
TestBed.configureTestingModule({
imports: [CommonModule, PortalModule, MatRippleModule, ScrollingModule, ObserversModule],
declarations: [MatTabHeader, MatInkBar, MatTabLabelWrapper, SimpleTabHeaderApp],
providers: [
ViewportRuler,
{provide: Directionality, useFactory: () => ({value: dir, change: change})},
],
providers: [ViewportRuler],
});

TestBed.compileComponents();
Expand Down Expand Up @@ -239,11 +232,10 @@ describe('MatTabHeader', () => {
describe('pagination', () => {
describe('ltr', () => {
beforeEach(() => {
dir = 'ltr';
fixture = TestBed.createComponent(SimpleTabHeaderApp);
fixture.detectChanges();

appComponent = fixture.componentInstance;
appComponent.dir = 'ltr';
fixture.detectChanges();
});

it('should show width when tab list width exceeds container', () => {
Expand Down Expand Up @@ -319,11 +311,9 @@ describe('MatTabHeader', () => {

describe('rtl', () => {
beforeEach(() => {
dir = 'rtl';
fixture = TestBed.createComponent(SimpleTabHeaderApp);
appComponent = fixture.componentInstance;
appComponent.dir = 'rtl';

fixture.detectChanges();
});

Expand Down Expand Up @@ -603,9 +593,9 @@ describe('MatTabHeader', () => {

fixture.detectChanges();

change.next();
fixture.componentInstance.dir = 'rtl';
fixture.detectChanges();
tick(20); // Angular turns rAF calls into 16.6ms timeouts in tests.
tick();

expect(inkBar.alignToElement).toHaveBeenCalled();
}));
Expand Down

0 comments on commit 6ec3e48

Please sign in to comment.