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

* fxShow and fxHide should restore original display modes when deactivating * combination uses of fxHide + fxShow should work properly * static uses of `fxHide=""` should hide the hosting elmement fixes #140, fixes #141.
angular · Jan 30, 2017 · 7f2b07b · 7f2b07b
1 parent 6e46561
commit 7f2b07b
Show file tree

Hide file tree

Showing 8 changed files with 134 additions and 37 deletions.
diff --git a/src/lib/flexbox/api/base.ts b/src/lib/flexbox/api/base.ts
@@ -6,6 +6,10 @@
  * found in the LICENSE file at https://angular.io/license
  */
 import {ElementRef, Renderer, OnDestroy} from '@angular/core';
+
+import {__platform_browser_private__} from '@angular/platform-browser';
+const getDOM = __platform_browser_private__.getDOM;
+
 import {applyCssPrefixes} from '../../utils/auto-prefixer';
 
 import {ResponsiveActivation, KeyOptions} from '../responsive/responsive-activation';
@@ -16,7 +20,7 @@ import {MediaQuerySubscriber} from '../../media-query/media-change';
  * Definition of a css style. Either a property name (e.g. "flex-basis") or an object
  * map of property name and value (e.g. {display: 'none', flex-order: 5}).
  */
-export type StyleDefinition = string|{[property: string]: string|number};
+export type StyleDefinition = string|{ [property: string]: string|number };
 
 /** Abstract base class for the Layout API styling directives. */
 export abstract class BaseFxDirective implements OnDestroy {
@@ -88,7 +92,8 @@ export abstract class BaseFxDirective implements OnDestroy {
    */
   protected _getDisplayStyle(): string {
     let element: HTMLElement = this._elementRef.nativeElement;
-    return (element.style as any)['display'] || "flex";
+    let value = (element.style as any)['display'] || getDOM().getComputedStyle(element)['display'];
+    return value.trim();
   }
 
   /**
@@ -109,7 +114,7 @@ export abstract class BaseFxDirective implements OnDestroy {
 
     // Iterate all properties in hashMap and set styles
     for (let key in styles) {
-      this._renderer.setElementStyle(element, key, styles[key]);
+      this._renderer.setElementStyle(element, key, styles[key] as string);
     }
   }
 
@@ -122,7 +127,7 @@ export abstract class BaseFxDirective implements OnDestroy {
     elements.forEach(el => {
       // Iterate all properties in hashMap and set styles
       for (let key in styles) {
-        this._renderer.setElementStyle(el, key, styles[key]);
+        this._renderer.setElementStyle(el, key, styles[key] as string);
       }
     });
 
@@ -172,4 +177,11 @@ export abstract class BaseFxDirective implements OnDestroy {
     return buffer;
   }
 
+  /**
+   * Fast validator for presence of attribute on the host element
+   */
+  protected hasKeyValue(key) {
+    return this._mqActivation.hasKeyValue(key);
+  }
+
 }
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>
+            This content to be shown ONLY when gt-sm
+          </div>
+       `);
+    expectNativeEl(fixture).toHaveCssStyle({'display': 'block'});
+
+    activateMediaQuery('md', true);
+    expectNativeEl(fixture).toHaveCssStyle({'display': 'none'});
+
+    activateMediaQuery('xs', true);
+    expectNativeEl(fixture).toHaveCssStyle({'display': '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,15 @@ 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;
+  }
+
+  /**
+   * Fast validator for presence of attribute on the host element
+   */
+  public hasKeyValue(key) {
+    let value = this._options.inputKeys[key];
+    return typeof value !== 'undefined';
   }
 
   /**
@@ -203,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 => {