Skip to content

Commit

Permalink
fix(fxFlex): apply correct flex-basis stylings
Browse files Browse the repository at this point in the history 
* Fix for when flex-basis is unitless and 0
* Fix for when no width/height is applied and flex-basis should be
  set
* Fix for IE flex-basis with calc values
* Fix for SSR properties set to 0

Fixes #277
Fixes #280
Fixes #323
Fixes #528
Fixes #534
  • Loading branch information
@CaerusKaru
CaerusKaru committed Mar 9, 2018
1 parent de7ab76 commit 6f8c7ab
Show file tree
Hide file tree
Showing 4 changed files with 135 additions and 38 deletions.
4 changes: 2 additions & 2 deletions src/lib/core/style-utils/style-utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export class StyleUtils {
* Find the DOM element's inline style value (if any)
*/
lookupInlineStyle(element: HTMLElement, styleName: string): string {
return element.style[styleName] || element.style.getPropertyValue(styleName);
return element.style[styleName] || element.style.getPropertyValue(styleName) || '';
}

/**
Expand All @@ -88,7 +88,7 @@ export class StyleUtils {
}
} else {
if (this._serverModuleLoaded) {
value = `${this._serverStylesheet.getStyleForElement(element, styleName)}`;
value = this._serverStylesheet.getStyleForElement(element, styleName);
}
}
}
Expand Down
11 changes: 9 additions & 2 deletions src/lib/core/stylesheet-map.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,15 @@ export class StylesheetMap {
/**
* Retrieve a given style for an HTML element
*/
getStyleForElement(el: HTMLElement, styleName: string): string|number {
getStyleForElement(el: HTMLElement, styleName: string): string {
const styles = this.stylesheet.get(el);
return (styles && styles.get(styleName)) || '';
let value = '';
if (styles) {
const style = styles.get(styleName);
if (typeof style === 'number' || typeof style === 'string') {
value = style + '';
}
}
return value;
}
}
52 changes: 49 additions & 3 deletions src/lib/flex/flex/flex.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,49 @@ describe('flex directive', () => {
}
});

