Skip to content

Commit

Permalink
fix(api): remove circular dependencies
Browse files Browse the repository at this point in the history
* fxShow and fxHide had 'unused' circular dependencies; which causes issues with the Clojure compiler
* add unit tests to show/hide to test for MatchMediObservable usages in templates

Fixes #88.
  • Loading branch information
ThomasBurleson committed Jan 4, 2017
1 parent dde6e87 commit 7bff29e
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 33 deletions.
38 changes: 35 additions & 3 deletions src/lib/flexbox/api/hide.spec.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import {Component, OnInit} from '@angular/core';
import {Component, OnInit, Inject} from '@angular/core';
import {CommonModule} from '@angular/common';
import {ComponentFixture, TestBed } from '@angular/core/testing';

import {MockMatchMedia} from '../../media-query/mock/mock-match-media';
import {MatchMedia} from '../../media-query/match-media';
import {MatchMedia, MatchMediaObservable} from '../../media-query/match-media';
import {BreakPointsProvider} from '../../media-query/providers/break-points-provider';
import {BreakPointRegistry} from '../../media-query/breakpoints/break-point-registry';
import {FlexLayoutModule} from '../_module';
Expand Down Expand Up @@ -100,6 +100,36 @@ describe('show directive', () => {
});

});

it('should support use of the `media` observable in templates ', () => {
fixture = createTestComponent(`
<div [fxHide]="media.isActive('xs')" >
...content
</div>
`);
expectNativeEl(fixture).toHaveCssStyle({ 'display': 'flex' });

activateMediaQuery('xs');
expectNativeEl(fixture).toHaveCssStyle({ 'display': 'none' });

activateMediaQuery('lg');
expectNativeEl(fixture).toHaveCssStyle({ 'display': 'flex' });
});

it('should support use of the `media` observable in adaptive templates ', () => {
fixture = createTestComponent(`
<div fxHide="false" [fxHide.md]="media.isActive('xs')" >
...content
</div>
`);
expectNativeEl(fixture).toHaveCssStyle({ 'display': 'flex' });

activateMediaQuery('xs');
expectNativeEl(fixture).toHaveCssStyle({ 'display': 'flex' });

activateMediaQuery('md');
expectNativeEl(fixture).toHaveCssStyle({ 'display': 'flex' });
});
});


Expand All @@ -114,10 +144,12 @@ describe('show directive', () => {
export class TestHideComponent implements OnInit {
isVisible = 0;
menuHidden: boolean = true;

constructor(@Inject(MatchMediaObservable) private media) { }

toggleMenu() {
this.menuHidden = !this.menuHidden;
}
constructor() { }
ngOnInit() { }
}

Expand Down
12 changes: 1 addition & 11 deletions src/lib/flexbox/api/hide.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import {BaseFxDirective} from './base';
import {MediaChange} from '../../media-query/media-change';
import {MediaMonitor} from '../../media-query/media-monitor';

import {ShowDirective} from "./show";
import {LayoutDirective} from './layout';

/**
Expand Down Expand Up @@ -65,7 +64,6 @@ export class HideDirective extends BaseFxDirective implements OnInit, OnChanges,
constructor(
monitor : MediaMonitor,
@Optional() @Self() private _layout: LayoutDirective,
@Optional() @Self() private _showDirective : ShowDirective,
protected elRef: ElementRef,
protected renderer: Renderer) {
super(monitor, elRef, renderer);
Expand All @@ -79,12 +77,6 @@ export class HideDirective extends BaseFxDirective implements OnInit, OnChanges,
}
}

/**
* Does the current element also use the fxShow API ?
*/
protected get usesShowAPI() {
return !!this._showDirective;
}

// *********************************************
// Lifecycle Methods
Expand Down Expand Up @@ -134,9 +126,7 @@ export class HideDirective extends BaseFxDirective implements OnInit, OnChanges,
}

let shouldHide = this._validateTruthy(value);
if ( shouldHide || !this.usesShowAPI ) {
this._applyStyleToElement(this._buildCSS(shouldHide));
}
this._applyStyleToElement(this._buildCSS(shouldHide));
}


