fix(fxFlexOffset): use parent flow direction for margin property (#369)

`fxFlexOffset` assigns inline `margin-left` style; assuming default flow directions == 'row'. If the parent flow direction == 'column', then a `margin-top` should be used.

Also added a subscription to an optional parent element LayoutDirective; which will trigger the FlexOffsetDirective to update the inline styles to match the current flow direction.

> Note: if `fxFlexOffset` is used **without**  a parent flexbox styling (set via css or directive), then a `display:flex; flex-direction:row;` will be auto assigned to the fxFlexOffset host element's parent.

Fixes #328.

Author: ThomasBurleson

src/lib/flexbox/api/base.ts

@@ -73,6 +73,13 @@ export abstract class BaseFxDirective implements OnDestroy, OnChanges {
   // Accessor Methods
   // *********************************************
 
+  /**
+   * Access to host element's parent DOM node
+   */
+  protected get parentElement(): any {
+    return this._elementRef.nativeElement.parentNode;
+  }
+
   /**
    * Access the current value (if any) of the @Input property.
    */

src/lib/flexbox/api/flex-offset.spec.ts

@@ -0,0 +1,159 @@
+/**
+ * @license
+ * Copyright Google Inc. All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+import {Component} from '@angular/core';
+import {CommonModule} from '@angular/common';
+import {ComponentFixture, TestBed, inject} from '@angular/core/testing';
+
+import {DEFAULT_BREAKPOINTS_PROVIDER} from '../../media-query/breakpoints/break-points-provider';
+import {BreakPointRegistry} from '../../media-query/breakpoints/break-point-registry';
+import {MockMatchMedia} from '../../media-query/mock/mock-match-media';
+import {MatchMedia} from '../../media-query/match-media';
+import {FlexLayoutModule} from '../../module';
+
+import {customMatchers, expect} from '../../utils/testing/custom-matchers';
+import {_dom as _} from '../../utils/testing/dom-tools';
+
+import {
+  makeExpectDOMFrom,
+  makeCreateTestComponent,
+  queryFor
+} from '../../utils/testing/helpers';
+
+describe('flex directive', () => {
+  let fixture: ComponentFixture<any>;
+  let matchMedia: MockMatchMedia;
+  let expectDOMFrom = makeExpectDOMFrom(() => TestFlexComponent);
+  let componentWithTemplate = (template: string) => {
+    fixture = makeCreateTestComponent(() => TestFlexComponent)(template);
+
+    inject([MatchMedia], (_matchMedia: MockMatchMedia) => {
+      matchMedia = _matchMedia;
+    })();
+  };
+
+  beforeEach(() => {
+    jasmine.addMatchers(customMatchers);
+
+    // Configure testbed to prepare services
+    TestBed.configureTestingModule({
+      imports: [CommonModule, FlexLayoutModule],
+      declarations: [TestFlexComponent],
+      providers: [
+        BreakPointRegistry, DEFAULT_BREAKPOINTS_PROVIDER,
+        {provide: MatchMedia, useClass: MockMatchMedia}
+      ]
+    });
+  });
+
+  describe('with static features', () => {
+
+    it('should add correct styles for default `fxFlexOffset` usage', () => {
+      componentWithTemplate(`<div fxFlexOffset='32px' fxFlex></div>`);
+      fixture.detectChanges();
+
+      let dom = fixture.debugElement.children[0].nativeElement;
+      let isBox = _.hasStyle(dom, 'margin-left', '32px');
+      let hasFlex = _.hasStyle(dom, 'flex', '1 1 1e-09px') ||         // IE
+          _.hasStyle(dom, 'flex', '1 1 1e-9px') ||          // Chrome
+          _.hasStyle(dom, 'flex', '1 1 0.000000001px') ||   // Safari
+          _.hasStyle(dom, 'flex', '1 1 0px');
+
+      expect(isBox).toBeTruthy();
+      expect(hasFlex).toBe(true);
+    });
+
+
+    it('should work with percentage values', () => {
+      expectDOMFrom(`<div fxFlexOffset='17' fxFlex='37'></div>`).toHaveCssStyle({
+        'flex': '1 1 100%',
+        'box-sizing': 'border-box',
+        'margin-left': '17%'
+      });
+    });
+
+    it('should work fxLayout parents', () => {
+      componentWithTemplate(`
+        <div fxLayout='column' class='test'>
+          <div fxFlex='30px' fxFlexOffset='17px'>  </div>
+        </div>
+      `);
+      fixture.detectChanges();
+      let parent = queryFor(fixture, '.test')[0].nativeElement;
+      let element = queryFor(fixture, '[fxFlex]')[0].nativeElement;
+
+      // parent flex-direction found with 'column' with child height styles
+      expect(parent).toHaveCssStyle({'flex-direction': 'column', 'display': 'flex'});
+      expect(element).toHaveCssStyle({'margin-top': '17px'});
+    });
+
+    it('should CSS stylesheet and not inject flex-direction on parent', () => {
+      componentWithTemplate(`
+        <style>
+          .test { flex-direction:column; display: flex; }
+        </style>
+        <div class='test'>
+          <div fxFlexOffset='41px' fxFlex='30px'></div>
+        </div>
+      `);
+
+      fixture.detectChanges();
+      let parent = queryFor(fixture, '.test')[0].nativeElement;
+      let element = queryFor(fixture, '[fxFlex]')[0].nativeElement;
+
+      // parent flex-direction found with 'column' with child height styles
+      expect(parent).toHaveCssStyle({'flex-direction': 'column', 'display': 'flex'});
+      expect(element).toHaveCssStyle({'margin-top': '41px'});
+    });
+
+    it('should work with styled-parent flex directions', () => {
+      componentWithTemplate(`
+        <div fxLayout='row'>
+          <div style='flex-direction:column' class='parent'>
+            <div fxFlex='60px' fxFlexOffset='21'>  </div>
+          </div>
+        </div>
+      `);
+      fixture.detectChanges();
+      let element = queryFor(fixture, '[fxFlex]')[0].nativeElement;
+      let parent = queryFor(fixture, '.parent')[0].nativeElement;
+
+      // parent flex-direction found with 'column'; set child with height styles
+      expect(element).toHaveCssStyle({'margin-top': '21%'});
+      expect(parent).toHaveCssStyle({'flex-direction': 'column'});
+    });
+
+    it('should ignore fxLayout settings on same element', () => {
+      expectDOMFrom(`
+          <div fxLayout='column' fxFlex='37%' fxFlexOffset='52px' >
+          </div>
+        `)
+          .not.toHaveCssStyle({
+        'flex-direction': 'row',
+        'flex': '1 1 100%',
+        'margin-left': '52px',
+      });
+    });
+
+  });
+
+});
+
+
+// *****************************************************************
+// Template Component
+// *****************************************************************
+
+@Component({
+  selector: 'test-component-shell',
+  template: `<span>PlaceHolder Template HTML</span>`
+})
+export class TestFlexComponent {
+  public direction = 'column';
+}
+
+

src/lib/flexbox/api/flex-offset.ts

@@ -12,15 +12,19 @@ import {
   OnInit,
   OnChanges,
   OnDestroy,
+  Optional,
   Renderer,
   SimpleChanges,
+  SkipSelf
 } from '@angular/core';
 
+import {Subscription} from 'rxjs/Subscription';
 
 import {BaseFxDirective} from './base';
 import {MediaChange} from '../../media-query/media-change';
 import {MediaMonitor} from '../../media-query/media-monitor';
-
+import {LayoutDirective} from './layout';
+import {isFlowHorizontal} from '../../utils/layout-validator';
 
 /**
  * 'flex-offset' flexbox styling directive
@@ -53,8 +57,14 @@ export class FlexOffsetDirective extends BaseFxDirective implements OnInit, OnCh
   @Input('fxFlexOffset.gt-lg') set offsetGtLg(val) { this._cacheInput('offsetGtLg', val); };
 
   /* tslint:enable */
-  constructor(monitor: MediaMonitor,  elRef: ElementRef, renderer: Renderer) {
+  constructor(monitor: MediaMonitor,
+              elRef: ElementRef,
+              renderer: Renderer,
+              @Optional() @SkipSelf() protected _container: LayoutDirective ) {
     super(monitor, elRef, renderer);
+
+
+    this.watchParentFlow();
   }
 
   // *********************************************
@@ -70,6 +80,16 @@ export class FlexOffsetDirective extends BaseFxDirective implements OnInit, OnCh
     }
   }
 
+  /**
+   * Cleanup
+   */
+  ngOnDestroy() {
+    super.ngOnDestroy();
+    if (this._layoutWatcher) {
+      this._layoutWatcher.unsubscribe();
+    }
+  }
+
   /**
    * After the initial onChanges, build an mqActivation object that bridges
    * mql change events to onMediaQueryChange handlers
@@ -84,7 +104,43 @@ export class FlexOffsetDirective extends BaseFxDirective implements OnInit, OnCh
   // Protected methods
   // *********************************************
 
+  /** The flex-direction of this element's host container. Defaults to 'row'. */
+  protected _layout = 'row';
+
+  /**
+   * Subscription to the parent flex container's layout changes.
+   * Stored so we can unsubscribe when this directive is destroyed.
+   */
+  protected _layoutWatcher: Subscription;
+
+  /**
+   * If parent flow-direction changes, then update the margin property
+   * used to offset
+   */
+  protected watchParentFlow() {
+    if (this._container) {
+      // Subscribe to layout immediate parent direction changes (if any)
+      this._layoutWatcher = this._container.layout$.subscribe((direction) => {
+        // `direction` === null if parent container does not have a `fxLayout`
+        this._onLayoutChange(direction);
+      });
+    }
+  }
 
+  /**
+   * Caches the parent container's 'flex-direction' and updates the element's style.
+   * Used as a handler for layout change events from the parent flex container.
+   */
+  protected _onLayoutChange(direction?: string) {
+    this._layout = direction || this._layout || 'row';
+    this._updateWithValue();
+  }
+
+  /**
+   * Using the current fxFlexOffset value, update the inline CSS
+   * NOTE: this will assign `margin-left` if the parent flex-direction == 'row',
+   *       otherwise `margin-top` is used for the offset.
+   */
   protected _updateWithValue(value?: string|number) {
     value = value || this._queryInput('offset') || 0;
     if (this._mqActivation) {
@@ -101,6 +157,8 @@ export class FlexOffsetDirective extends BaseFxDirective implements OnInit, OnCh
       offset = offset + '%';
     }
 
-    return {'margin-left': `${offset}`};
+    // The flex-direction of this element's flex container. Defaults to 'row'.
+    let layout = this._getFlowDirection(this.parentElement, true);
+    return isFlowHorizontal(layout) ? {'margin-left': `${offset}`} : {'margin-top': `${offset}`};
   }
 }