it('should add correct styles for `fxFlex="0%"` usage', () => {
it('should add correct styles for flex-basis unitless 0 input', () => {
componentWithTemplate(`<div fxFlex="1 1 0"></div>`);

fixture.detectChanges();
expectNativeEl(fixture).toHaveStyle({
'flex': '1 1 0%',
'box-sizing': 'border-box',
}, styler);

expectNativeEl(fixture).not.toHaveStyle({
'max-width': '*'
}, styler);
});

it('should add correct styles for flex-basis 0px input', () => {
componentWithTemplate(`<div fxFlex="1 1 0px"></div>`);

fixture.detectChanges();
expectNativeEl(fixture).toHaveStyle({
'flex': '1 1 0%',
'box-sizing': 'border-box',
}, styler);

expectNativeEl(fixture).not.toHaveStyle({
'max-width': '*'
}, styler);
});

it('should add correct styles for noshrink with basis', () => {
componentWithTemplate(`<div fxFlex="1 0 50%"></div>`);

fixture.detectChanges();
expectNativeEl(fixture).toHaveStyle({
'flex': '1 0 50%',
'box-sizing': 'border-box',
}, styler);

expectNativeEl(fixture).not.toHaveStyle({
'max-width': '*'
}, styler);
});

it('should add correct styles for `fxFlex="2%"` usage', () => {
componentWithTemplate(`<div fxFlex='2%'></div>`);

fixture.detectChanges();
Expand Down Expand Up @@ -282,7 +324,9 @@ describe('flex directive', () => {
if (!(platform.FIREFOX || platform.TRIDENT)) {
expectNativeEl(fixture).toHaveStyle({
'box-sizing': 'border-box',
'flex': '1 1 calc(30vw - 10px)'
'flex-grow': '1',
'flex-shrink': '1',
'flex-basis': 'calc(30vw - 10px)'
}, styler);
}
});
Expand All @@ -295,7 +339,9 @@ describe('flex directive', () => {
setTimeout(() => {
expectNativeEl(fixture).toHaveStyle({
'box-sizing': 'border-box',
'flex': '1 1 calc(75% - 10px)' // correct version has whitespace
'flex-grow': '1',
'flex-shrink': '1',
'flex-basis': 'calc(75% - 10px)' // correct version has whitespace
}, styler);
});
}
Expand Down
106 changes: 75 additions & 31 deletions src/lib/flex/flex/flex.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,29 @@ export class FlexDirective extends BaseFxDirective implements OnInit, OnChanges,
// The flex-direction of this element's flex container. Defaults to 'row'.
let layout = this._getFlowDirection(this.parentElement, true);
let direction = (layout.indexOf('column') > -1) ? 'column' : 'row';
let css, isValue;

let max = isFlowHorizontal(direction) ? 'max-width' : 'max-height';
let min = isFlowHorizontal(direction) ? 'min-width' : 'min-height';

let hasCalc = String(basis).indexOf('calc') > -1;
let usingCalc = hasCalc || (basis == 'auto');
let isPercent = String(basis).indexOf('%') > -1 && !hasCalc;
let hasUnits = String(basis).indexOf('px') > -1 || String(basis).indexOf('em') > -1 ||
String(basis).indexOf('vw') > -1 || String(basis).indexOf('vh') > -1;
let isPx = String(basis).indexOf('px') > -1 || usingCalc;

let isValue = (hasCalc || hasUnits);

grow = (grow == '0') ? 0 : grow;
shrink = (shrink == '0') ? 0 : shrink;

// make box inflexible when shrink and grow are both zero
// should not set a min when the grow is zero
// should not set a max when the shrink is zero
let isFixed = !grow && !shrink;

let css = {};

// flex-basis allows you to specify the initial/starting main-axis size of the element,
// before anything else is computed. It can either be a percentage or an absolute value.
// It is, however, not the breaking point for flex-grow/shrink properties
Expand All @@ -182,68 +200,94 @@ export class FlexDirective extends BaseFxDirective implements OnInit, OnChanges,
};
switch (basis || '') {
case '':
css = extendObject(clearStyles, {'flex': `${grow} ${shrink} 0.000000001px`});
basis = '0.000000001px';
break;
case 'initial': // default
case 'nogrow':
grow = 0;
css = extendObject(clearStyles, {'flex': '0 1 auto'});
basis = 'auto';
break;
case 'grow':
css = extendObject(clearStyles, {'flex': '1 1 100%'});
basis = '100%';
break;
case 'noshrink':
shrink = 0;
css = extendObject(clearStyles, {'flex': '1 0 auto'});
basis = 'auto';
break;
case 'auto':
css = extendObject(clearStyles, {'flex': `${grow} ${shrink} auto`});
break;
case 'none':
grow = 0;
shrink = 0;
css = extendObject(clearStyles, {'flex': '0 0 auto'});
basis = 'auto';
break;
default:
let hasCalc = String(basis).indexOf('calc') > -1;
let isPercent = String(basis).indexOf('%') > -1 && !hasCalc;

isValue = hasCalc ||
String(basis).indexOf('px') > -1 ||
String(basis).indexOf('em') > -1 ||
String(basis).indexOf('vw') > -1 ||
String(basis).indexOf('vh') > -1;

// Defaults to percentage sizing unless `px` is explicitly set
if (!isValue && !isPercent && !isNaN(basis as any)) {
basis = basis + '%';
}

// Fix for issue 280
if (basis === '0%') {
isValue = true;
}

if (basis === '0px') {
basis = '0%';
}

css = extendObject(clearStyles, { // fix issue #5345
'flex': `${grow} ${shrink} ${isValue ? basis : '100%'}`
});
// fix issue #5345
if (hasCalc) {
css = extendObject(clearStyles, {
'flex-grow': grow,
'flex-shrink': shrink,
'flex-basis': isValue ? basis : '100%'
});
} else {
css = extendObject(clearStyles, {
'flex': `${grow} ${shrink} ${isValue ? basis : '100%'}`
});
}

break;
}

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;
if (!(css['flex'] || css['flex-grow'])) {
if (hasCalc) {
css = extendObject(clearStyles, {
'flex-grow': grow,
'flex-shrink': shrink,
'flex-basis': basis
});
} else {
css = extendObject(clearStyles, {
'flex': `${grow} ${shrink} ${basis}`
});
}
}

// Fix for issues 277 and 534
// TODO(CaerusKaru): convert this to just width/height
if (basis !== '0%') {
css[min] = isFixed || (isPx && grow) ? basis : null;
css[max] = isFixed || (!usingCalc && shrink) ? basis : null;
}

// make box inflexible when shrink and grow are both zero
// should not set a min when the grow is zero
// should not set a max when the shrink is zero
let isFixed = !grow && !shrink;
css[min] = (basis == '0%') ? 0 : isFixed || (isPx && grow) ? basis : null;
css[max] = (basis == '0%') ? 0 : isFixed || (!usingCalc && shrink) ? basis : null;
// Fix for issue 528
if (!css[min] && !css[max]) {
if (hasCalc) {
css = extendObject(clearStyles, {
'flex-grow': grow,
'flex-shrink': shrink,
'flex-basis': basis
});
} else {
css = extendObject(clearStyles, {
'flex': `${grow} ${shrink} ${basis}`
});
}
}

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

}

0 comments on commit 6f8c7ab

Please sign in to comment.