From 1cae5d55a120b62aa15641cc152e43e2df4462ba Mon Sep 17 00:00:00 2001 From: Nicolas Gallagher Date: Sat, 29 Apr 2017 11:09:43 -0700 Subject: [PATCH] [fix] setNativeProps DOM style copying The 'style' object of an HTML node is a 'CSSStyleDeclaration'. Use the 'CSSStyleDeclaration' API to copy the inline styles, rather than treating it like a plain object. This avoids errors that were resulting from indices and property names being used a key-value pairs in the resulting style copy. Fix #460 Ref #454 --- src/apis/StyleSheet/StyleRegistry.js | 22 +++++-------------- .../__tests__/StyleRegistry-test.js | 2 +- src/modules/NativeMethodsMixin/index.js | 17 ++++++++++---- 3 files changed, 19 insertions(+), 22 deletions(-) diff --git a/src/apis/StyleSheet/StyleRegistry.js b/src/apis/StyleSheet/StyleRegistry.js index 1ce52731..c994ef84 100644 --- a/src/apis/StyleSheet/StyleRegistry.js +++ b/src/apis/StyleSheet/StyleRegistry.js @@ -11,8 +11,6 @@ import { prefixInlineStyles } from '../../modules/prefixStyles'; import ReactNativePropRegistry from '../../modules/ReactNativePropRegistry'; import StyleManager from './StyleManager'; -const vendorPrefixPattern = /^(webkit|moz|ms)/; - const createCacheKey = id => { const prefix = I18nManager.isRTL ? 'rtl' : 'ltr'; return `${prefix}-${id}`; @@ -88,10 +86,10 @@ class StyleRegistry { * To determine the next style, some of the existing DOM state must be * converted back into React Native styles. */ - resolveStateful(rnStyleNext, { classList: domClassList, style: domStyle }) { + resolveStateful(rnStyleNext, { classList: rdomClassList, style: rdomStyle }) { // Convert the DOM classList back into a React Native form // Preserves unrecognized class names. - const { classList: rnClassList, style: rnStyle } = domClassList.reduce( + const { classList: rnClassList, style: rnStyle } = rdomClassList.reduce( (styleProps, className) => { const { prop, value } = this.styleManager.getDeclaration(className); if (prop) { @@ -104,24 +102,12 @@ class StyleRegistry { { classList: [], style: {} } ); - // DOM style may include vendor prefixes and properties set by other libraries. - // Preserve it but transform back into React DOM style. - const rdomStyle = Object.keys(domStyle).reduce((acc, styleName) => { - const value = domStyle[styleName]; - if (value !== '') { - const reactStyleName = vendorPrefixPattern.test(styleName) - ? styleName.charAt(0).toUpperCase() + styleName.slice(1) - : styleName; - acc[reactStyleName] = value; - } - return acc; - }, {}); - // Create next DOM style props from current and next RN styles const { classList: rdomClassListNext, style: rdomStyleNext } = this.resolve([ rnStyle, rnStyleNext ]); + // Next class names take priority over current inline styles const style = { ...rdomStyle }; rdomClassListNext.forEach(className => { @@ -130,8 +116,10 @@ class StyleRegistry { style[prop] = ''; } }); + // Next inline styles take priority over current inline styles Object.assign(style, rdomStyleNext); + // Add the current class names not managed by React Native const className = classListToString(rdomClassListNext.concat(rnClassList)); diff --git a/src/apis/StyleSheet/__tests__/StyleRegistry-test.js b/src/apis/StyleSheet/__tests__/StyleRegistry-test.js index 4dc5d088..148aa04b 100644 --- a/src/apis/StyleSheet/__tests__/StyleRegistry-test.js +++ b/src/apis/StyleSheet/__tests__/StyleRegistry-test.js @@ -82,7 +82,7 @@ describe('apis/StyleSheet/StyleRegistry', () => { // note: this also checks for correctly uppercasing the first letter of DOM vendor prefixes const domStyleProps = { classList: [], - style: { opacity: 0.5, webkitTransform: 'scale(1)', transform: 'scale(1)' } + style: { opacity: 0.5, WebkitTransform: 'scale(1)', transform: 'scale(1)' } }; const domStyleNextProps = styleRegistry.resolveStateful( { opacity: 1, transform: [{ scale: 2 }] }, diff --git a/src/modules/NativeMethodsMixin/index.js b/src/modules/NativeMethodsMixin/index.js index 37fcdb69..d0cedd86 100644 --- a/src/modules/NativeMethodsMixin/index.js +++ b/src/modules/NativeMethodsMixin/index.js @@ -11,6 +11,9 @@ import findNodeHandle from '../findNodeHandle'; import StyleRegistry from '../../apis/StyleSheet/registry'; import UIManager from '../../apis/UIManager'; +const hyphenPattern = /-([a-z])/g; +const toCamelCase = str => str.replace(hyphenPattern, m => m[1].toUpperCase()); + const NativeMethodsMixin = { /** * Removes focus from an input or view. This is the opposite of `focus()`. @@ -69,11 +72,17 @@ const NativeMethodsMixin = { setNativeProps(nativeProps: Object) { // Copy of existing DOM state const node = findNodeHandle(this); + const nodeStyle = node.style; const classList = Array.prototype.slice.call(node.classList); - const style = { ...node.style }; - // See #454 - delete style.length; - + const style = {}; + // DOM style is a CSSStyleDeclaration + // https://developer.mozilla.org/en-US/docs/Web/API/CSSStyleDeclaration + for (let i = node.style.length; i > 0; i -= 1) { + const property = nodeStyle.item(i); + // DOM style uses hyphenated prop names and may include vendor prefixes + // Transform back into React DOM style. + style[toCamelCase(property)] = nodeStyle.getPropertyValue(property); + } const domStyleProps = { classList, style }; // Next DOM state