From cf3747957ab210e31504109bb6b3e34e773a5b9a Mon Sep 17 00:00:00 2001 From: Mayank Sunil Pagar Date: Tue, 8 Nov 2022 12:53:41 -0800 Subject: [PATCH] feat: mapped layout props for view component (#34590) Summary: This PR adds mapping for layout props, it maps marginInlineStart: 'marginStart', marginInlineEnd: 'marginEnd', marginBlockStart: 'marginTop', marginBlockEnd: 'marginBottom', marginBlock: 'marginVertical', marginInline: 'marginHorizontal', paddingInlineStart: 'paddingStart', paddingInlineEnd: 'paddingEnd', paddingBlockStart: 'paddingTop', paddingBlockEnd: 'paddingBottom', paddingBlock: 'paddingVertical', paddingInline: 'paddingHorizontal', as requested on https://github.com/facebook/react-native/issues/34425 ## Changelog [General][Added] - Added CSS logical properties by mapping layout props. Pull Request resolved: https://github.com/facebook/react-native/pull/34590 Test Plan: ```js Hello World! ``` Reviewed By: cipolleschi Differential Revision: D41108750 Pulled By: necolas fbshipit-source-id: 870b9b58a740aba12290a0604a9f6b52aa52de4c --- Libraries/Components/TextInput/TextInput.js | 12 ++- .../__snapshots__/TextInput-test.js.snap | 2 + Libraries/Components/View/View.js | 8 +- Libraries/Image/Image.android.js | 4 + Libraries/Image/Image.ios.js | 3 + Libraries/StyleSheet/StyleSheetTypes.d.ts | 12 +++ Libraries/StyleSheet/StyleSheetTypes.js | 68 +++++++++++++++ .../__tests__/processStyles-test.js | 83 +++++++++++++++++++ Libraries/StyleSheet/processStyles.js | 45 ++++++++++ Libraries/Text/Text.js | 42 +++++----- 10 files changed, 254 insertions(+), 25 deletions(-) create mode 100644 Libraries/StyleSheet/__tests__/processStyles-test.js create mode 100644 Libraries/StyleSheet/processStyles.js diff --git a/Libraries/Components/TextInput/TextInput.js b/Libraries/Components/TextInput/TextInput.js index ada8c7bcdc9af6..e23c62b2305fbd 100644 --- a/Libraries/Components/TextInput/TextInput.js +++ b/Libraries/Components/TextInput/TextInput.js @@ -19,6 +19,7 @@ import type {TextInputType} from './TextInput.flow'; import usePressability from '../../Pressability/usePressability'; import flattenStyle from '../../StyleSheet/flattenStyle'; +import processLayoutProps from '../../StyleSheet/processStyles'; import StyleSheet, { type ColorValue, type TextStyleProp, @@ -1406,11 +1407,14 @@ function InternalTextInput(props: Props): React.Node { ? RCTMultilineTextInputView : RCTSinglelineTextInputView; - const style = + let style = props.multiline === true - ? StyleSheet.flatten([styles.multilineInput, props.style]) + ? [styles.multilineInput, props.style] : props.style; + style = flattenStyle(style); + style = processLayoutProps(style); + const useOnChangeSync = (props.unstable_onChangeSync || props.unstable_onChangeTextSync) && !(props.onChange || props.onChangeText); @@ -1442,7 +1446,9 @@ function InternalTextInput(props: Props): React.Node { /> ); } else if (Platform.OS === 'android') { - const style = [props.style]; + let style = flattenStyle(props.style); + style = processLayoutProps(style); + const autoCapitalize = props.autoCapitalize || 'sentences'; const _accessibilityLabelledBy = props?.['aria-labelledby'] ?? props?.accessibilityLabelledBy; diff --git a/Libraries/Components/TextInput/__tests__/__snapshots__/TextInput-test.js.snap b/Libraries/Components/TextInput/__tests__/__snapshots__/TextInput-test.js.snap index ea3b4c1f062744..033943923caa09 100644 --- a/Libraries/Components/TextInput/__tests__/__snapshots__/TextInput-test.js.snap +++ b/Libraries/Components/TextInput/__tests__/__snapshots__/TextInput-test.js.snap @@ -32,6 +32,7 @@ exports[`TextInput tests should render as expected: should deep render when mock onStartShouldSetResponder={[Function]} rejectResponderTermination={true} selection={null} + style={Object {}} submitBehavior="blurAndSubmit" text="" underlineColorAndroid="transparent" @@ -70,6 +71,7 @@ exports[`TextInput tests should render as expected: should deep render when not onStartShouldSetResponder={[Function]} rejectResponderTermination={true} selection={null} + style={Object {}} submitBehavior="blurAndSubmit" text="" underlineColorAndroid="transparent" diff --git a/Libraries/Components/View/View.js b/Libraries/Components/View/View.js index 8ef1f814a312fb..8ea48f9673f849 100644 --- a/Libraries/Components/View/View.js +++ b/Libraries/Components/View/View.js @@ -11,6 +11,7 @@ import type {ViewProps} from './ViewPropTypes'; import flattenStyle from '../../StyleSheet/flattenStyle'; +import processLayoutProps from '../../StyleSheet/processStyles'; import TextAncestor from '../../Text/TextAncestor'; import {getAccessibilityRoleFromRole} from '../../Utilities/AcessibilityMapping'; import ViewNativeComponent from './ViewNativeComponent'; @@ -57,7 +58,6 @@ const View: React.AbstractComponent< nativeID, pointerEvents, role, - style, tabIndex, ...otherProps }: ViewProps, @@ -81,8 +81,10 @@ const View: React.AbstractComponent< text: ariaValueText ?? accessibilityValue?.text, }; - const flattenedStyle = flattenStyle(style); - const newPointerEvents = flattenedStyle?.pointerEvents || pointerEvents; + let style = flattenStyle(otherProps.style); + style = processLayoutProps(style); + + const newPointerEvents = style?.pointerEvents || pointerEvents; return ( diff --git a/Libraries/Image/Image.android.js b/Libraries/Image/Image.android.js index f874c6abc93f02..5e2c45caf835b0 100644 --- a/Libraries/Image/Image.android.js +++ b/Libraries/Image/Image.android.js @@ -13,6 +13,7 @@ import type {ImageAndroid} from './Image.flow'; import type {ImageProps as ImagePropsType} from './ImageProps'; import flattenStyle from '../StyleSheet/flattenStyle'; +import processLayoutProps from '../StyleSheet/processStyles'; import StyleSheet from '../StyleSheet/StyleSheet'; import TextAncestor from '../Text/TextAncestor'; import ImageAnalyticsTagContext from './ImageAnalyticsTagContext'; @@ -165,6 +166,9 @@ const BaseImage = (props: ImagePropsType, forwardedRef) => { } const {height, width, ...restProps} = props; + + style = processLayoutProps(style); + const {onLoadStart, onLoad, onLoadEnd, onError} = props; const nativeProps = { ...restProps, diff --git a/Libraries/Image/Image.ios.js b/Libraries/Image/Image.ios.js index 6f1d72fcb5a8c3..8b15f880bc7fe2 100644 --- a/Libraries/Image/Image.ios.js +++ b/Libraries/Image/Image.ios.js @@ -14,6 +14,7 @@ import type {ImageIOS} from './Image.flow'; import type {ImageProps as ImagePropsType} from './ImageProps'; import flattenStyle from '../StyleSheet/flattenStyle'; +import processLayoutProps from '../StyleSheet/processStyles'; import StyleSheet from '../StyleSheet/StyleSheet'; import ImageAnalyticsTagContext from './ImageAnalyticsTagContext'; import ImageInjection from './ImageInjection'; @@ -137,6 +138,8 @@ const BaseImage = (props: ImagePropsType, forwardedRef) => { // $FlowFixMe[prop-missing] const tintColor = props.tintColor || style.tintColor; + style = processLayoutProps(style); + if (props.children != null) { throw new Error( 'The component cannot contain children. If you want to render content on top of the image, consider using the component or absolute positioning.', diff --git a/Libraries/StyleSheet/StyleSheetTypes.d.ts b/Libraries/StyleSheet/StyleSheetTypes.d.ts index 5e0b8d8e1ab309..d6a8b4f31101c6 100644 --- a/Libraries/StyleSheet/StyleSheetTypes.d.ts +++ b/Libraries/StyleSheet/StyleSheetTypes.d.ts @@ -69,9 +69,15 @@ export interface FlexStyle { | undefined; left?: number | string | undefined; margin?: number | string | undefined; + marginBlock?: number | string | undefined; + marginBlockEnd?: number | string | undefined; + marginBlockStart?: number | string | undefined; marginBottom?: number | string | undefined; marginEnd?: number | string | undefined; marginHorizontal?: number | string | undefined; + marginInline?: number | string | undefined; + marginInlineEnd?: number | string | undefined; + marginInlineStart?: number | string | undefined; marginLeft?: number | string | undefined; marginRight?: number | string | undefined; marginStart?: number | string | undefined; @@ -84,8 +90,14 @@ export interface FlexStyle { overflow?: 'visible' | 'hidden' | 'scroll' | undefined; padding?: number | string | undefined; paddingBottom?: number | string | undefined; + paddingBlock?: number | string | undefined; + paddingBlockEnd?: number | string | undefined; + paddingBlockStart?: number | string | undefined; paddingEnd?: number | string | undefined; paddingHorizontal?: number | string | undefined; + paddingInline?: number | string | undefined; + paddingInlineEnd?: number | string | undefined; + paddingInlineStart?: number | string | undefined; paddingLeft?: number | string | undefined; paddingRight?: number | string | undefined; paddingStart?: number | string | undefined; diff --git a/Libraries/StyleSheet/StyleSheetTypes.js b/Libraries/StyleSheet/StyleSheetTypes.js index 14cf6d7dc9471a..69d9b48de32508 100644 --- a/Libraries/StyleSheet/StyleSheetTypes.js +++ b/Libraries/StyleSheet/StyleSheetTypes.js @@ -179,6 +179,23 @@ type ____LayoutStyle_Internal = $ReadOnly<{ */ margin?: DimensionValue, + /** Setting `marginBlock` has the same effect as setting both + * `marginTop` and `marginBottom`. + */ + marginBlock?: DimensionValue, + + /** `marginBlockEnd` works like `margin-bottom` in CSS. + * See https://developer.mozilla.org/en-US/docs/Web/CSS/margin-bottom + * for more details. + */ + marginBlockEnd?: DimensionValue, + + /** `marginBlockStart` works like `margin-top` in CSS. + * See https://developer.mozilla.org/en-US/docs/Web/CSS/margin-top + * for more details. + */ + marginBlockStart?: DimensionValue, + /** `marginBottom` works like `margin-bottom` in CSS. * See https://developer.mozilla.org/en-US/docs/Web/CSS/margin-bottom * for more details. @@ -196,6 +213,23 @@ type ____LayoutStyle_Internal = $ReadOnly<{ */ marginHorizontal?: DimensionValue, + /** Setting `marginInline` has the same effect as setting + * both `marginLeft` and `marginRight`. + */ + marginInline?: DimensionValue, + + /** + * When direction is `ltr`, `marginInlineEnd` is equivalent to `marginRight`. + * When direction is `rtl`, `marginInlineEnd` is equivalent to `marginLeft`. + */ + marginInlineEnd?: DimensionValue, + + /** + * When direction is `ltr`, `marginInlineStart` is equivalent to `marginLeft`. + * When direction is `rtl`, `marginInlineStart` is equivalent to `marginRight`. + */ + marginInlineStart?: DimensionValue, + /** `marginLeft` works like `margin-left` in CSS. * See https://developer.mozilla.org/en-US/docs/Web/CSS/margin-left * for more details. @@ -232,6 +266,23 @@ type ____LayoutStyle_Internal = $ReadOnly<{ */ padding?: DimensionValue, + /** Setting `paddingBlock` is like setting both of + * `paddingTop` and `paddingBottom`. + */ + paddingBlock?: DimensionValue, + + /** `paddingBlockEnd` works like `padding-bottom` in CSS. + * See https://developer.mozilla.org/en-US/docs/Web/CSS/padding-bottom + * for more details. + */ + paddingBlockEnd?: DimensionValue, + + /** `paddingBlockStart` works like `padding-top` in CSS. + * See https://developer.mozilla.org/en-US/docs/Web/CSS/padding-top + * for more details. + */ + paddingBlockStart?: DimensionValue, + /** `paddingBottom` works like `padding-bottom` in CSS. * See https://developer.mozilla.org/en-US/docs/Web/CSS/padding-bottom * for more details. @@ -249,6 +300,23 @@ type ____LayoutStyle_Internal = $ReadOnly<{ */ paddingHorizontal?: DimensionValue, + /** Setting `paddingInline` is like setting both of + * `paddingLeft` and `paddingRight`. + */ + paddingInline?: DimensionValue, + + /** + * When direction is `ltr`, `paddingInlineEnd` is equivalent to `paddingRight`. + * When direction is `rtl`, `paddingInlineEnd` is equivalent to `paddingLeft`. + */ + paddingInlineEnd?: DimensionValue, + + /** + * When direction is `ltr`, `paddingInlineStart` is equivalent to `paddingLeft`. + * When direction is `rtl`, `paddingInlineStart` is equivalent to `paddingRight`. + */ + paddingInlineStart?: DimensionValue, + /** `paddingLeft` works like `padding-left` in CSS. * See https://developer.mozilla.org/en-US/docs/Web/CSS/padding-left * for more details. diff --git a/Libraries/StyleSheet/__tests__/processStyles-test.js b/Libraries/StyleSheet/__tests__/processStyles-test.js new file mode 100644 index 00000000000000..0e4f45c2f7dfd1 --- /dev/null +++ b/Libraries/StyleSheet/__tests__/processStyles-test.js @@ -0,0 +1,83 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @format + * @emails oncall+react_native + */ + +'use strict'; + +const processLayoutProps = require('../processStyles'); + +describe('processLayoutProps', () => { + it('it should map layout style properties', () => { + const style = { + marginInlineStart: 10, + marginInlineEnd: 20, + marginBlockStart: 30, + marginBlockEnd: 40, + marginBlock: 50, + marginInline: 60, + paddingInlineStart: 70, + paddingInlineEnd: 80, + paddingBlockStart: 90, + paddingBlockEnd: 100, + paddingBlock: 110, + paddingInline: 120, + }; + const processedStyle = processLayoutProps(style); + expect(processedStyle.marginStart).toBe(10); + expect(processedStyle.marginEnd).toBe(20); + expect(processedStyle.marginTop).toBe(30); + expect(processedStyle.marginBottom).toBe(40); + expect(processedStyle.marginVertical).toBe(50); + expect(processedStyle.marginHorizontal).toBe(60); + expect(processedStyle.paddingStart).toBe(70); + expect(processedStyle.paddingEnd).toBe(80); + expect(processedStyle.paddingTop).toBe(90); + expect(processedStyle.paddingBottom).toBe(100); + expect(processedStyle.paddingVertical).toBe(110); + expect(processedStyle.paddingHorizontal).toBe(120); + + expect(processedStyle.marginInlineStart).toBe(undefined); + expect(processedStyle.marginInlineEnd).toBe(undefined); + expect(processedStyle.marginBlockStart).toBe(undefined); + expect(processedStyle.marginBlockEnd).toBe(undefined); + expect(processedStyle.marginBlock).toBe(undefined); + expect(processedStyle.marginInline).toBe(undefined); + expect(processedStyle.paddingInlineStart).toBe(undefined); + expect(processedStyle.paddingInlineEnd).toBe(undefined); + expect(processedStyle.paddingBlockStart).toBe(undefined); + expect(processedStyle.paddingBlockEnd).toBe(undefined); + expect(processedStyle.paddingBlock).toBe(undefined); + expect(processedStyle.paddingInline).toBe(undefined); + }); + + it('should override style properties', () => { + const style = {marginStart: 20, marginInlineStart: 40}; + const processedStyle = processLayoutProps(style); + expect(processedStyle.marginStart).toBe(40); + }); + + it('should overwrite properties with `undefined`', () => { + const style = {marginInlineStart: 40, marginStart: undefined}; + const processedStyle = processLayoutProps(style); + expect(processedStyle.marginStart).toBe(40); + }); + + it('should not fail on falsy values', () => { + expect(() => processLayoutProps({})).not.toThrow(); + expect(() => processLayoutProps(null)).not.toThrow(); + expect(() => processLayoutProps(false)).not.toThrow(); + expect(() => processLayoutProps(undefined)).not.toThrow(); + }); + + it('should not change style if there is no layout style property', () => { + const style = {backgroundColor: '#000', width: 10}; + const processedStyle = processLayoutProps(style); + expect(processedStyle).toStrictEqual(style); + }); +}); diff --git a/Libraries/StyleSheet/processStyles.js b/Libraries/StyleSheet/processStyles.js new file mode 100644 index 00000000000000..91c210a6e91da1 --- /dev/null +++ b/Libraries/StyleSheet/processStyles.js @@ -0,0 +1,45 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @format + * @flow strict-local + */ + +'use strict'; + +import type {____FlattenStyleProp_Internal} from './StyleSheetTypes'; + +function processLayoutProps( + flattenedStyle: ____FlattenStyleProp_Internal, +): ____FlattenStyleProp_Internal { + const _flattenedStyle = {...flattenedStyle}; + const layoutPropMap = { + marginInlineStart: 'marginStart', + marginInlineEnd: 'marginEnd', + marginBlockStart: 'marginTop', + marginBlockEnd: 'marginBottom', + marginBlock: 'marginVertical', + marginInline: 'marginHorizontal', + paddingInlineStart: 'paddingStart', + paddingInlineEnd: 'paddingEnd', + paddingBlockStart: 'paddingTop', + paddingBlockEnd: 'paddingBottom', + paddingBlock: 'paddingVertical', + paddingInline: 'paddingHorizontal', + }; + if (_flattenedStyle) { + Object.keys(layoutPropMap).forEach(key => { + if (_flattenedStyle && _flattenedStyle[key] !== undefined) { + _flattenedStyle[layoutPropMap[key]] = _flattenedStyle[key]; + delete _flattenedStyle[key]; + } + }); + } + + return _flattenedStyle; +} + +module.exports = processLayoutProps; diff --git a/Libraries/Text/Text.js b/Libraries/Text/Text.js index 1f6524721cc820..1fca19fdada83f 100644 --- a/Libraries/Text/Text.js +++ b/Libraries/Text/Text.js @@ -9,17 +9,18 @@ */ import type {PressEvent} from '../Types/CoreEventTypes'; +import type {TextProps} from './TextProps'; import * as PressabilityDebug from '../Pressability/PressabilityDebug'; import usePressability from '../Pressability/usePressability'; import flattenStyle from '../StyleSheet/flattenStyle'; import processColor from '../StyleSheet/processColor'; +import processLayoutProps from '../StyleSheet/processStyles'; import StyleSheet from '../StyleSheet/StyleSheet'; import {getAccessibilityRoleFromRole} from '../Utilities/AcessibilityMapping'; import Platform from '../Utilities/Platform'; import TextAncestor from './TextAncestor'; import {NativeText, NativeVirtualText} from './TextNativeComponent'; -import {type TextProps} from './TextProps'; import * as React from 'react'; import {useContext, useMemo, useState} from 'react'; @@ -174,19 +175,7 @@ const Text: React.AbstractComponent< ? null : processColor(restProps.selectionColor); - let style = flattenStyle(restProps.style); - - let _selectable = restProps.selectable; - if (style?.userSelect != null) { - _selectable = userSelectToSelectableMap[style.userSelect]; - } - - if (style?.verticalAlign != null) { - style = StyleSheet.compose(style, { - textAlignVertical: - verticalAlignToTextAlignVerticalMap[style.verticalAlign], - }); - } + let style; if (__DEV__) { if (PressabilityDebug.isEnabled() && onPress != null) { @@ -194,6 +183,8 @@ const Text: React.AbstractComponent< color: 'magenta', }); } + } else { + style = restProps.style; } let numberOfLines = restProps.numberOfLines; @@ -211,10 +202,23 @@ const Text: React.AbstractComponent< default: accessible, }); - let flattenedStyle = flattenStyle(style); + style = flattenStyle(style); + style = processLayoutProps(style); - if (typeof flattenedStyle?.fontWeight === 'number') { - flattenedStyle.fontWeight = flattenedStyle?.fontWeight.toString(); + if (typeof style?.fontWeight === 'number') { + style.fontWeight = style?.fontWeight.toString(); + } + + let _selectable = restProps.selectable; + if (style?.userSelect != null) { + _selectable = userSelectToSelectableMap[style.userSelect]; + } + + if (style?.verticalAlign != null) { + style = StyleSheet.compose(style, { + textAlignVertical: + verticalAlignToTextAlignVerticalMap[style.verticalAlign], + }); } const _hasOnPressOrOnLongPress = @@ -235,7 +239,7 @@ const Text: React.AbstractComponent< nativeID={id ?? nativeID} numberOfLines={numberOfLines} selectionColor={selectionColor} - style={flattenedStyle} + style={style} ref={forwardedRef} /> ) : ( @@ -261,7 +265,7 @@ const Text: React.AbstractComponent< nativeID={id ?? nativeID} numberOfLines={numberOfLines} selectionColor={selectionColor} - style={flattenedStyle} + style={style} ref={forwardedRef} />