Skip to content
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

fix(cdk/layout): support readonly arrays for public methods #20252

Merged
merged 2 commits into from
Aug 19, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 15 additions & 15 deletions src/cdk/layout/breakpoints-observer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@
* found in the LICENSE file at https://angular.io/license
*/

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


/** The current state of a layout breakpoint. */
Expand Down Expand Up @@ -60,7 +60,7 @@ export class BreakpointObserver implements OnDestroy {
* @param value One or more media queries to check.
* @returns Whether any of the media queries match.
*/
isMatched(value: string | string[]): boolean {
isMatched(value: string | readonly string[]): boolean {
const queries = splitQueries(coerceArray(value));
return queries.some(mediaQuery => this._registerQuery(mediaQuery).mql.matches);
}
Expand All @@ -71,7 +71,7 @@ export class BreakpointObserver implements OnDestroy {
* @param value One or more media queries to check.
* @returns A stream of matches for the given queries.
*/
observe(value: string | string[]): Observable<BreakpointState> {
observe(value: string | readonly string[]): Observable<BreakpointState> {
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be ReadonlyArray<string>?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ReadonlyArray<string> and readonly string[] have exactly the same meaning. I can adjust if the ReadonlyArray<> style is more adopted in the repo.

PS: It would be nice to pick one of the styles and use it consistently. In angular repo someone created an issue relating this.

Copy link
Member

Choose a reason for hiding this comment

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

I do prefer readonly string[] myself

const queries = splitQueries(coerceArray(value));
const observables = queries.map(query => this._registerQuery(query).observable);

Expand All @@ -80,14 +80,14 @@ export class BreakpointObserver implements OnDestroy {
stateObservable = concat(
stateObservable.pipe(take(1)),
stateObservable.pipe(skip(1), debounceTime(0)));
return stateObservable.pipe(map((breakpointStates: InternalBreakpointState[]) => {
return stateObservable.pipe(map(breakpointStates => {
const response: BreakpointState = {
matches: false,
breakpoints: {},
};
breakpointStates.forEach((state: InternalBreakpointState) => {
response.matches = response.matches || state.matches;
response.breakpoints[state.query] = state.matches;
breakpointStates.forEach(({matches, query}) => {
response.matches = response.matches || matches;
response.breakpoints[query] = matches;
});
return response;
}));
Expand All @@ -100,10 +100,10 @@ export class BreakpointObserver implements OnDestroy {
return this._queries.get(query)!;
}

const mql: MediaQueryList = this._mediaMatcher.matchMedia(query);
const mql = this._mediaMatcher.matchMedia(query);

// Create callback for match changes and add it is as a listener.
const queryObservable = new Observable<MediaQueryList>((observer: Observer<MediaQueryList>) => {
const queryObservable = new Observable((observer: Observer<MediaQueryList>) => {
// Listener callback methods are wrapped to be placed back in ngZone. Callbacks must be placed
// back into the zone because matchMedia is only included in Zone.js by loading the
// webapis-media-query.js file alongside the zone.js file. Additionally, some browsers do not
Expand All @@ -117,7 +117,7 @@ export class BreakpointObserver implements OnDestroy {
};
}).pipe(
startWith(mql),
map((nextMql: MediaQueryList) => ({query, matches: nextMql.matches})),
map(({matches}) => ({query, matches})),
takeUntil(this._destroySubject)
);

Expand All @@ -132,8 +132,8 @@ export class BreakpointObserver implements OnDestroy {
* Split each query string into separate query strings if two queries are provided as comma
* separated.
*/
function splitQueries(queries: string[]): string[] {
return queries.map((query: string) => query.split(','))
.reduce((a1: string[], a2: string[]) => a1.concat(a2))
function splitQueries(queries: readonly string[]): readonly string[] {
return queries.map(query => query.split(','))
.reduce((a1, a2) => a1.concat(a2))
.map(query => query.trim());
}
4 changes: 2 additions & 2 deletions tools/public_api_guard/cdk/layout.d.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
export declare class BreakpointObserver implements OnDestroy {
constructor(_mediaMatcher: MediaMatcher, _zone: NgZone);
isMatched(value: string | string[]): boolean;
isMatched(value: string | readonly string[]): boolean;
ngOnDestroy(): void;
observe(value: string | string[]): Observable<BreakpointState>;
observe(value: string | readonly string[]): Observable<BreakpointState>;
static ɵfac: i0.ɵɵFactoryDef<BreakpointObserver, never>;
static ɵprov: i0.ɵɵInjectableDef<BreakpointObserver>;
}
Expand Down