Skip to content

Commit

Permalink
fix(breakpoint-observer): fix the breakpoint observer emit count and …
Browse files Browse the repository at this point in the history
…accuracy (#15964)

The breakpoint observer emits multiple and incorrect states when more than one query changes.
Debounce the observer emissions to eliminate the incorrect states and emit once.

References #10925
  • Loading branch information
TrevorKarjanis authored and andrewseguin committed Jun 26, 2019
1 parent 453b1e8 commit bb66df7
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 25 deletions.
42 changes: 32 additions & 10 deletions src/cdk/layout/breakpoints-observer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@
import {LayoutModule} from './layout-module';
import {BreakpointObserver, BreakpointState} from './breakpoints-observer';
import {MediaMatcher} from './media-matcher';
import {fakeAsync, TestBed, inject, flush} from '@angular/core/testing';
import {fakeAsync, TestBed, inject, flush, tick} from '@angular/core/testing';
import {Injectable} from '@angular/core';
import {Subscription} from 'rxjs';
import {take} from 'rxjs/operators';
import {skip, take} from 'rxjs/operators';

describe('BreakpointObserver', () => {
let breakpointObserver: BreakpointObserver;
Expand Down Expand Up @@ -93,10 +93,10 @@ describe('BreakpointObserver', () => {
queryMatchState = state.matches;
});

flush();
tick();
expect(queryMatchState).toBeTruthy();
mediaMatcher.setMatchesQuery(query, false);
flush();
tick();
expect(queryMatchState).toBeFalsy();
}));

Expand All @@ -108,36 +108,54 @@ describe('BreakpointObserver', () => {
breakpointObserver.observe([queryOne, queryTwo]).subscribe((breakpoint: BreakpointState) => {
state = breakpoint;
});
expect(state.breakpoints).toEqual({[queryOne]: true, [queryTwo]: true});

mediaMatcher.setMatchesQuery(queryOne, false);
mediaMatcher.setMatchesQuery(queryTwo, false);
flush();
tick();
expect(state.breakpoints).toEqual({[queryOne]: false, [queryTwo]: false});

mediaMatcher.setMatchesQuery(queryOne, true);
mediaMatcher.setMatchesQuery(queryTwo, false);
flush();
tick();
expect(state.breakpoints).toEqual({[queryOne]: true, [queryTwo]: false});
}));

it('emits a true matches state when the query is matched', fakeAsync(() => {
const query = '(width: 999px)';
breakpointObserver.observe(query).subscribe();
mediaMatcher.setMatchesQuery(query, true);
tick();
expect(breakpointObserver.isMatched(query)).toBeTruthy();
}));

it('emits a false matches state when the query is not matched', fakeAsync(() => {
const query = '(width: 999px)';
breakpointObserver.observe(query).subscribe();
mediaMatcher.setMatchesQuery(query, false);
tick();
expect(breakpointObserver.isMatched(query)).toBeFalsy();
}));

it('emits one event when multiple queries change', fakeAsync(() => {
const observer = jasmine.createSpy('observer');
const queryOne = '(width: 700px)';
const queryTwo = '(width: 999px)';
breakpointObserver.observe([queryOne, queryTwo])
.pipe(skip(1))
.subscribe(observer);

mediaMatcher.setMatchesQuery(queryOne, false);
mediaMatcher.setMatchesQuery(queryTwo, false);

tick();
expect(observer).toHaveBeenCalledTimes(1);
}));

it('should not complete other subscribers when preceding subscriber completes', fakeAsync(() => {
const queryOne = '(width: 700px)';
const queryTwo = '(width: 999px)';
const breakpoint = breakpointObserver.observe([queryOne, queryTwo]);
const breakpoint = breakpointObserver.observe([queryOne, queryTwo]).pipe(skip(1));
const subscriptions: Subscription[] = [];
let emittedValues: number[] = [];

Expand All @@ -148,14 +166,14 @@ describe('BreakpointObserver', () => {

mediaMatcher.setMatchesQuery(queryOne, true);
mediaMatcher.setMatchesQuery(queryTwo, false);
flush();
tick();

expect(emittedValues).toEqual([1, 2, 3, 4]);
emittedValues = [];

mediaMatcher.setMatchesQuery(queryOne, false);
mediaMatcher.setMatchesQuery(queryTwo, true);
flush();
tick();

expect(emittedValues).toEqual([1, 3, 4]);

Expand All @@ -172,7 +190,11 @@ export class FakeMediaQueryList {
/** Toggles the matches state and "emits" a change event. */
setMatches(matches: boolean) {
this.matches = matches;
this._listeners.forEach(listener => listener(this as any));

/** Simulate an asynchronous task. */
setTimeout(() => {
this._listeners.forEach(listener => listener(this as any));
});
}

/** Registers a callback method for change events. */
Expand Down
33 changes: 18 additions & 15 deletions src/cdk/layout/breakpoints-observer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@

import {Injectable, NgZone, OnDestroy} from '@angular/core';
import {MediaMatcher} from './media-matcher';
import {asapScheduler, combineLatest, Observable, Subject, Observer} from 'rxjs';
import {debounceTime, map, startWith, takeUntil} from 'rxjs/operators';
import {combineLatest, concat, Observable, Subject, Observer} from 'rxjs';
import {debounceTime, map, skip, startWith, take, takeUntil} from 'rxjs/operators';
import {coerceArray} from '@angular/cdk/coercion';


Expand Down Expand Up @@ -75,19 +75,22 @@ export class BreakpointObserver implements OnDestroy {
const queries = splitQueries(coerceArray(value));
const observables = queries.map(query => this._registerQuery(query).observable);

return combineLatest(observables).pipe(
debounceTime(0, asapScheduler),
map((breakpointStates: InternalBreakpointState[]) => {
const response: BreakpointState = {
matches: false,
breakpoints: {},
};
breakpointStates.forEach((state: InternalBreakpointState) => {
response.matches = response.matches || state.matches;
response.breakpoints[state.query] = state.matches;
});
return response;
}));
let stateObservable = combineLatest(observables);
// Emit the first state immediately, and then debounce the subsequent emissions.
stateObservable = concat(
stateObservable.pipe(take(1)),
stateObservable.pipe(skip(1), debounceTime(0)));
return stateObservable.pipe(map((breakpointStates: InternalBreakpointState[]) => {
const response: BreakpointState = {
matches: false,
breakpoints: {},
};
breakpointStates.forEach((state: InternalBreakpointState) => {
response.matches = response.matches || state.matches;
response.breakpoints[state.query] = state.matches;
});
return response;
}));
}

/** Registers a specific query to be listened for. */
Expand Down

0 comments on commit bb66df7

Please sign in to comment.