From eb43a8f3e7817d1fc62425216848353a3aeeb786 Mon Sep 17 00:00:00 2001 From: Nicolas Gallagher Date: Sat, 29 Apr 2017 19:49:59 -0700 Subject: [PATCH] [fix] setNativeProps with RTL layout Ensure that 'setNativeProps' doesn't try to i18n flip styles that have already been flipped. This is hacked into the current design. Registering both RTL and LTR styles is not implemented yet either. --- src/apis/StyleSheet/StyleRegistry.js | 31 ++++++++++--------- .../__tests__/StyleRegistry-test.js | 11 +++++++ .../__snapshots__/StyleRegistry-test.js.snap | 12 +++++++ src/modules/NativeMethodsMixin/index.js | 5 +-- 4 files changed, 43 insertions(+), 16 deletions(-) diff --git a/src/apis/StyleSheet/StyleRegistry.js b/src/apis/StyleSheet/StyleRegistry.js index 9b2a2714..d8b780ef 100644 --- a/src/apis/StyleSheet/StyleRegistry.js +++ b/src/apis/StyleSheet/StyleRegistry.js @@ -49,7 +49,7 @@ class StyleRegistry { /** * Resolves a React Native style object to DOM attributes */ - resolve(reactNativeStyle) { + resolve(reactNativeStyle, options) { if (!reactNativeStyle) { return undefined; } @@ -57,12 +57,12 @@ class StyleRegistry { // fast and cachable if (typeof reactNativeStyle === 'number') { const key = createCacheKey(reactNativeStyle); - return this._resolveStyleIfNeeded(key, reactNativeStyle); + return this._resolveStyleIfNeeded(reactNativeStyle, { key, ...options }); } // resolve a plain RN style object if (!Array.isArray(reactNativeStyle)) { - return this._resolveStyle(reactNativeStyle); + return this._resolveStyle(reactNativeStyle, options); } // flatten the style array @@ -77,7 +77,7 @@ class StyleRegistry { } } const key = isArrayOfNumbers ? createCacheKey(flatArray.join('-')) : null; - return this._resolveStyleIfNeeded(key, flatArray); + return this._resolveStyleIfNeeded(flatArray, { key, ...options }); } /** @@ -87,7 +87,9 @@ class StyleRegistry { * To determine the next style, some of the existing DOM state must be * converted back into React Native styles. */ - resolveStateful(rnStyleNext, { classList: rdomClassList, style: rdomStyle }) { + resolveStateful(rnStyleNext, domStyleProps, options) { + const { classList: rdomClassList, style: rdomStyle } = domStyleProps; + // Convert the DOM classList back into a React Native form // Preserves unrecognized class names. const { classList: rnClassList, style: rnStyle } = rdomClassList.reduce( @@ -104,10 +106,10 @@ class StyleRegistry { ); // Create next DOM style props from current and next RN styles - const { classList: rdomClassListNext, style: rdomStyleNext } = this.resolve([ - rnStyle, - rnStyleNext - ]); + const { classList: rdomClassListNext, style: rdomStyleNext } = this.resolve( + [rnStyle, rnStyleNext], + options + ); // Next class names take priority over current inline styles const style = { ...rdomStyle }; @@ -130,8 +132,9 @@ class StyleRegistry { /** * Resolves a React Native style object */ - _resolveStyle(reactNativeStyle) { - const domStyle = createReactDOMStyle(i18nStyle(flattenStyle(reactNativeStyle))); + _resolveStyle(reactNativeStyle, options) { + const flatStyle = flattenStyle(reactNativeStyle); + const domStyle = createReactDOMStyle(options.i18n === false ? flatStyle : i18nStyle(flatStyle)); const props = Object.keys(domStyle).reduce( (props, styleProp) => { @@ -163,15 +166,15 @@ class StyleRegistry { /** * Caching layer over 'resolveStyle' */ - _resolveStyleIfNeeded(key, style) { + _resolveStyleIfNeeded(style, { key, ...rest }) { if (key) { if (!this.cache[key]) { // slow: convert style object to props and cache - this.cache[key] = this._resolveStyle(style); + this.cache[key] = this._resolveStyle(style, rest); } return this.cache[key]; } - return this._resolveStyle(style); + return this._resolveStyle(style, rest); } } diff --git a/src/apis/StyleSheet/__tests__/StyleRegistry-test.js b/src/apis/StyleSheet/__tests__/StyleRegistry-test.js index 148aa04b..c9ec76d1 100644 --- a/src/apis/StyleSheet/__tests__/StyleRegistry-test.js +++ b/src/apis/StyleSheet/__tests__/StyleRegistry-test.js @@ -1,5 +1,6 @@ /* eslint-env jasmine, jest */ +import I18nManager from '../../I18nManager'; import StyleRegistry from '../StyleRegistry'; let styleRegistry; @@ -46,6 +47,16 @@ describe('apis/StyleSheet/StyleRegistry', () => { testResolve(a, b, c); }); + test('with register before RTL, resolves to className', () => { + const a = styleRegistry.register({ left: '12.34%' }); + const b = styleRegistry.register({ textAlign: 'left' }); + const c = styleRegistry.register({ marginLeft: 10 }); + I18nManager.forceRTL(true); + const resolved = styleRegistry.resolve([a, b, c]); + I18nManager.forceRTL(false); + expect(resolved).toMatchSnapshot(); + }); + test('with register, resolves to mixed', () => { const a = styleA; const b = styleRegistry.register(styleB); diff --git a/src/apis/StyleSheet/__tests__/__snapshots__/StyleRegistry-test.js.snap b/src/apis/StyleSheet/__tests__/__snapshots__/StyleRegistry-test.js.snap index 90e75dcf..62fb7cf8 100644 --- a/src/apis/StyleSheet/__tests__/__snapshots__/StyleRegistry-test.js.snap +++ b/src/apis/StyleSheet/__tests__/__snapshots__/StyleRegistry-test.js.snap @@ -1,5 +1,17 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`apis/StyleSheet/StyleRegistry resolve with register before RTL, resolves to className 1`] = ` +Object { + "classList": Array [], + "className": "", + "style": Object { + "marginRight": "10px", + "right": "12.34%", + "textAlign": "right", + }, +} +`; + exports[`apis/StyleSheet/StyleRegistry resolve with register, resolves to className 1`] = ` Object { "classList": Array [ diff --git a/src/modules/NativeMethodsMixin/index.js b/src/modules/NativeMethodsMixin/index.js index fe04586a..8f814ad6 100644 --- a/src/modules/NativeMethodsMixin/index.js +++ b/src/modules/NativeMethodsMixin/index.js @@ -8,6 +8,7 @@ import createDOMProps from '../createDOMProps'; import findNodeHandle from '../findNodeHandle'; +import i18nStyle from '../../apis/StyleSheet/i18nStyle'; import StyleRegistry from '../../apis/StyleSheet/registry'; import UIManager from '../../apis/UIManager'; @@ -88,8 +89,8 @@ const NativeMethodsMixin = { const domStyleProps = { classList, style }; // Next DOM state - const domProps = createDOMProps(nativeProps, style => - StyleRegistry.resolveStateful(style, domStyleProps) + const domProps = createDOMProps(i18nStyle(nativeProps), style => + StyleRegistry.resolveStateful(style, domStyleProps, { i18n: false }) ); UIManager.updateView(node, domProps, this); }