src/lib/flexbox/api/flex.ts

@@ -27,6 +27,7 @@ import {MediaMonitor} from '../../media-query/media-monitor';
 import {LayoutDirective} from './layout';
 import {LayoutWrapDirective} from './layout-wrap';
 import {validateBasis} from '../../utils/basis-validator';
+import {isFlowHorizontal} from '../../utils/layout-validator';
 
 
 /** Built-in aliases for different flex-basis values. */
@@ -237,8 +238,8 @@ export class FlexDirective extends BaseFxDirective implements OnInit, OnChanges,
           break;
       }
 
-      let max = (direction === 'row') ? 'max-width' : 'max-height';
-      let min = (direction === 'row') ? 'min-width' : 'min-height';
+      let max = isFlowHorizontal(direction) ? 'max-width' : 'max-height';
+      let min = isFlowHorizontal(direction) ? 'min-width' : 'min-height';
 
       let usingCalc = (String(basis).indexOf('calc') > -1) || (basis == 'auto');
       let isPx = String(basis).indexOf('px') > -1 || usingCalc;
@@ -254,7 +255,4 @@ export class FlexDirective extends BaseFxDirective implements OnInit, OnChanges,
     return extendObject(css, {'box-sizing': 'border-box'});
   }
 
-  protected get parentElement(): any {
-    return this._elementRef.nativeElement.parentNode;
-  }
 }

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

