Skip to content

Commit 1e96cea

Browse files
CaerusKaruCaerusKaru
authored
fix(fxFlex): apply correct flex-basis stylings (angular#629)
* 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 angular#277 Fixes angular#280 Fixes angular#323 Fixes angular#528 Fixes angular#534
1 parent de7ab76 commit 1e96cea

File tree

4 files changed

+135
-38
lines changed

4 files changed

+135
-38
lines changed

‎src/lib/core/style-utils/style-utils.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ export class StyleUtils {
7070
* Find the DOM element's inline style value (if any)
7171
*/
7272
lookupInlineStyle(element: HTMLElement, styleName: string): string {
73-
return element.style[styleName] || element.style.getPropertyValue(styleName);
73+
return element.style[styleName] || element.style.getPropertyValue(styleName) || '';
7474
}
7575

7676
/**
@@ -88,7 +88,7 @@ export class StyleUtils {
8888
}
8989
} else {
9090
if (this._serverModuleLoaded) {
91-
value = `${this._serverStylesheet.getStyleForElement(element, styleName)}`;
91+
value = this._serverStylesheet.getStyleForElement(element, styleName);
9292
}
9393
}
9494
}

‎src/lib/core/stylesheet-map.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,15 @@ export class StylesheetMap {
4040
/**
4141
* Retrieve a given style for an HTML element
4242
*/
43-
getStyleForElement(el: HTMLElement, styleName: string): string|number {
43+
getStyleForElement(el: HTMLElement, styleName: string): string {
4444
const styles = this.stylesheet.get(el);
45-
return (styles && styles.get(styleName)) || '';
45+
let value = '';
46+
if (styles) {
47+
const style = styles.get(styleName);
48+
if (typeof style === 'number' || typeof style === 'string') {
49+
value = style + '';
50+
}
51+
}
52+
return value;
4653
}
4754
}

‎src/lib/flex/flex/flex.spec.ts‎

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,49 @@ describe('flex directive', () => {
142142
}
143143
});
144144

145-
it('should add correct styles for `fxFlex="0%"` usage', () => {
145+
it('should add correct styles for flex-basis unitless 0 input', () => {
146+
componentWithTemplate(`<div fxFlex="1 1 0"></div>`);
147+
148+
fixture.detectChanges();
149+
expectNativeEl(fixture).toHaveStyle({
150+
'flex': '1 1 0%',
151+
'box-sizing': 'border-box',
152+
}, styler);
153+
154+
expectNativeEl(fixture).not.toHaveStyle({
155+
'max-width': '*'
156+
}, styler);
157+
});
158+
159+
it('should add correct styles for flex-basis 0px input', () => {
160+
componentWithTemplate(`<div fxFlex="1 1 0px"></div>`);
161+
162+
fixture.detectChanges();
163+
expectNativeEl(fixture).toHaveStyle({
164+
'flex': '1 1 0%',
165+
'box-sizing': 'border-box',
166+
}, styler);
167+
168+
expectNativeEl(fixture).not.toHaveStyle({
169+
'max-width': '*'
170+
}, styler);
171+
});
172+
173+
it('should add correct styles for noshrink with basis', () => {
174+
componentWithTemplate(`<div fxFlex="1 0 50%"></div>`);
175+
176+
fixture.detectChanges();
177+
expectNativeEl(fixture).toHaveStyle({
178+
'flex': '1 0 50%',
179+
'box-sizing': 'border-box',
180+
}, styler);
181+
182+
expectNativeEl(fixture).not.toHaveStyle({
183+
'max-width': '*'
184+
}, styler);
185+
});
186+
187+
it('should add correct styles for `fxFlex="2%"` usage', () => {
146188
componentWithTemplate(`<div fxFlex='2%'></div>`);
147189

148190
fixture.detectChanges();
@@ -282,7 +324,9 @@ describe('flex directive', () => {
282324
if (!(platform.FIREFOX || platform.TRIDENT)) {
283325
expectNativeEl(fixture).toHaveStyle({
284326
'box-sizing': 'border-box',
285-
'flex': '1 1 calc(30vw - 10px)'
327+
'flex-grow': '1',
328+
'flex-shrink': '1',
329+
'flex-basis': 'calc(30vw - 10px)'
286330
}, styler);
287331
}
288332
});
@@ -295,7 +339,9 @@ describe('flex directive', () => {
295339
setTimeout(() => {
296340
expectNativeEl(fixture).toHaveStyle({
297341
'box-sizing': 'border-box',
298-
'flex': '1 1 calc(75% - 10px)' // correct version has whitespace
342+
'flex-grow': '1',
343+
'flex-shrink': '1',
344+
'flex-basis': 'calc(75% - 10px)' // correct version has whitespace
299345
}, styler);
300346
});
301347
}

‎src/lib/flex/flex/flex.ts‎

Lines changed: 75 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -157,11 +157,29 @@ export class FlexDirective extends BaseFxDirective implements OnInit, OnChanges,
157157
// The flex-direction of this element's flex container. Defaults to 'row'.
158158
let layout = this._getFlowDirection(this.parentElement, true);
159159
let direction = (layout.indexOf('column') > -1) ? 'column' : 'row';
160-
let css, isValue;
160+
161+
let max = isFlowHorizontal(direction) ? 'max-width' : 'max-height';
162+
let min = isFlowHorizontal(direction) ? 'min-width' : 'min-height';
163+
164+
let hasCalc = String(basis).indexOf('calc') > -1;
165+
let usingCalc = hasCalc || (basis == 'auto');
166+
let isPercent = String(basis).indexOf('%') > -1 && !hasCalc;
167+
let hasUnits = String(basis).indexOf('px') > -1 || String(basis).indexOf('em') > -1 ||
168+
String(basis).indexOf('vw') > -1 || String(basis).indexOf('vh') > -1;
169+
let isPx = String(basis).indexOf('px') > -1 || usingCalc;
170+
171+
let isValue = (hasCalc || hasUnits);
161172

162173
grow = (grow == '0') ? 0 : grow;
163174
shrink = (shrink == '0') ? 0 : shrink;
164175

176+
// make box inflexible when shrink and grow are both zero
177+
// should not set a min when the grow is zero
178+
// should not set a max when the shrink is zero
179+
let isFixed = !grow && !shrink;
180+
181+
let css = {};
182+
165183
// flex-basis allows you to specify the initial/starting main-axis size of the element,
166184
// before anything else is computed. It can either be a percentage or an absolute value.
167185
// It is, however, not the breaking point for flex-grow/shrink properties
@@ -182,68 +200,94 @@ export class FlexDirective extends BaseFxDirective implements OnInit, OnChanges,
182200
};
183201
switch (basis || '') {
184202
case '':
185-
css = extendObject(clearStyles, {'flex': `${grow} ${shrink} 0.000000001px`});
203+
basis = '0.000000001px';
186204
break;
187205
case 'initial': // default
188206
case 'nogrow':
189207
grow = 0;
190-
css = extendObject(clearStyles, {'flex': '0 1 auto'});
208+
basis = 'auto';
191209
break;
192210
case 'grow':
193-
css = extendObject(clearStyles, {'flex': '1 1 100%'});
211+
basis = '100%';
194212
break;
195213
case 'noshrink':
196214
shrink = 0;
197-
css = extendObject(clearStyles, {'flex': '1 0 auto'});
215+
basis = 'auto';
198216
break;
199217
case 'auto':
200-
css = extendObject(clearStyles, {'flex': `${grow} ${shrink} auto`});
201218
break;
202219
case 'none':
203220
grow = 0;
204221
shrink = 0;
205-
css = extendObject(clearStyles, {'flex': '0 0 auto'});
222+
basis = 'auto';
206223
break;
207224
default:
208-
let hasCalc = String(basis).indexOf('calc') > -1;
209-
let isPercent = String(basis).indexOf('%') > -1 && !hasCalc;
210-
211-
isValue = hasCalc ||
212-
String(basis).indexOf('px') > -1 ||
213-
String(basis).indexOf('em') > -1 ||
214-
String(basis).indexOf('vw') > -1 ||
215-
String(basis).indexOf('vh') > -1;
216-
217225
// Defaults to percentage sizing unless `px` is explicitly set
218226
if (!isValue && !isPercent && !isNaN(basis as any)) {
219227
basis = basis + '%';
220228
}
221229

230+
// Fix for issue 280
231+
if (basis === '0%') {
232+
isValue = true;
233+
}
234+
222235
if (basis === '0px') {
223236
basis = '0%';
224237
}
225238

226-
css = extendObject(clearStyles, { // fix issue #5345
227-
'flex': `${grow} ${shrink} ${isValue ? basis : '100%'}`
228-
});
239+
// fix issue #5345
240+
if (hasCalc) {
241+
css = extendObject(clearStyles, {
242+
'flex-grow': grow,
243+
'flex-shrink': shrink,
244+
'flex-basis': isValue ? basis : '100%'
245+
});
246+
} else {
247+
css = extendObject(clearStyles, {
248+
'flex': `${grow} ${shrink} ${isValue ? basis : '100%'}`
249+
});
250+
}
251+
229252
break;
230253
}
231254

232-
let max = isFlowHorizontal(direction) ? 'max-width' : 'max-height';
233-
let min = isFlowHorizontal(direction) ? 'min-width' : 'min-height';
234-
235-
let usingCalc = (String(basis).indexOf('calc') > -1) || (basis == 'auto');
236-
let isPx = String(basis).indexOf('px') > -1 || usingCalc;
255+
if (!(css['flex'] || css['flex-grow'])) {
256+
if (hasCalc) {
257+
css = extendObject(clearStyles, {
258+
'flex-grow': grow,
259+
'flex-shrink': shrink,
260+
'flex-basis': basis
261+
});
262+
} else {
263+
css = extendObject(clearStyles, {
264+
'flex': `${grow} ${shrink} ${basis}`
265+
});
266+
}
267+
}
237268

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

239-
// make box inflexible when shrink and grow are both zero
240-
// should not set a min when the grow is zero
241-
// should not set a max when the shrink is zero
242-
let isFixed = !grow && !shrink;
243-
css[min] = (basis == '0%') ? 0 : isFixed || (isPx && grow) ? basis : null;
244-
css[max] = (basis == '0%') ? 0 : isFixed || (!usingCalc && shrink) ? basis : null;
276+
// Fix for issue 528
277+
if (!css[min] && !css[max]) {
278+
if (hasCalc) {
279+
css = extendObject(clearStyles, {
280+
'flex-grow': grow,
281+
'flex-shrink': shrink,
282+
'flex-basis': basis
283+
});
284+
} else {
285+
css = extendObject(clearStyles, {
286+
'flex': `${grow} ${shrink} ${basis}`
287+
});
288+
}
289+
}
245290

246291
return extendObject(css, {'box-sizing': 'border-box'});
247292
}
248-
249293
}

0 commit comments

Comments
 (0)