Skip to content

Commit

Permalink
fix(fxFlexOffset): use parent flow direction for margin property (#369)
Browse files Browse the repository at this point in the history 
`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.
  • Loading branch information
@ThomasBurleson @mmalerba
ThomasBurleson authored and mmalerba committed Aug 9, 2017
1 parent 37a0b85 commit f0473e9
Show file tree
Hide file tree
Showing 6 changed files with 242 additions and 13 deletions.
7 changes: 7 additions & 0 deletions src/lib/flexbox/api/base.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
159 changes: 159 additions & 0 deletions src/lib/flexbox/api/flex-offset.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -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';
}


64 changes: 61 additions & 3 deletions src/lib/flexbox/api/flex-offset.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();
}

// *********************************************
Expand All @@ -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
Expand All @@ -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) {
Expand All @@ -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}`};
}
}
8 changes: 3 additions & 5 deletions src/lib/flexbox/api/flex.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}
}
7 changes: 3 additions & 4 deletions src/lib/flexbox/api/layout-align.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
});
}
}
Expand Down
10 changes: 9 additions & 1 deletion src/lib/utils/layout-validator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -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
Expand Down

0 comments on commit f0473e9

Please sign in to comment.