@@ -24,8 +24,7 @@ import {MediaChange} from '../../media-query/media-change';
 import {MediaMonitor} from '../../media-query/media-monitor';
 
 import {LayoutDirective} from './layout';
-import {LAYOUT_VALUES} from '../../utils/layout-validator';
-
+import {LAYOUT_VALUES, isFlowHorizontal} from '../../utils/layout-validator';
 
 /**
  * 'layout-align' flexbox styling directive
@@ -204,8 +203,8 @@ export class LayoutAlignDirective extends BaseFxDirective implements OnInit, OnC
       // Use `null` values to remove style
       this._applyStyleToElement({
         'box-sizing': 'border-box',
-        'max-width': (layout === 'column') ? '100%' : null,
-        'max-height': (layout === 'row') ? '100%' : null
+        'max-width': !isFlowHorizontal(layout) ? '100%' : null,
+        'max-height': isFlowHorizontal(layout) ? '100%' : null
       });
     }
   }

src/lib/utils/layout-validator.ts

@@ -20,7 +20,7 @@ export function buildLayoutCSS(value: string) {
   * Validate the value to be one of the acceptable value options
   * Use default fallback of 'row'
   */
-function validateValue(value: string) {
+export function validateValue(value: string) {
   value = value ? value.toLowerCase() : '';
   let [direction, wrap] = value.split(' ');
   if (!LAYOUT_VALUES.find(x => x === direction)) {
@@ -29,6 +29,14 @@ function validateValue(value: string) {
   return [direction, validateWrapValue(wrap)];
 }
 
+/**
+ * Determine if the validated, flex-direction value specifies
+ * a horizontal/row flow.
+ */
+export function isFlowHorizontal(value: string): boolean {
+  let [flow, _] = validateValue(value);
+  return flow.indexOf('row') > -1;
+}
 
 /**
  * Convert layout-wrap='<value>' to expected flex-wrap style