Skip to content

Commit

Permalink
fix(ObservableMedia): provide consistent reporting of active breakpoint
Browse files Browse the repository at this point in the history
**ObservableMedia** reports of the active breakpoint are not consistent:

*  during page startup - active breakpoint as "gt-sm"  instead of "md", "lg", or "xl".
*  during manual resize - active breakpoint as "xs", "sm", "md", "lg", or "xl".

fixes #185.
  • Loading branch information
ThomasBurleson committed Feb 19, 2017
1 parent 77af707 commit 7228054
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 11 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
"@types/glob": "^5.0.29",
"@types/gulp": "^3.8.29",
"@types/hammerjs": "~2.0.33",
"@types/jasmine": "^2.2.34",
"@types/jasmine": "2.5.38",
"@types/merge2": "0.0.28",
"@types/minimist": "^1.1.28",
"@types/node": "^6.0.45",
Expand Down
2 changes: 1 addition & 1 deletion src/lib/media-query/match-media.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ describe('match-media-observable', () => {

// "all" mediaQuery is already active; total count should be (3)

expect(activationCount).toEqual(3);
expect(activationCount).toEqual(2);
expect(deactivationCount).toEqual(0);

subscription.unsubscribe();
Expand Down
4 changes: 2 additions & 2 deletions src/lib/media-query/mock/mock-match-media.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export class MockMatchMedia extends MatchMedia {
* "xs" active == false
*
*/
private _activateWithOverlaps(mediaQuery: string, useOverlaps: boolean) {
private _activateWithOverlaps(mediaQuery: string, useOverlaps: boolean): boolean {
if (useOverlaps) {
let bp = this._breakpoints.findByQuery(mediaQuery);
switch (bp ? bp.alias : 'unknown') {
Expand All @@ -96,7 +96,7 @@ export class MockMatchMedia extends MatchMedia {
}
}
// Activate last since the responsiveActivation is watching *this* mediaQuery
this._activateByQuery(mediaQuery);
return this._activateByQuery(mediaQuery);
}

/**
Expand Down
7 changes: 5 additions & 2 deletions src/lib/media-query/observable-media-service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ describe('match-media-observable-provider', () => {
subscription.unsubscribe();
}));

it('can can subscribe to built-in mediaQueries', async(inject(
fit('can subscribe to built-in mediaQueries', inject(
[ObservableMedia, MatchMedia],
(media$, matchMedia) => {
let current: MediaChange;
Expand All @@ -117,6 +117,9 @@ describe('match-media-observable-provider', () => {
matchMedia.activate('md');
expect(current.mediaQuery).toEqual(findMediaQuery('md'));

// Allow overlapping activations to be announced to observers
media$.filterOverlaps = false;

matchMedia.activate('gt-lg');
expect(current.mediaQuery).toEqual(findMediaQuery('gt-lg'));

Expand All @@ -127,7 +130,7 @@ describe('match-media-observable-provider', () => {
matchMedia.autoRegisterQueries = true;
subscription.unsubscribe();
}
})));
}));

it('can `.unsubscribe()` properly', async(inject(
[ObservableMedia, MatchMedia],
Expand Down
20 changes: 15 additions & 5 deletions src/lib/media-query/observable-media-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,12 @@ import {BreakPoint} from './breakpoints/break-point';
*/
export abstract class ObservableMedia implements Subscribable<MediaChange> {
abstract isActive(query: string): boolean;

abstract asObservable(): Observable<MediaChange>;

abstract subscribe(next?: (value: MediaChange) => void,
error?: (error: any) => void,
complete?: () => void): Subscription;
error?: (error: any) => void,
complete?: () => void): Subscription;
}

/**
Expand Down Expand Up @@ -74,12 +76,15 @@ export abstract class ObservableMedia implements Subscribable<MediaChange> {
*/
@Injectable()
export class MediaService implements ObservableMedia {
private observable$: Observable<MediaChange>;
/**
* Should we announce gt-<xxx> breakpoint activations ?
*/
public filterOverlaps = true;

constructor(private mediaWatcher: MatchMedia,
private breakpoints: BreakPointRegistry) {
this._registerBreakPoints();
this.observable$ = this._buildObservable();
this._registerBreakPoints();
}

/**
Expand Down Expand Up @@ -137,6 +142,10 @@ export class MediaService implements ObservableMedia {
.map((change: MediaChange) => {
// Inject associated (if any) alias information into the MediaChange event
return mergeAlias(change, this._findByQuery(change.mediaQuery));
})
.filter((change: MediaChange) => {
let bp = this.breakpoints.findByQuery(change.mediaQuery);
return !bp ? true : !(this.filterOverlaps && bp.overlapping);
});
}

Expand All @@ -145,7 +154,7 @@ export class MediaService implements ObservableMedia {
*/
private _findByAlias(alias) {
return this.breakpoints.findByAlias(alias);
};
}

/**
* Breakpoint locator by mediaQuery
Expand All @@ -162,6 +171,7 @@ export class MediaService implements ObservableMedia {
return bp ? bp.mediaQuery : query;
};

private observable$: Observable<MediaChange>;
}

/**
Expand Down

0 comments on commit 7228054

Please sign in to comment.