fix(fxLayoutGap): correctly handle lack of fallback value (#1037)

In most directives, style generation and removal happens in
a contained cycle: styles are generated, cached, and then
removed when no longer needed. This is facilitated by caching
the results of the `StyleBuilder`s for each directive in a
local object on the directive, and then removing those styles
when called.

In the case of `fxLayoutGap`, most of the style generation is
applied to the children, and so in most cases just removing the
parent styles is insufficient, and in other cases, just
ineffective (because no parent styles are actually generated).
To fix this, we override the default style clearing method,
and replace it with one that actually removes the child styles
(and the parent styles if any are present).

Fixes #1011

Author: CaerusKaru

src/lib/flex/layout-gap/layout-gap.spec.ts

@@ -374,6 +374,7 @@ describe('layout-gap directive', () => {
 
       let nodes = queryFor(fixture, '[fxFlex]');
       expect(nodes.length).toEqual(3);
+      mediaController.activate('sm');
       expectEl(nodes[0]).not.toHaveStyle({'margin-right': '*'}, styler);
       expectEl(nodes[1]).not.toHaveStyle({'margin-right': '*'}, styler);
       expectEl(nodes[2]).not.toHaveStyle({'margin-right': '*'}, styler);
@@ -384,6 +385,11 @@ describe('layout-gap directive', () => {
       expectEl(nodes[1]).toHaveStyle({'margin-right': '24px'}, styler);
       expectEl(nodes[2]).not.toHaveStyle({'margin-right': '24px'}, styler);
       expectEl(nodes[2]).not.toHaveStyle({'margin-right': '0px'}, styler);
+
+      mediaController.activate('sm');
+      expectEl(nodes[0]).not.toHaveStyle({'margin-right': '*'}, styler);
+      expectEl(nodes[1]).not.toHaveStyle({'margin-right': '*'}, styler);
+      expectEl(nodes[2]).not.toHaveStyle({'margin-right': '*'}, styler);
     });
 
     it('should set gap with responsive layout change', () => {
@@ -503,6 +509,36 @@ describe('layout-gap directive', () => {
       expectNativeEl(fixture).toHaveStyle(expectedMargin, styler);
     });
 
+    it('should set gap without fallback', () => {
+      let template = `
+        <div fxLayoutAlign='center center' fxLayoutGap.md="24px grid">
+          <div fxFlex></div>
+          <div fxFlex></div>
+          <div fxFlex></div>
+        </div>
+      `;
+      createTestComponent(template);
+      fixture.detectChanges();
+
+      let nodes = queryFor(fixture, '[fxFlex]');
+      expect(nodes.length).toEqual(3);
+      mediaController.activate('sm');
+      expectEl(nodes[0]).not.toHaveStyle({'padding': '*'}, styler);
+      expectEl(nodes[1]).not.toHaveStyle({'padding': '*'}, styler);
+      expectEl(nodes[2]).not.toHaveStyle({'padding': '*'}, styler);
+
+      mediaController.activate('md');
+      fixture.detectChanges();
+      expectEl(nodes[0]).toHaveStyle({'padding': '0px 24px 24px 0px'}, styler);
+      expectEl(nodes[1]).toHaveStyle({'padding': '0px 24px 24px 0px'}, styler);
+      expectEl(nodes[2]).toHaveStyle({'padding': '0px 24px 24px 0px'}, styler);
+
+      mediaController.activate('sm');
+      expectEl(nodes[0]).not.toHaveStyle({'padding': '*'}, styler);
+      expectEl(nodes[1]).not.toHaveStyle({'padding': '*'}, styler);
+      expectEl(nodes[2]).not.toHaveStyle({'padding': '*'}, styler);
+    });
+
     it('should add gap styles correctly for rtl', () => {
       fakeDocument.body.dir = 'rtl';
       let template = `

src/lib/flex/layout-gap/layout-gap.ts

@@ -197,6 +197,21 @@ export class LayoutGapDirective extends BaseDirective2 implements AfterContentIn
     }
   }
 
+  /** We need to override clearStyles because in most cases mru isn't populated */
+  protected clearStyles() {
+    const gridMode = Object.keys(this.mru).length > 0;
+    const childrenStyle = gridMode ? 'padding' :
+      getMarginType(this.directionality.value, this.layout);
+
+    // If there are styles on the parent remove them
+    if (gridMode) {
+      super.clearStyles();
+    }
+
+    // Then remove the children styles too
+    this.styleUtils.applyStyleToElements({[childrenStyle]: ''}, this.childrenNodes);
+  }
+
   /** Determine if an element will show or hide based on current activation */
   protected willDisplay(source: HTMLElement): boolean {
     const value = this.marshal.getValue(source, 'show-hide');
@@ -262,28 +277,25 @@ function buildGridMargin(value: string, directionality: string): StyleDefinition
   return {'margin': `${marginTop} ${marginRight} ${marginBottom} ${marginLeft}`};
 }
 
-function buildGapCSS(gapValue: string,
-                     parent: {directionality: string, layout: string}): StyleDefinition {
-  let key, margins: {[key: string]: string | null} = {...CLEAR_MARGIN_CSS};
-
-  switch (parent.layout) {
+function getMarginType(directionality: string, layout: string) {
+  switch (layout) {
     case 'column':
-      key = 'margin-bottom';
-      break;
+      return 'margin-bottom';
     case 'column-reverse':
-      key = 'margin-top';
-      break;
+      return 'margin-top';
     case 'row':
-      key = parent.directionality === 'rtl' ? 'margin-left' : 'margin-right';
-      break;
+      return directionality === 'rtl' ? 'margin-left' : 'margin-right';
     case 'row-reverse':
-      key = parent.directionality === 'rtl' ? 'margin-right' : 'margin-left';
-      break;
+      return directionality === 'rtl' ? 'margin-right' : 'margin-left';
     default :
-      key = parent.directionality === 'rtl' ? 'margin-left' : 'margin-right';
-      break;
+      return directionality === 'rtl' ? 'margin-left' : 'margin-right';
   }
-  margins[key] = gapValue;
+}
 
+function buildGapCSS(gapValue: string,
+                     parent: {directionality: string, layout: string}): StyleDefinition {
+  const key = getMarginType(parent.directionality, parent.layout);
+  const margins: {[key: string]: string | null} = {...CLEAR_MARGIN_CSS};
+  margins[key] = gapValue;
   return margins;
 }