fix(fxLayoutGap): skip hidden element nodes (#145)

Fixes #136.

Author: ThomasBurleson

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 {
@@ -86,9 +90,10 @@ export abstract class BaseFxDirective implements OnDestroy {
    * Note: this allows use to preserve the original style
    * and optional restore it when the mediaQueries deactivate
    */
-  protected _getDisplayStyle(): string {
-    let element: HTMLElement = this._elementRef.nativeElement;
-    return (element.style as any)['display'] || "flex";
+  protected _getDisplayStyle(source?: HTMLElement): string {
+    let element: HTMLElement = source || this._elementRef.nativeElement;
+    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);
+  }
+
 }

src/lib/flexbox/api/layout-gap.spec.ts

@@ -41,6 +41,22 @@ describe('layout-gap directive', () => {
     });
   });
 
+  function verifyCorrectMargin(layout: string, marginKey: string) {
+    const margin = '8px';
+    const template = `
+            <div fxLayout='${layout}' fxLayoutGap='${margin}'>
+                <span></span>
+                <span></span>
+            </div>
+        `;
+
+    const fixture1 = createTestComponent(template);
+    fixture1.detectChanges();
+
+    const nodes = queryFor(fixture1, 'span');
+    expect(nodes[1].nativeElement).toHaveCssStyle({[marginKey]: margin});
+  }
+
   describe('with static features', () => {
 
     it('should not add gap styles for a single child', () => {
@@ -163,23 +179,29 @@ describe('layout-gap directive', () => {
       expect(nodes[1].nativeElement).not.toHaveCssStyle({'margin-bottom': '8px'});
       expect(nodes[1].nativeElement).toHaveCssStyle({'margin-right': '8px'});
     });
-  });
 
-  function verifyCorrectMargin(layout: string, marginKey: string) {
-    const margin = '8px';
-    const template = `
-            <div fxLayout='${layout}' fxLayoutGap='${margin}'>
-                <span></span>
-                <span></span>
-            </div>
-        `;
+    it('should recognize hidden elements when applying gaps', () => {
+      let styles = ['.col1 { display:none !important;'];
+      let template = `
+        <div class="container" fxLayout="row" fxLayoutGap="16px">
+          <div fxFlex class="col1">Div 1</div>
+          <div fxFlex class="col2">Div 2</div>
+          <div fxFlex class="col3">Div 3</div>
+        </div>
+      `;
+      fixture = createTestComponent(template, styles);
+      fixture.componentInstance.direction = 'row';
+      fixture.detectChanges();
 
-    const fixture1 = createTestComponent(template);
-    fixture1.detectChanges();
+      let nodes = queryFor(fixture, '[fxFlex]');
 
-    const nodes = queryFor(fixture1, 'span');
-    expect(nodes[1].nativeElement).toHaveCssStyle({[marginKey]: margin});
-  }
+      expect(nodes.length).toEqual(3);
+      expect(nodes[0].nativeElement).not.toHaveCssStyle({'margin-left': '0px'});
+      expect(nodes[1].nativeElement).toHaveCssStyle({'margin-left': '0px'});
+      expect(nodes[2].nativeElement).toHaveCssStyle({'margin-left': '16px'});
+
+    });
+  });
 
   describe('with responsive features', () => {
 

src/lib/flexbox/api/layout-gap.ts

@@ -171,18 +171,18 @@ export class LayoutGapDirective extends BaseFxDirective implements AfterContentI
       value = this._mqActivation.activatedInput;
     }
 
-    // Reset 1st child element to 0px gap
+    // Gather all non-hidden Element nodes
     let items = this.childrenNodes
-        .filter(el => (el.nodeType === 1))   // only Element types
-        .filter((el, j) => j == 0);
-    this._applyStyleToElements(this._buildCSS(0), items);
+          .filter(el => (el.nodeType === 1))   // only Element types
+          .filter(el => this._getDisplayStyle(el) != "none");
+
+    // Reset 1st child element to 0px gap
+    let skipped = items.filter((el, j) => j == 0);
+    this._applyStyleToElements(this._buildCSS(0), skipped);
 
     // For each `element` child, set the padding styles...
-    items = this.childrenNodes
-        .filter(el => (el.nodeType === 1))   // only Element types
-        .filter((el, j) => j > 0);          // skip first element since gaps are needed
+    items = items.filter((el, j) => j > 0);          // skip first element since gaps are needed
     this._applyStyleToElements(this._buildCSS(value), items);
-
   }
 
   /**

src/lib/flexbox/responsive/responsive-activation.ts

@@ -81,6 +81,14 @@ export class ResponsiveActivation {
     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';
+  }
+
   /**
    * Remove interceptors, restore original functions, and forward the onDestroy() call
    */

src/lib/utils/testing/helpers.ts

@@ -46,13 +46,18 @@ export function makeCreateTestComponent(getClass: ComponentClazzFn) {
   let componentAny: Type<any>;
 
   // Return actual `createTestComponent()` function
-  return function createTestComponent(template: string): ComponentFixture<Type<any>> {
+  return function createTestComponent(template: string, styles?: any): ComponentFixture<Type<any>> {
     if (!componentAny) {
       // Defer access to Component class to enable metadata to be configured properly...
       componentAny = getClass();
     }
     return TestBed
-        .overrideComponent(componentAny, {set: {template: template}})
+        .overrideComponent(componentAny, {
+          set: {
+            template: template,
+            styles: styles || [],
+          }
+        })
         .createComponent(componentAny);
   };
 }