Expand Down
26 changes: 23 additions & 3 deletions src/lib/flexbox/api/show.spec.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import {Component, OnInit} from '@angular/core';
import {Component, OnInit, Inject} from '@angular/core';
import {CommonModule} from '@angular/common';
import {ComponentFixture, TestBed } from '@angular/core/testing';

import {MockMatchMedia} from '../../media-query/mock/mock-match-media';
import {MatchMedia} from '../../media-query/match-media';
import {MatchMedia, MatchMediaObservable} from '../../media-query/match-media';
import {BreakPointsProvider} from '../../media-query/providers/break-points-provider';
import {BreakPointRegistry} from '../../media-query/breakpoints/break-point-registry';
import {FlexLayoutModule} from '../_module';
Expand Down Expand Up @@ -66,6 +66,9 @@ describe('show directive', () => {
</div>
`);
expectNativeEl(fixture).toHaveCssStyle({ 'display': 'none' });

fixture.componentInstance.isVisible = true;
expectNativeEl(fixture).toHaveCssStyle({ 'display': 'flex' });
});

it('should update styles with binding changes', () => {
Expand Down Expand Up @@ -105,6 +108,21 @@ describe('show directive', () => {
expectNativeEl(fixture).toHaveCssStyle({ 'display': 'none' });
});

it('should support use of the `media` observable in templates ', () => {
fixture = createTestComponent(`
<div [fxShow]="media.isActive('xs')" >
...content
</div>
`);
expectNativeEl(fixture).toHaveCssStyle({ 'display': 'none' });

activateMediaQuery('xs', true);
expectNativeEl(fixture).toHaveCssStyle({ 'display': 'flex' });

activateMediaQuery('gt-md', true);
expectNativeEl(fixture).toHaveCssStyle({ 'display': 'none' });
});

});
});

Expand All @@ -120,10 +138,12 @@ describe('show directive', () => {
export class TestShowComponent implements OnInit {
isVisible = 0;
menuOpen: boolean = true;

constructor(@Inject(MatchMediaObservable) private media) { }

toggleMenu() {
this.menuOpen = !this.menuOpen;
}
constructor() { }
ngOnInit() { }
}

Expand Down
18 changes: 2 additions & 16 deletions src/lib/flexbox/api/show.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ import {
Renderer,
SimpleChanges,
Self,
Optional,
Inject,
forwardRef
Optional
} from '@angular/core';

import {Subscription} from 'rxjs/Subscription';
Expand All @@ -19,7 +17,6 @@ import {BaseFxDirective} from './base';
import {MediaChange} from '../../media-query/media-change';
import {MediaMonitor} from '../../media-query/media-monitor';

import {HideDirective} from "./hide";
import {LayoutDirective} from './layout';


Expand Down Expand Up @@ -70,7 +67,6 @@ export class ShowDirective extends BaseFxDirective implements OnInit, OnChanges,
constructor(
monitor : MediaMonitor,
@Optional() @Self() private _layout: LayoutDirective,
@Inject(forwardRef(() => HideDirective)) @Optional() @Self() private _hideDirective,
protected elRef: ElementRef,
protected renderer: Renderer)
{
Expand All @@ -85,14 +81,6 @@ export class ShowDirective extends BaseFxDirective implements OnInit, OnChanges,
}
}

/**
* Does the current element also use the fxShow API ?
*/
protected get usesHideAPI() {
return !!this._hideDirective;
}


// *********************************************
// Lifecycle Methods
// *********************************************
Expand Down Expand Up @@ -138,9 +126,7 @@ export class ShowDirective extends BaseFxDirective implements OnInit, OnChanges,
}

let shouldShow = this._validateTruthy(value);
if ( shouldShow || !this.usesHideAPI ) {
this._applyStyleToElement(this._buildCSS(shouldShow));
}
this._applyStyleToElement(this._buildCSS(shouldShow));
}


Expand Down

0 comments on commit 7bff29e

Please sign in to comment.