fix(api): restore orig display mode and more... (#143)

ThomasBurleson · kara · commit d269d73e78a5 · 2017-02-02T15:54:46.000-08:00
fixes #140, fixes #141.
diff --git a/src/lib/flexbox/api/hide.spec.ts b/src/lib/flexbox/api/hide.spec.ts
@@ -17,7 +17,7 @@ import {ObservableMediaService} from '../../media-query/observable-media-service
 import {BreakPointsProvider} from '../../media-query/providers/break-points-provider';
 import {BreakPointRegistry} from '../../media-query/breakpoints/break-point-registry';
 
-import {customMatchers, expect} from '../../utils/testing/custom-matchers';
+import {customMatchers, expect, NgMatchers} from '../../utils/testing/custom-matchers';
 import {
   makeCreateTestComponent, expectNativeEl, queryFor
 } from '../../utils/testing/helpers';
@@ -27,9 +27,22 @@ import {MediaQueriesModule} from '../../media-query/_module';
 describe('hide directive', () => {
   let fixture: ComponentFixture<any>;
   let createTestComponent = makeCreateTestComponent(() => TestHideComponent);
-  let activateMediaQuery = (alias) => {
+  let activateMediaQuery: Function = (alias, useOverlaps = false): void => {
     let matchMedia: MockMatchMedia = fixture.debugElement.injector.get(MatchMedia);
-    matchMedia.activate(alias);
+    matchMedia.activate(alias, useOverlaps);
+  };
+  let makeExpectWithActivation = (_fixture_: ComponentFixture<any>, selector: string) => {
+    fixture = _fixture_;
+    return (alias?: string): NgMatchers => {
+      if (alias) {
+        activateMediaQuery(alias);
+      }
+      fixture.detectChanges();
+
+      let nodes = queryFor(fixture, selector);
+      expect(nodes.length).toEqual(1);
+      return expect(nodes[0].nativeElement);
+    };
   };
 
   beforeEach(() => {
@@ -102,7 +115,7 @@ describe('hide directive', () => {
         </button>
       `);
       expectNativeEl(fixture, {isHidden: true}).toHaveCssStyle({'display': 'none'});
-      expectNativeEl(fixture, {isHidden: false}).toHaveCssStyle({'display': 'block'});
+      expectNativeEl(fixture, {isHidden: false}).toHaveCssStyle({'display': 'inline-block'});
     });
 
     it('should use "flex" display style when the element also has an fxLayout', () => {
@@ -226,23 +239,41 @@ describe('hide directive', () => {
             </div>
           </div>
         `;
+      let expectActivation = makeExpectWithActivation(createTestComponent(template), '.hideOnMd');
 
-      fixture = createTestComponent(template);
-      fixture.detectChanges();
-
-      let nodes = queryFor(fixture, ".hideOnMd");
-      expect(nodes.length).toEqual(1);
-      expect(nodes[0].nativeElement).toHaveCssStyle({'display': 'block'});
+      expectActivation().toHaveCssStyle({'display': 'block'});
+      expectActivation('md').toHaveCssStyle({'display': 'none'});
+    });
 
-      activateMediaQuery('md');
-      fixture.detectChanges();
+    it('should restore proper display mode when not hiding', () => {
+      let template = `
+              <div>
+                <span fxHide.xs class="hideOnXs">Label</span>
+              </div>
+           `;
+      let expectActivation = makeExpectWithActivation(createTestComponent(template), '.hideOnXs');
 
-      nodes = queryFor(fixture, ".hideOnMd");
-      expect(nodes.length).toEqual(1);
-      expect(nodes[0].nativeElement).toHaveCssStyle({'display': 'none'});
+      expectActivation().toHaveCssStyle({'display': 'inline'});
+      expectActivation('xs').toHaveCssStyle({'display': 'none'});
+      expectActivation('md').toHaveCssStyle({'display': 'inline'});
     });
   });
 
+  it('should support hide and show', () => {
+    fixture = createTestComponent(`
+          <div fxShow fxHide.gt-sm style="display:inline-block;">
+            This content to be shown ONLY when gt-sm
+          </div>
+       `);
+    expectNativeEl(fixture).toHaveCssStyle({'display': 'inline-block'});
+
+    activateMediaQuery('md', true);
+    expectNativeEl(fixture).toHaveCssStyle({'display': 'none'});
+
+    activateMediaQuery('xs', true);
+    expectNativeEl(fixture).toHaveCssStyle({'display': 'inline-block'});
+  });
+
 });
 
 
diff --git a/src/lib/flexbox/api/hide.ts b/src/lib/flexbox/api/hide.ts
@@ -101,14 +101,14 @@ export class HideDirective extends BaseFxDirective implements OnInit, OnChanges,
               protected renderer: Renderer) {
     super(monitor, elRef, renderer);
 
-    this._display = this._getDisplayStyle(); // re-invoke override to use `this._layout`
+    /**
+     * The Layout can set the display:flex (and incorrectly affect the Hide/Show directives.
+     * Whenever Layout [on the same element] resets its CSS, then update the Hide/Show CSS
+     */
     if (_layout) {
-      /**
-       * The Layout can set the display:flex (and incorrectly affect the Hide/Show directives.
-       * Whenever Layout [on the same element] resets its CSS, then update the Hide/Show CSS
-       */
       this._layoutWatcher = _layout.layout$.subscribe(() => this._updateWithValue());
     }
+    this._display = this._getDisplayStyle(); // re-invoke override to use `this._layout`
   }
 
 
@@ -122,8 +122,7 @@ export class HideDirective extends BaseFxDirective implements OnInit, OnChanges,
    * unless it was already explicitly defined.
    */
   protected _getDisplayStyle(): string {
-    let element: HTMLElement = this._elementRef.nativeElement;
-    return (element.style as any)['display'] || (this._layout ? "flex" : "block");
+    return this._layout ? "flex" : super._getDisplayStyle();
   }
 
   /**
@@ -140,9 +139,15 @@ export class HideDirective extends BaseFxDirective implements OnInit, OnChanges,
   /**
    * After the initial onChanges, build an mqActivation object that bridges
    * mql change events to onMediaQueryChange handlers
+   * NOTE: fxHide has special fallback defaults.
+   *       - If the non-responsive fxHide="" is specified we default to hide==true
+   *       - If the non-responsive fxHide is NOT specified, use default hide == false
+   *       This logic supports mixed usages with fxShow; e.g. `<div fxHide fxShow.gt-sm>`
    */
   ngOnInit() {
-    let value = this._getDefaultVal("hide", false);
+    // If the attribute 'fxHide' is specified we default to hide==true, otherwise do nothing..
+    let value = (this._queryInput('hide') == "") ? true : this._getDefaultVal("hide", false);
+
     this._listenForMediaQueryChanges('hide', value, (changes: MediaChange) => {
       this._updateWithValue(changes.value);
     });
diff --git a/src/lib/flexbox/api/show.spec.ts b/src/lib/flexbox/api/show.spec.ts
@@ -199,6 +199,25 @@ describe('show directive', () => {
 
 
   });
+
+  describe('with fxHide features', () => {
+
+    it('should support hide and show', () => {
+      fixture = createTestComponent(`
+          <div fxHide fxShow.gt-sm>
+            This content to be shown ONLY when gt-sm
+          </div>
+       `);
+      expectNativeEl(fixture).toHaveCssStyle({'display': 'none'});
+
+      activateMediaQuery('md', true);
+      expectNativeEl(fixture).toHaveCssStyle({'display': 'block'});
+
+      activateMediaQuery('xs', true);
+      expectNativeEl(fixture).toHaveCssStyle({'display': 'none'});
+    });
+  });
+
 });
 
 
