From fbba32defb010e985cc9849a8cc2fc40b68080f5 Mon Sep 17 00:00:00 2001 From: Nicolas Gallagher Date: Mon, 18 Sep 2017 19:15:02 -0700 Subject: [PATCH] [fix] cross-browser flex styles Fix #616 Close #648 --- .../renderApplication-test.js.snap | 10 ++-- .../__tests__/createReactDOMStyle-test.js | 49 ++++++++++--------- src/apis/StyleSheet/createReactDOMStyle.js | 49 ++++++++++++++----- 3 files changed, 68 insertions(+), 40 deletions(-) diff --git a/src/apis/AppRegistry/__tests__/__snapshots__/renderApplication-test.js.snap b/src/apis/AppRegistry/__tests__/__snapshots__/renderApplication-test.js.snap index ee0f15ff..919385de 100644 --- a/src/apis/AppRegistry/__tests__/__snapshots__/renderApplication-test.js.snap +++ b/src/apis/AppRegistry/__tests__/__snapshots__/renderApplication-test.js.snap @@ -50,10 +50,9 @@ exports[`apis/AppRegistry/renderApplication getApplication 3`] = ` .rn-boxSizing-deolkf{box-sizing:border-box} .rn-display-6koalj{display:-webkit-box;display:-moz-box;display:-ms-flexbox;display:-webkit-flex;display:flex} .rn-display-xoduu5{display:-webkit-inline-box;display:-moz-inline-box;display:-ms-inline-flexbox;display:-webkit-inline-flex;display:inline-flex} -.rn-flexShrink-1qe8dj5{-webkit-flex-shrink:0;flex-shrink:0} -.rn-flexShrink-1wbh5a2{-webkit-flex-shrink:1;flex-shrink:1} -.rn-flexBasis-1mlwlqe{-webkit-flex-basis:auto;flex-basis:auto} -.rn-flexBasis-1ro0kt6{-webkit-flex-basis:0%;flex-basis:0%} +.rn-flexShrink-1pxmb3b{-webkit-flex-shrink:0 !important;flex-shrink:0 !important} +.rn-flexShrink-1awmn5t{-webkit-flex-shrink:1 !important;flex-shrink:1 !important} +.rn-flexBasis-7vfszb{-webkit-flex-basis:auto !important;flex-basis:auto !important} .rn-flexDirection-eqz5dr{-webkit-box-direction:normal;-webkit-box-orient:vertical;-webkit-flex-direction:column;flex-direction:column} .rn-marginTop-1mnahxq{margin-top:0px} .rn-marginRight-61z16t{margin-right:0px} @@ -67,5 +66,6 @@ exports[`apis/AppRegistry/renderApplication getApplication 3`] = ` .rn-paddingLeft-gy4na3{padding-left:0px} .rn-zIndex-1lgpqti{z-index:0} .rn-zIndex-1wyyakw{z-index:-1} -.rn-flexGrow-16y2uox{-webkit-flex-grow:1;flex-grow:1}" +.rn-flex-13awgt0{-webkit-flex:1;flex:1} +.rn-flexGrow-1m1wadx{-webkit-flex-grow:1 !important;flex-grow:1 !important}" `; diff --git a/src/apis/StyleSheet/__tests__/createReactDOMStyle-test.js b/src/apis/StyleSheet/__tests__/createReactDOMStyle-test.js index 1a5a27e8..8ba7fa9c 100644 --- a/src/apis/StyleSheet/__tests__/createReactDOMStyle-test.js +++ b/src/apis/StyleSheet/__tests__/createReactDOMStyle-test.js @@ -26,44 +26,44 @@ describe('apis/StyleSheet/createReactDOMStyle', () => { test('flex defaults', () => { expect(createReactDOMStyle({ display: 'flex' })).toEqual({ display: 'flex', - flexShrink: 0, - flexBasis: 'auto' + flexShrink: '0 !important', + flexBasis: 'auto !important' }); }); test('flex: -1', () => { expect(createReactDOMStyle({ display: 'flex', flex: -1 })).toEqual({ display: 'flex', - flexGrow: 0, - flexShrink: 1, - flexBasis: 'auto' + flexGrow: '0 !important', + flexShrink: '1 !important', + flexBasis: 'auto !important' }); }); test('flex: 0', () => { expect(createReactDOMStyle({ display: 'flex', flex: 0 })).toEqual({ display: 'flex', - flexGrow: 0, - flexShrink: 0, - flexBasis: 'auto' + flexGrow: '0 !important', + flexShrink: '0 !important', + flexBasis: 'auto !important' }); }); test('flex: 1', () => { expect(createReactDOMStyle({ display: 'flex', flex: 1 })).toEqual({ display: 'flex', - flexGrow: 1, - flexShrink: 1, - flexBasis: '0%' + flex: 1, + flexGrow: '1 !important', + flexShrink: '1 !important' }); }); test('flex: 10', () => { expect(createReactDOMStyle({ display: 'flex', flex: 10 })).toEqual({ display: 'flex', - flexGrow: 10, - flexShrink: 1, - flexBasis: '0%' + flex: 10, + flexGrow: '10 !important', + flexShrink: '1 !important' }); }); @@ -71,16 +71,17 @@ describe('apis/StyleSheet/createReactDOMStyle', () => { // is flex-basis applied? expect(createReactDOMStyle({ display: 'flex', flexBasis: '25%' })).toEqual({ display: 'flex', - flexShrink: 0, - flexBasis: '25%' + flexShrink: '0 !important', + flexBasis: '25% !important' }); // can flex-basis override the 'flex' expansion? expect(createReactDOMStyle({ display: 'flex', flex: 1, flexBasis: '25%' })).toEqual({ display: 'flex', - flexGrow: 1, - flexShrink: 1, - flexBasis: '25%' + flex: 1, + flexGrow: '1 !important', + flexShrink: '1 !important', + flexBasis: '25% !important' }); }); @@ -88,16 +89,16 @@ describe('apis/StyleSheet/createReactDOMStyle', () => { // is flex-shrink applied? expect(createReactDOMStyle({ display: 'flex', flexShrink: 1 })).toEqual({ display: 'flex', - flexShrink: 1, - flexBasis: 'auto' + flexShrink: '1 !important', + flexBasis: 'auto !important' }); // can flex-shrink override the 'flex' expansion? expect(createReactDOMStyle({ display: 'flex', flex: 1, flexShrink: 2 })).toEqual({ display: 'flex', - flexGrow: 1, - flexShrink: 2, - flexBasis: '0%' + flex: 1, + flexGrow: '1 !important', + flexShrink: '2 !important' }); }); }); diff --git a/src/apis/StyleSheet/createReactDOMStyle.js b/src/apis/StyleSheet/createReactDOMStyle.js index b0516164..1387eb67 100644 --- a/src/apis/StyleSheet/createReactDOMStyle.js +++ b/src/apis/StyleSheet/createReactDOMStyle.js @@ -158,29 +158,56 @@ const createReducer = (style, styleProps) => { case 'display': { resolvedStyle.display = value; - // defaults of 'flexBasis:auto' and 'flexShrink:0' have lowest precedence - if (style.display === 'flex') { + // A flex container in React Native has these defaults which should be + // set only if there is no otherwise supplied flex style. + if (style.display === 'flex' && style.flex == null) { if (style.flexShrink == null) { - resolvedStyle.flexShrink = 0; + resolvedStyle.flexShrink = '0 !important'; } if (style.flexBasis == null) { - resolvedStyle.flexBasis = 'auto'; + resolvedStyle.flexBasis = 'auto !important'; } } break; } + // The 'flex' property value in React Native must be a positive integer, + // 0, or -1. + // + // On the web, a positive integer value for 'flex' is complicated by + // browser differences. Although browsers render styles like 'flex:2' + // consistently, they don't all set the same value for the resulting + // 'flexBasis' (See #616). Expanding 'flex' in 'StyleSheet' would mean + // setting different values for different browsers. + // + // This fix instead relies on the browser expanding 'flex' itself. And + // because the 'flex' style is not being expanded the generated CSS is + // likely to contain source order "conflicts". To avoid the browser + // relying on source order to resolve the styles, all the longhand flex + // property values must use '!important'. case 'flex': { if (value > 0) { - resolvedStyle.flexGrow = value; - resolvedStyle.flexShrink = 1; - resolvedStyle.flexBasis = '0%'; + resolvedStyle.flex = value; + resolvedStyle.flexGrow = `${value} !important`; + resolvedStyle.flexShrink = '1 !important'; } else if (value === 0) { - resolvedStyle.flexGrow = 0; - resolvedStyle.flexShrink = 0; + resolvedStyle.flexGrow = '0 !important'; + resolvedStyle.flexShrink = '0 !important'; + resolvedStyle.flexBasis = 'auto !important'; } else if (value === -1) { - resolvedStyle.flexGrow = 0; - resolvedStyle.flexShrink = 1; + resolvedStyle.flexGrow = '0 !important'; + resolvedStyle.flexShrink = '1 !important'; + resolvedStyle.flexBasis = 'auto !important'; + } + break; + } + + case 'flexGrow': + case 'flexShrink': + case 'flexBasis': { + if (value != null) { + const hasImportant = `${value}`.indexOf('!important') > -1; + resolvedStyle[prop] = hasImportant ? value : `${value} !important`; } break; }