Skip to content

fix(material/tabs): avoid timeouts in background tabs #24000

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 1 commit into from
Mar 10, 2022
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
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();
Copy link
Member Author

Choose a reason for hiding this comment

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

I deleted this, because it wasn't actually doing anything. There's a dir directive instance in the test which supersedes it.

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