From 14c91cdf59949959dd2e39af4fed5bee01c3cba1 Mon Sep 17 00:00:00 2001 From: Gabriel Donadel Dall'Agnol Date: Tue, 27 Sep 2022 04:05:52 -0700 Subject: [PATCH] feat: Add string support for aspectRatio (#34629) Summary: This updates `aspectRatio` to support string values and ratio formats, i.e., `'16 / 9'`, thus aligning it with the [CSS Box Sizing Module Level 4](https://drafts.csswg.org/css-sizing-4/#aspect-ratio) specification as requested on https://github.com/facebook/react-native/issues/34425. This also adds unit tests to the `processAspectRatio` function ensuring the style processing works as expected. ## Changelog [General] [Added] - Add string support for aspectRatio Pull Request resolved: https://github.com/facebook/react-native/pull/34629 Test Plan: This can be tested either through `processAspectRatio-tests` or by using the following code: ```js ``` https://user-images.githubusercontent.com/11707729/189029904-da1dc0a6-85de-46aa-8ec2-3567802c8719.mov Reviewed By: jacdebug Differential Revision: D39423304 Pulled By: cipolleschi fbshipit-source-id: d323de93d6524e411e7ab9943335a8ca323b6e61 --- .../View/ReactNativeStyleAttributes.js | 3 +- Libraries/StyleSheet/StyleSheetTypes.d.ts | 2 +- Libraries/StyleSheet/StyleSheetTypes.js | 10 ++-- .../processAspectRatio-test.js.snap | 7 +++ .../__tests__/processAspectRatio-test.js | 50 +++++++++++++++++ Libraries/StyleSheet/processAspectRatio.js | 53 +++++++++++++++++++ 6 files changed, 120 insertions(+), 5 deletions(-) create mode 100644 Libraries/StyleSheet/__tests__/__snapshots__/processAspectRatio-test.js.snap create mode 100644 Libraries/StyleSheet/__tests__/processAspectRatio-test.js create mode 100644 Libraries/StyleSheet/processAspectRatio.js diff --git a/Libraries/Components/View/ReactNativeStyleAttributes.js b/Libraries/Components/View/ReactNativeStyleAttributes.js index ab2b30344237fa..e83109fc925020 100644 --- a/Libraries/Components/View/ReactNativeStyleAttributes.js +++ b/Libraries/Components/View/ReactNativeStyleAttributes.js @@ -9,6 +9,7 @@ */ import type {AnyAttributeType} from '../../Renderer/shims/ReactNativeTypes'; +import processAspectRatio from '../../StyleSheet/processAspectRatio'; import processColor from '../../StyleSheet/processColor'; import processFontVariant from '../../StyleSheet/processFontVariant'; import processTransform from '../../StyleSheet/processTransform'; @@ -23,7 +24,7 @@ const ReactNativeStyleAttributes: {[string]: AnyAttributeType, ...} = { alignContent: true, alignItems: true, alignSelf: true, - aspectRatio: true, + aspectRatio: {process: processAspectRatio}, borderBottomWidth: true, borderEndWidth: true, borderLeftWidth: true, diff --git a/Libraries/StyleSheet/StyleSheetTypes.d.ts b/Libraries/StyleSheet/StyleSheetTypes.d.ts index 419712fad42b73..1346e434f61b33 100644 --- a/Libraries/StyleSheet/StyleSheetTypes.d.ts +++ b/Libraries/StyleSheet/StyleSheetTypes.d.ts @@ -33,7 +33,7 @@ export interface FlexStyle { | undefined; alignItems?: FlexAlignType | undefined; alignSelf?: 'auto' | FlexAlignType | undefined; - aspectRatio?: number | undefined; + aspectRatio?: number | string | undefined; borderBottomWidth?: number | undefined; borderEndWidth?: number | string | undefined; borderLeftWidth?: number | undefined; diff --git a/Libraries/StyleSheet/StyleSheetTypes.js b/Libraries/StyleSheet/StyleSheetTypes.js index b50b47b4be64de..2e8690de7f68ee 100644 --- a/Libraries/StyleSheet/StyleSheetTypes.js +++ b/Libraries/StyleSheet/StyleSheetTypes.js @@ -446,8 +446,7 @@ type ____LayoutStyle_Internal = $ReadOnly<{ flexBasis?: number | string, /** - * Aspect ratio control the size of the undefined dimension of a node. Aspect ratio is a - * non-standard property only available in react native and not CSS. + * Aspect ratio control the size of the undefined dimension of a node. * * - On a node with a set width/height aspect ratio control the size of the unset dimension * - On a node with a set flex basis aspect ratio controls the size of the node in the cross axis @@ -457,8 +456,13 @@ type ____LayoutStyle_Internal = $ReadOnly<{ * - On a node with flex grow/shrink aspect ratio controls the size of the node in the cross axis * if unset * - Aspect ratio takes min/max dimensions into account + * + * Supports a number or a ratio, e.g.: + * - aspectRatio: '1 / 1' + * - aspectRatio: '1' + * - aspectRatio: '1' */ - aspectRatio?: number, + aspectRatio?: number | string, /** `zIndex` controls which components display on top of others. * Normally, you don't use `zIndex`. Components render according to diff --git a/Libraries/StyleSheet/__tests__/__snapshots__/processAspectRatio-test.js.snap b/Libraries/StyleSheet/__tests__/__snapshots__/processAspectRatio-test.js.snap new file mode 100644 index 00000000000000..bf6d088a94157e --- /dev/null +++ b/Libraries/StyleSheet/__tests__/__snapshots__/processAspectRatio-test.js.snap @@ -0,0 +1,7 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`processAspectRatio should not accept invalid formats 1`] = `"aspectRatio must either be a number, a ratio or \`auto\`. You passed: 0a"`; + +exports[`processAspectRatio should not accept invalid formats 2`] = `"aspectRatio must either be a number, a ratio or \`auto\`. You passed: 1 / 1 1"`; + +exports[`processAspectRatio should not accept invalid formats 3`] = `"aspectRatio must either be a number, a ratio or \`auto\`. You passed: auto 1/1"`; diff --git a/Libraries/StyleSheet/__tests__/processAspectRatio-test.js b/Libraries/StyleSheet/__tests__/processAspectRatio-test.js new file mode 100644 index 00000000000000..53629a580ceef4 --- /dev/null +++ b/Libraries/StyleSheet/__tests__/processAspectRatio-test.js @@ -0,0 +1,50 @@ +/** + * 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 processAspectRatio = require('../processAspectRatio'); + +describe('processAspectRatio', () => { + it('should accept numbers', () => { + expect(processAspectRatio(1)).toBe(1); + expect(processAspectRatio(0)).toBe(0); + expect(processAspectRatio(1.5)).toBe(1.5); + }); + + it('should accept string numbers', () => { + expect(processAspectRatio('1')).toBe(1); + expect(processAspectRatio('0')).toBe(0); + expect(processAspectRatio('1.5')).toBe(1.5); + expect(processAspectRatio('+1.5')).toBe(1.5); + expect(processAspectRatio(' 1')).toBe(1); + expect(processAspectRatio(' 0 ')).toBe(0); + }); + + it('should accept `auto`', () => { + expect(processAspectRatio('auto')).toBe(undefined); + expect(processAspectRatio(' auto')).toBe(undefined); + expect(processAspectRatio(' auto ')).toBe(undefined); + }); + + it('should accept ratios', () => { + expect(processAspectRatio('+1/1')).toBe(1); + expect(processAspectRatio('0 / 10')).toBe(0); + expect(processAspectRatio('117/ 13')).toBe(9); + expect(processAspectRatio('1.5 /1.2')).toBe(1.25); + expect(processAspectRatio('1/0')).toBe(Infinity); + }); + + it('should not accept invalid formats', () => { + expect(() => processAspectRatio('0a')).toThrowErrorMatchingSnapshot(); + expect(() => processAspectRatio('1 / 1 1')).toThrowErrorMatchingSnapshot(); + expect(() => processAspectRatio('auto 1/1')).toThrowErrorMatchingSnapshot(); + }); +}); diff --git a/Libraries/StyleSheet/processAspectRatio.js b/Libraries/StyleSheet/processAspectRatio.js new file mode 100644 index 00000000000000..bde4a4a645c0e5 --- /dev/null +++ b/Libraries/StyleSheet/processAspectRatio.js @@ -0,0 +1,53 @@ +/** + * 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 + */ + +'use strict'; + +const invariant = require('invariant'); + +function processAspectRatio(aspectRatio: number | string): ?number { + if (typeof aspectRatio === 'number') { + return aspectRatio; + } + + const matches = aspectRatio.split('/').map(s => s.trim()); + + if (matches.includes('auto')) { + if (__DEV__) { + invariant( + matches.length, + 'aspectRatio does not support `auto `. You passed: %s', + aspectRatio, + ); + } + return; + } + + const hasNonNumericValues = matches.some(n => Number.isNaN(Number(n))); + if (__DEV__) { + invariant( + !hasNonNumericValues && (matches.length === 1 || matches.length === 2), + 'aspectRatio must either be a number, a ratio or `auto`. You passed: %s', + aspectRatio, + ); + } + + if (hasNonNumericValues) { + return; + } + + if (matches.length === 2) { + return Number(matches[0]) / Number(matches[1]); + } + + return Number(matches[0]); +} + +module.exports = processAspectRatio;