Skip to content

Commit

Permalink
fix(fxFlex): fix non-wrapping behavior and default fxFlex value
Browse files Browse the repository at this point in the history 
* Set the non-wrapping behavior for fxFlex to be 100% instead of
  the width/height value
* Set the default value for fxFlex to auto instead of 1e-9px
  • Loading branch information
@CaerusKaru @ThomasBurleson
CaerusKaru authored and ThomasBurleson committed Mar 19, 2018
1 parent fbd5507 commit 3cfafd1
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 100 deletions.
21 changes: 5 additions & 16 deletions src/lib/flex/flex-offset/flex-offset.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import {Component, PLATFORM_ID} from '@angular/core';
import {CommonModule, isPlatformServer} from '@angular/common';
import {ComponentFixture, inject, TestBed} from '@angular/core/testing';
import {Platform, PlatformModule} from '@angular/cdk/platform';
import {DIR_DOCUMENT} from '@angular/cdk/bidi';
import {SERVER_TOKEN, StyleUtils} from '@angular/flex-layout/core';

Expand All @@ -25,15 +24,13 @@ describe('flex-offset directive', () => {
let fixture: ComponentFixture<any>;
let fakeDocument: {body: {dir?: string}, documentElement: {dir?: string}};
let styler: StyleUtils;
let platform: Platform;
let platformId: Object;
let componentWithTemplate = (template: string) => {
fixture = makeCreateTestComponent(() => TestFlexComponent)(template);

inject([StyleUtils, Platform, PLATFORM_ID],
(_styler: StyleUtils, _platform: Platform, _platformId: Object) => {
inject([StyleUtils, PLATFORM_ID],
(_styler: StyleUtils, _platformId: Object) => {
styler = _styler;
platform = _platform;
platformId = _platformId;
})();
};
Expand All @@ -44,7 +41,7 @@ describe('flex-offset directive', () => {

// Configure testbed to prepare services
TestBed.configureTestingModule({
imports: [CommonModule, FlexLayoutModule, PlatformModule],
imports: [CommonModule, FlexLayoutModule],
declarations: [TestFlexComponent],
providers: [
{provide: DIR_DOCUMENT, useValue: fakeDocument},
Expand All @@ -61,22 +58,14 @@ describe('flex-offset directive', () => {

let dom = fixture.debugElement.children[0];
expectEl(dom).toHaveStyle({'margin-left': '32px'}, styler);
if (platform.BLINK) {
expectEl(dom).toHaveStyle({'flex': '1 1 1e-09px'}, styler);
} else if (platform.FIREFOX) {
expectEl(dom).toHaveStyle({'flex': '1 1 1e-9px'}, styler);
} else if (platform.EDGE || platform.TRIDENT) {
expectEl(dom).toHaveStyle({'flex': '1 1 0px'}, styler);
} else {
expectEl(dom).toHaveStyle({'flex': '1 1 0.000000001px'}, styler);
}
expectEl(dom).toHaveStyle({'flex': '1 1 0%'}, styler);
});


it('should work with percentage values', () => {
componentWithTemplate(`<div fxFlexOffset='17' fxFlex='37'></div>`);
expectNativeEl(fixture).toHaveStyle({
'flex': '1 1 37%',
'flex': '1 1 100%',
'box-sizing': 'border-box',
'margin-left': '17%'
}, styler);
Expand Down
12 changes: 6 additions & 6 deletions src/lib/flex/flex-offset/flex-offset.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
} from '@angular/flex-layout/core';
import {Subscription} from 'rxjs/Subscription';

import {LayoutDirective} from '../layout/layout';
import {Layout, LayoutDirective} from '../layout/layout';
import {isFlowHorizontal} from '../../utils/layout-validator';

/**
Expand Down Expand Up @@ -117,7 +117,7 @@ export class FlexOffsetDirective extends BaseFxDirective implements OnInit, OnCh
// *********************************************

/** The flex-direction of this element's host container. Defaults to 'row'. */
protected _layout = 'row';
protected _layout = {direction: 'row', wrap: false};

/**
* Subscription to the parent flex container's layout changes.
Expand All @@ -132,9 +132,9 @@ export class FlexOffsetDirective extends BaseFxDirective implements OnInit, OnCh
protected watchParentFlow() {
if (this._container) {
// Subscribe to layout immediate parent direction changes (if any)
this._layoutWatcher = this._container.layout$.subscribe((direction) => {
this._layoutWatcher = this._container.layout$.subscribe((layout) => {
// `direction` === null if parent container does not have a `fxLayout`
this._onLayoutChange(direction);
this._onLayoutChange(layout);
});
}
}
Expand All @@ -143,8 +143,8 @@ export class FlexOffsetDirective extends BaseFxDirective implements OnInit, OnCh
* 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';
protected _onLayoutChange(layout?: Layout) {
this._layout = layout || this._layout || {direction: 'row', wrap: false};
this._updateWithValue();
}

Expand Down
72 changes: 18 additions & 54 deletions src/lib/flex/flex/flex.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,16 +72,7 @@ describe('flex directive', () => {

let dom = fixture.debugElement.children[0];
expectEl(dom).toHaveStyle({'box-sizing': 'border-box'}, styler);

if (platform.BLINK) {
expectEl(dom).toHaveStyle({'flex': '1 1 1e-09px'}, styler);
} else if (platform.FIREFOX) {
expectEl(dom).toHaveStyle({'flex': '1 1 1e-9px'}, styler);
} else if (platform.EDGE || platform.TRIDENT) {
expectEl(dom).toHaveStyle({'flex': '1 1 0px'}, styler);
} else {
expectEl(dom).toHaveStyle({'flex': '1 1 0.000000001px'}, styler);
}
expectEl(dom).toHaveStyle({'flex': '1 1 0%'}, styler);
});

it('should apply `fxGrow` value to flex-grow when used default `fxFlex`', () => {
Expand All @@ -91,16 +82,7 @@ describe('flex directive', () => {
let dom = fixture.debugElement.children[0];

expectEl(dom).toHaveStyle({'box-sizing': 'border-box'}, styler);

if (platform.BLINK) {
expectEl(dom).toHaveStyle({'flex': '10 1 1e-09px'}, styler);
} else if (platform.FIREFOX) {
expectEl(dom).toHaveStyle({'flex': '10 1 1e-9px'}, styler);
} else if (platform.EDGE || platform.TRIDENT) {
expectEl(dom).toHaveStyle({'flex': '10 1 0px'}, styler);
} else {
expectEl(dom).toHaveStyle({'flex': '10 1 0.000000001px'}, styler);
}
expectEl(dom).toHaveStyle({'flex': '10 1 0%'}, styler);
});

it('should apply `fxShrink` value to flex-shrink when used default `fxFlex`', () => {
Expand All @@ -110,16 +92,7 @@ describe('flex directive', () => {
let dom = fixture.debugElement.children[0];

expectEl(dom).toHaveStyle({'box-sizing': 'border-box'}, styler);

if (platform.BLINK) {
expectEl(dom).toHaveStyle({'flex': '1 10 1e-09px'}, styler);
} else if (platform.FIREFOX) {
expectEl(dom).toHaveStyle({'flex': '1 10 1e-9px'}, styler);
} else if (platform.EDGE || platform.TRIDENT) {
expectEl(dom).toHaveStyle({'flex': '1 10 0px'}, styler);
} else {
expectEl(dom).toHaveStyle({'flex': '1 10 0.000000001px'}, styler);
}
expectEl(dom).toHaveStyle({'flex': '1 10 0%'}, styler);
});

it('should apply both `fxGrow` and `fxShrink` when used with default fxFlex', () => {
Expand All @@ -128,16 +101,7 @@ describe('flex directive', () => {

let dom = fixture.debugElement.children[0];
expectEl(dom).toHaveStyle({'box-sizing': 'border-box'}, styler);

if (platform.BLINK) {
expectEl(dom).toHaveStyle({'flex': '4 5 1e-09px'}, styler);
} else if (platform.FIREFOX) {
expectEl(dom).toHaveStyle({'flex': '4 5 1e-9px'}, styler);
} else if (platform.EDGE || platform.TRIDENT) {
expectEl(dom).toHaveStyle({'flex': '4 5 0px'}, styler);
} else {
expectEl(dom).toHaveStyle({'flex': '4 5 0.000000001px'}, styler);
}
expectEl(dom).toHaveStyle({'flex': '4 5 0%'}, styler);
});

it('should add correct styles for flex-basis unitless 0 input', () => {
Expand Down Expand Up @@ -188,15 +152,15 @@ describe('flex directive', () => {
fixture.detectChanges();
expectNativeEl(fixture).toHaveStyle({
'max-width': '2%',
'flex': '1 1 2%',
'flex': '1 1 100%',
'box-sizing': 'border-box',
}, styler);
});

it('should work with percentage values', () => {
componentWithTemplate(`<div fxFlex='37%'></div>`);
expectNativeEl(fixture).toHaveStyle({
'flex': '1 1 37%',
'flex': '1 1 100%',
'max-width': '37%',
'box-sizing': 'border-box',
}, styler);
Expand Down Expand Up @@ -470,23 +434,23 @@ describe('flex directive', () => {
fixture.detectChanges();
expectEl(queryFor(fixture, '[fxFlex]')[0])
.toHaveStyle({
'flex': '1 1 37%',
'flex': '1 1 100%',
'max-height': '37%',
}, styler);
});

it('should set max-width for `fxFlex="<%val>"`', () => {
componentWithTemplate(`<div fxFlex='37%'></div>`);
expectNativeEl(fixture).toHaveStyle({
'flex': '1 1 37%',
'flex': '1 1 100%',
'max-width': '37%',
}, styler);
});

it('should set max-width for `fxFlex="2%"` usage', () => {
componentWithTemplate(`<div fxFlex='2%'></div>`);
expectNativeEl(fixture).toHaveStyle({
'flex': '1 1 2%',
'flex': '1 1 100%',
'max-width': '2%',
}, styler);
});
Expand All @@ -508,15 +472,15 @@ describe('flex directive', () => {
fixture.detectChanges();

expectNativeEl(fixture).toHaveStyle({
'flex': '1 1 50%',
'flex': '1 1 100%',
'max-width': '50%'
}, styler);

matchMedia.activate('sm', true);
fixture.detectChanges();

expectNativeEl(fixture).toHaveStyle({
'flex': '1 1 33%',
'flex': '1 1 100%',
'max-width': '33%'
}, styler);
});
Expand Down Expand Up @@ -544,9 +508,9 @@ describe('flex directive', () => {
fixture.detectChanges();

nodes = queryFor(fixture, '[fxFlex]');
expectEl(nodes[0]).toHaveStyle({'flex': '1 1 50%', 'max-height': '50%'}, styler);
expectEl(nodes[1]).toHaveStyle({'flex': '1 1 24.4%', 'max-height': '24.4%'}, styler);
expectEl(nodes[2]).toHaveStyle({'flex': '1 1 25.6%', 'max-height': '25.6%'}, styler);
expectEl(nodes[0]).toHaveStyle({'flex': '1 1 100%', 'max-height': '50%'}, styler);
expectEl(nodes[1]).toHaveStyle({'flex': '1 1 100%', 'max-height': '24.4%'}, styler);
expectEl(nodes[2]).toHaveStyle({'flex': '1 1 100%', 'max-height': '25.6%'}, styler);

matchMedia.activate('sm');
fixture.detectChanges();
Expand Down Expand Up @@ -595,9 +559,9 @@ describe('flex directive', () => {
fixture.detectChanges();
nodes = queryFor(fixture, '[fxFlex]');

expectEl(nodes[0]).toHaveStyle({'flex': '1 1 50%', 'max-height': '50%'}, styler);
expectEl(nodes[1]).toHaveStyle({'flex': '1 1 24.4%', 'max-height': '24.4%'}, styler);
expectEl(nodes[2]).toHaveStyle({'flex': '1 1 25.6%', 'max-height': '25.6%'}, styler);
expectEl(nodes[0]).toHaveStyle({'flex': '1 1 100%', 'max-height': '50%'}, styler);
expectEl(nodes[1]).toHaveStyle({'flex': '1 1 100%', 'max-height': '24.4%'}, styler);
expectEl(nodes[2]).toHaveStyle({'flex': '1 1 100%', 'max-height': '25.6%'}, styler);
});

it('should fallback to the default layout from lt-md selectors', () => {
Expand All @@ -619,7 +583,7 @@ describe('flex directive', () => {
nodes = queryFor(fixture, '[fxFlex]');

expectEl(nodes[0]).toHaveStyle({
'flex': '1 1 50%',
'flex': '1 1 100%',
'max-height': '50%'
}, styler);

Expand Down
27 changes: 13 additions & 14 deletions src/lib/flex/flex/flex.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {
import {Subscription} from 'rxjs/Subscription';

import {extendObject} from '../../utils/object-extend';
import {LayoutDirective} from '../layout/layout';
import {Layout, LayoutDirective} from '../layout/layout';
import {validateBasis} from '../../utils/basis-validator';
import {isFlowHorizontal} from '../../utils/layout-validator';

Expand All @@ -52,7 +52,7 @@ export type FlexBasisAlias = 'grow' | 'initial' | 'auto' | 'none' | 'nogrow' | '
export class FlexDirective extends BaseFxDirective implements OnInit, OnChanges, OnDestroy {

/** The flex-direction of this element's flex container. Defaults to 'row'. */
protected _layout: string;
protected _layout: Layout;

/**
* Subscription to the parent flex container's layout changes.
Expand Down Expand Up @@ -98,9 +98,9 @@ export class FlexDirective extends BaseFxDirective implements OnInit, OnChanges,
if (_container) {
// If this flex item is inside of a flex container marked with
// Subscribe to layout immediate parent direction changes
this._layoutWatcher = _container.layout$.subscribe((direction) => {
this._layoutWatcher = _container.layout$.subscribe((layout) => {
// `direction` === null if parent container does not have a `fxLayout`
this._onLayoutChange(direction);
this._onLayoutChange(layout);
});
}
}
Expand Down Expand Up @@ -139,8 +139,8 @@ export class FlexDirective extends BaseFxDirective implements OnInit, OnChanges,
* 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';
protected _onLayoutChange(layout?: Layout) {
this._layout = layout || this._layout || {direction: 'row', wrap: false};
this._updateStyle();
}

Expand Down Expand Up @@ -208,7 +208,7 @@ export class FlexDirective extends BaseFxDirective implements OnInit, OnChanges,
};
switch (basis || '') {
case '':
basis = MIN_FLEX;
basis = direction === 'row' ? '0%' : 'auto';
break;
case 'initial': // default
case 'nogrow':
Expand Down Expand Up @@ -275,8 +275,7 @@ export class FlexDirective extends BaseFxDirective implements OnInit, OnChanges,
}

// Fix for issues 277 and 534
// TODO(CaerusKaru): convert this to just width/height
if (basis !== '0%' && basis !== MIN_FLEX) {
if (basis !== '0%') {
css[min] = isFixed || (isPx && grow) ? basis : null;
css[max] = isFixed || (!usingCalc && shrink) ? basis : null;
}
Expand All @@ -296,13 +295,13 @@ export class FlexDirective extends BaseFxDirective implements OnInit, OnChanges,
}
} else {
// Fix for issue 660
css[hasCalc ? 'flex-basis' : 'flex'] = css[max] ?
(hasCalc ? css[max] : `${grow} ${shrink} ${css[max]}`) :
(hasCalc ? css[min] : `${grow} ${shrink} ${css[min]}`);
if (this._layout && this._layout.wrap) {
css[hasCalc ? 'flex-basis' : 'flex'] = css[max] ?
(hasCalc ? css[max] : `${grow} ${shrink} ${css[max]}`) :
(hasCalc ? css[min] : `${grow} ${shrink} ${css[min]}`);
}
}

return extendObject(css, {'box-sizing': 'border-box'});
}
}

const MIN_FLEX = '0.000000001px';
6 changes: 3 additions & 3 deletions src/lib/flex/layout-align/layout-align.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {BaseFxDirective, MediaChange, MediaMonitor, StyleUtils} from '@angular/f
import {Subscription} from 'rxjs/Subscription';

import {extendObject} from '../../utils/object-extend';
import {LayoutDirective} from '../layout/layout';
import {Layout, LayoutDirective} from '../layout/layout';
import {LAYOUT_VALUES, isFlowHorizontal} from '../../utils/layout-validator';

/**
Expand Down Expand Up @@ -124,8 +124,8 @@ export class LayoutAlignDirective extends BaseFxDirective implements OnInit, OnC
/**
* Cache the parent container 'flex-direction' and update the 'flex' styles
*/
protected _onLayoutChange(direction) {
this._layout = (direction || '').toLowerCase();
protected _onLayoutChange(layout: Layout) {
this._layout = (layout.direction || '').toLowerCase();
if (!LAYOUT_VALUES.find(x => x === this._layout)) {
this._layout = 'row';
}
Expand Down
6 changes: 3 additions & 3 deletions src/lib/flex/layout-gap/layout-gap.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {Directionality} from '@angular/cdk/bidi';
import {BaseFxDirective, MediaChange, MediaMonitor, StyleUtils} from '@angular/flex-layout/core';
import {Subscription} from 'rxjs/Subscription';

import {LayoutDirective} from '../layout/layout';
import {Layout, LayoutDirective} from '../layout/layout';
import {LAYOUT_VALUES} from '../../utils/layout-validator';

/**
Expand Down Expand Up @@ -143,8 +143,8 @@ export class LayoutGapDirective extends BaseFxDirective
/**
* Cache the parent container 'flex-direction' and update the 'margin' styles
*/
protected _onLayoutChange(direction) {
this._layout = (direction || '').toLowerCase();
protected _onLayoutChange(layout: Layout) {
this._layout = (layout.direction || '').toLowerCase();
if (!LAYOUT_VALUES.find(x => x === this._layout)) {
this._layout = 'row';
}
Expand Down
Loading

0 comments on commit 3cfafd1

Please sign in to comment.