diff --git a/src/lib/flexbox/api/show.ts b/src/lib/flexbox/api/show.ts
@@ -25,6 +25,7 @@ import {MediaChange} from '../../media-query/media-change';
 import {MediaMonitor} from '../../media-query/media-monitor';
 
 import {LayoutDirective} from './layout';
+import {HideDirective} from './hide';
 
 
 const FALSY = ['false', false, 0];
@@ -100,6 +101,7 @@ export class ShowDirective extends BaseFxDirective implements OnInit, OnChanges,
    */
   constructor(monitor: MediaMonitor,
               @Optional() @Self() private _layout: LayoutDirective,
+              @Optional() @Self() private _hide: HideDirective,
               protected elRef: ElementRef,
               protected renderer: Renderer) {
 
@@ -148,10 +150,16 @@ export class ShowDirective extends BaseFxDirective implements OnInit, OnChanges,
   ngOnInit() {
     let value = this._getDefaultVal("show", true);
 
+    // Build _mqActivation controller
     this._listenForMediaQueryChanges('show', value, (changes: MediaChange) => {
-      this._updateWithValue(changes.value);
+      if (!this._delegateToHide(changes)) {
+        this._updateWithValue(changes.value);
+      }
     });
-    this._updateWithValue();
+
+    if (!this._delegateToHide()) {
+      this._updateWithValue();
+    }
   }
 
   ngOnDestroy() {
@@ -165,6 +173,21 @@ export class ShowDirective extends BaseFxDirective implements OnInit, OnChanges,
   // Protected methods
   // *********************************************
 
+  /**
+   * If deactiving Show, then delegate action to the Hide directive if it is
+   * specified on same element.
+   */
+  protected _delegateToHide(changes?: MediaChange) {
+    if (this._hide) {
+      let delegate = (changes && !changes.matches) || (!changes && !this.hasKeyValue('show'));
+      if (delegate) {
+        this._hide.ngOnChanges({});
+        return true;
+      }
+    }
+    return false;
+  }
+
   /** Validate the visibility value and then update the host's inline display style */
   private _updateWithValue(value?: string|number|boolean) {
     value = value || this._getDefaultVal("show", true);
diff --git a/src/lib/flexbox/responsive/responsive-activation.ts b/src/lib/flexbox/responsive/responsive-activation.ts
@@ -7,6 +7,7 @@
  */
 import {Subscription} from 'rxjs/Subscription';
 import 'rxjs/add/operator/map';
+import 'rxjs/add/operator/filter';
 import {extendObject} from '../../utils/object-extend';
 
 import {MediaChange, MediaQuerySubscriber} from '../../media-query/media-change';
@@ -23,7 +24,7 @@ export interface BreakPointX extends BreakPoint {
 export class KeyOptions {
   constructor(public baseKey: string,
               public defaultValue: string|number|boolean,
-              public inputKeys: {[key: string]: any}) {
+              public inputKeys: { [key: string]: any }) {
   }
 }
 
@@ -78,7 +79,7 @@ export class ResponsiveActivation {
    */
   get activatedInput(): any {
     let key = this.activatedInputKey;
-    return this._hasKeyValue(key) ? this._lookupKeyValue(key) : this._options.defaultValue;
+    return this.hasKeyValue(key) ? this._lookupKeyValue(key) : this._options.defaultValue;
   }
 
   /**
@@ -211,8 +212,4 @@ export class ResponsiveActivation {
     return this._options.inputKeys[key];
   }
 
-  private _hasKeyValue(key) {
-    let value = this._options.inputKeys[key];
-    return typeof value !== 'undefined';
-  }
 }
diff --git a/src/lib/media-query/mock/mock-match-media.ts b/src/lib/media-query/mock/mock-match-media.ts
@@ -217,7 +217,9 @@ export class MockMediaQueryList implements MediaQueryList {
     if (this._listeners.indexOf(listener) === -1) {
       this._listeners.push(listener);
     }
-    if (this._isActive) { listener(this); }
+    if (this._isActive) {
+      listener(this);
+    }
   }
 
   removeListener(listener: MediaQueryListListener) {
diff --git a/src/lib/utils/testing/custom-matchers.ts b/src/lib/utils/testing/custom-matchers.ts
@@ -148,7 +148,7 @@ export const customMatchers: jasmine.CustomMatcherFactories = {
  * (Safari, IE, etc) will use prefixed style instead of defaults.
  */
 function hasPrefixedStyles(actual, key, value) {
-  let hasStyle = getDOM().hasStyle(actual, key, value);
+  let hasStyle = getDOM().hasStyle(actual, key, value.trim());
   if (!hasStyle) {
     let prefixedStyles = applyCssPrefixes({[key]: value});
     Object.keys(prefixedStyles).forEach(prop => {

Original file line number	Diff line number	Diff line change
`@@ -7,6 +7,7 @@`
`7`	`7`	`*/`
`8`	`8`	`import {Subscription} from 'rxjs/Subscription';`
`9`	`9`	`import 'rxjs/add/operator/map';`
	`10`	`+import 'rxjs/add/operator/filter';`
`10`	`11`	`import {extendObject} from '../../utils/object-extend';`
`11`	`12`
`12`	`13`	`import {MediaChange, MediaQuerySubscriber} from '../../media-query/media-change';`
`@@ -23,7 +24,7 @@ export interface BreakPointX extends BreakPoint {`
`23`	`24`	`export class KeyOptions {`
`24`	`25`	`constructor(public baseKey: string,`
`25`	`26`	`public defaultValue: string\|number\|boolean,`
`26`		`- public inputKeys: {[key: string]: any}) {`
	`27`	`+ public inputKeys: { [key: string]: any }) {`
`27`	`28`	`}`
`28`	`29`	`}`
`29`	`30`
`@@ -78,7 +79,7 @@ export class ResponsiveActivation {`
`78`	`79`	`*/`
`79`	`80`	`get activatedInput(): any {`
`80`	`81`	`let key = this.activatedInputKey;`
`81`		`- return this._hasKeyValue(key) ? this._lookupKeyValue(key) : this._options.defaultValue;`
	`82`	`+ return this.hasKeyValue(key) ? this._lookupKeyValue(key) : this._options.defaultValue;`
`82`	`83`	`}`
`83`	`84`
`84`	`85`	`/**`
`@@ -211,8 +212,4 @@ export class ResponsiveActivation {`
`211`	`212`	`return this._options.inputKeys[key];`
`212`	`213`	`}`
`213`	`214`
`214`		`- private _hasKeyValue(key) {`
`215`		`- let value = this._options.inputKeys[key];`
`216`		`- return typeof value !== 'undefined';`
`217`		`- }`
`218`	`215`	`}`
Original file line number	Diff line number	Diff line change
`@@ -217,7 +217,9 @@ export class MockMediaQueryList implements MediaQueryList {`
`217`	`217`	`if (this._listeners.indexOf(listener) === -1) {`
`218`	`218`	`this._listeners.push(listener);`
`219`	`219`	`}`
`220`		`- if (this._isActive) { listener(this); }`
	`220`	`+ if (this._isActive) {`
	`221`	`+ listener(this);`
	`222`	`+ }`
`221`	`223`	`}`
`222`	`224`
`223`	`225`	`removeListener(listener: MediaQueryListListener) {`