From a27671d7cf85b98ebfe08aa36cb75d4c29fca3c6 Mon Sep 17 00:00:00 2001 From: Nicolas Gallagher Date: Wed, 4 Jan 2017 10:43:16 -0800 Subject: [PATCH] [fix] passing on RN style props in createDOMElement The 'createDOMElement' function wasn't pulling 'style' out of the props. A change to the logic that sets DOM props meant that if 'StyleRegistry.resolve' didn't return a 'style' object, the React Native styles would be passed through to the underlying DOM node. Fix #315 --- src/apis/StyleSheet/registry.js | 21 ++++--- .../__snapshots__/index-test.js.snap | 58 ++++++++++++------- src/modules/createDOMElement/index.js | 7 ++- 3 files changed, 55 insertions(+), 31 deletions(-) diff --git a/src/apis/StyleSheet/registry.js b/src/apis/StyleSheet/registry.js index 98ceeae6..a35f5a04 100644 --- a/src/apis/StyleSheet/registry.js +++ b/src/apis/StyleSheet/registry.js @@ -24,6 +24,14 @@ const createClassName = (prop, value) => { return `rn-${prop}:${val}`; }; +/** + * Formatting improves debugging in devtools and snapshot + */ +const mapDeclarationsToClassName = (style, fn) => { + const result = mapKeyValue(style, fn).join('\n').trim(); + return `\n${result}`; +}; + /** * Inject a CSS rule for a given declaration and record the availability of the * resulting class name. @@ -50,11 +58,11 @@ const injectClassNameIfNeeded = (prop, value) => { let resolvedPropsCache = {}; const registerStyle = (id, flatStyle) => { const style = createReactDOMStyle(flatStyle); - const className = mapKeyValue(style, (prop, value) => { + const className = mapDeclarationsToClassName(style, (prop, value) => { if (value != null) { return injectClassNameIfNeeded(prop, value); } - }).join(' ').trim(); + }); const key = `${prefix}-${id}`; resolvedPropsCache[key] = { className }; @@ -70,7 +78,7 @@ const resolveProps = (reactNativeStyle) => { const domStyle = createReactDOMStyle(flatStyle); const style = {}; - const _className = mapKeyValue(domStyle, (prop, value) => { + const className = mapDeclarationsToClassName(domStyle, (prop, value) => { if (value != null) { const singleClassName = createClassName(prop, value); if (injectedClassNames[singleClassName]) { @@ -80,12 +88,7 @@ const resolveProps = (reactNativeStyle) => { style[prop] = value; } } - }) - // improves debugging in devtools and snapshots - .join('\n') - .trim(); - - const className = `\n${_className}`; + }); const props = { className, diff --git a/src/components/Text/__tests__/__snapshots__/index-test.js.snap b/src/components/Text/__tests__/__snapshots__/index-test.js.snap index 57d58899..a229e53d 100644 --- a/src/components/Text/__tests__/__snapshots__/index-test.js.snap +++ b/src/components/Text/__tests__/__snapshots__/index-test.js.snap @@ -1,15 +1,24 @@ exports[`components/Text prop "children" 1`] = ` + className=" +rn-borderTopWidth:0px +rn-borderRightWidth:0px +rn-borderBottomWidth:0px +rn-borderLeftWidth:0px +rn-color:inherit +rn-display:inline +rn-font:inherit +rn-marginTop:0px +rn-marginRight:0px +rn-marginBottom:0px +rn-marginLeft:0px +rn-paddingTop:0px +rn-paddingRight:0px +rn-paddingBottom:0px +rn-paddingLeft:0px +rn-textDecoration:none +rn-whiteSpace:pre-wrap +rn-wordWrap:break-word"> children `; @@ -44,16 +53,25 @@ rn-wordWrap:break-word" exports[`components/Text prop "selectable" 1`] = ` + className=" +rn-borderTopWidth:0px +rn-borderRightWidth:0px +rn-borderBottomWidth:0px +rn-borderLeftWidth:0px +rn-color:inherit +rn-display:inline +rn-font:inherit +rn-marginTop:0px +rn-marginRight:0px +rn-marginBottom:0px +rn-marginLeft:0px +rn-paddingTop:0px +rn-paddingRight:0px +rn-paddingBottom:0px +rn-paddingLeft:0px +rn-textDecoration:none +rn-whiteSpace:pre-wrap +rn-wordWrap:break-word" /> `; exports[`components/Text prop "selectable" 2`] = ` diff --git a/src/modules/createDOMElement/index.js b/src/modules/createDOMElement/index.js index f1fb35c7..a57c228b 100644 --- a/src/modules/createDOMElement/index.js +++ b/src/modules/createDOMElement/index.js @@ -25,6 +25,7 @@ const createDOMElement = (component, rnProps = emptyObject) => { accessibilityLiveRegion, accessibilityRole, accessible = true, + style: rnStyle, // we need to remove the RN styles from 'domProps' testID, type, ...domProps @@ -33,7 +34,7 @@ const createDOMElement = (component, rnProps = emptyObject) => { const accessibilityComponent = accessibilityRole && roleComponents[accessibilityRole]; const Component = accessibilityComponent || component; - const { className, style } = StyleRegistry.resolve(domProps.style) || emptyObject; + const { className, style } = StyleRegistry.resolve(rnStyle) || emptyObject; if (!accessible) { domProps['aria-hidden'] = true; } if (accessibilityLabel) { domProps['aria-label'] = accessibilityLabel; } @@ -53,7 +54,9 @@ const createDOMElement = (component, rnProps = emptyObject) => { if (style) { domProps.style = style; } - if (type) { domProps.type = type; } + if (type) { + domProps.type = type; + } return (