Skip to content

Commit bd9348a

Browse files
camchismeta-codesync[bot]
authored andcommitted
Keep percentages in translateX/translateY string transforms (#59366)
Summary: `processTransform` drops the `%` unit when it parses `translateX(...)` or `translateY(...)` from a CSS transform string: ```js processTransform('translateX(10%)'); // [{translateX: 10}], expected [{translateX: '10%'}] ``` Native reads a bare number as points, so a view styled with `transform: 'translateX(10%)'` moves 10 points instead of 10% of its width. `translate(10%, 20px)` and the array form `[{translateX: '10%'}]` already keep the percentage, and so does the native CSS parser (`enableNativeCSSParsing`). Only the `translateX`/`translateY` branch of the JS string parser discards it. The same parser also required a digit before the decimal point, so `translateX(.5px)` and `matrix(.5, …)` parsed `.5` as `5`. The array form and the native CSS parser both accept these. #57487 fixed the same problem in `processTransformOrigin`. ## Changelog: [GENERAL] [FIXED] - Keep percentages in `translateX`/`translateY` string transforms and parse numbers without a leading zero Pull Request resolved: #59366 Test Plan: New tests fail without the fix and pass with it: - `yarn fantom packages/react-native/Libraries/StyleSheet/__tests__/processTransform-itest.js`: 19 passed. Without the fix, 2 fail: `translateX(10%)` gives `{translateX: 10}`, and `translateX(.5px)` gives `{translateX: 5}`. - `yarn fantom packages/react-native/Libraries/Components/View/__tests__/View-itest.js`: 228 passed. The new test renders a 200×50 `<View>` with `transform: 'translateX(10%) translateY(20%)'` and checks `getBoundingClientRect()`. Without the fix, the 2 `enableNativeCSSParsing` 🛑 variants fail with `Expected 20 but received 10`. The 2 ✅ variants already passed because the native parser handles it. - `CSSTransformTest.cpp`: added `matrix_leading_dot`, `translate_x_leading_dot` and `translate_y_leading_dot` to show the native parser already handles these inputs. I couldn't find an OSS runner for these tests, so I compiled the file against googletest 1.15.2 and the in-repo headers: 95 tests passed. Changing an expected value to `5.0f` makes `translate_x_leading_dot` fail with `Which is: 0.5`, so the assertion is checking the parsed value. - `yarn flow-check`: 0 errors. ESLint `--max-warnings 0` and Prettier on the changed JS files are clean, and `node scripts/clang-format.js --check` on the changed C++ file is clean. Not run: RNTester on a device. The change is JS-only and covered by the Fantom layout test above. Reviewed By: rubennorte Differential Revision: D127122803 Pulled By: javache fbshipit-source-id: a7f8873c12bc845c074405e6d79da00b723d223c
1 parent 4fac9c8 commit bd9348a

4 files changed

Lines changed: 104 additions & 5 deletions

File tree

‎packages/react-native/Libraries/Components/View/__tests__/View-itest.js‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,31 @@ describe('<View>', () => {
197197
).toEqual(<rn-view transform='[{"translateX": 10}]' />);
198198
});
199199

200+
it('resolves percentages in string translateX and translateY against the view size', () => {
201+
const root = Fantom.createRoot();
202+
203+
const viewRef = createRef<HostInstance>();
204+
Fantom.runTask(() => {
205+
root.render(
206+
<View
207+
ref={viewRef}
208+
style={{
209+
width: 200,
210+
height: 50,
211+
transform: 'translateX(10%) translateY(20%)',
212+
}}
213+
/>,
214+
);
215+
});
216+
217+
const viewBounds = nullthrows(
218+
viewRef.current,
219+
).getBoundingClientRect();
220+
// 10% of the 200 width and 20% of the 50 height.
221+
expect(viewBounds.x).toBe(20);
222+
expect(viewBounds.y).toBe(10);
223+
});
224+
200225
[
201226
[undefined, {x: -5, y: 0, width: 20, height: 10}],
202227
['50% 50%', {x: -5, y: 0, width: 20, height: 10}],

‎packages/react-native/Libraries/StyleSheet/__tests__/processTransform-itest.js‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,4 +148,35 @@ describe('processTransform', () => {
148148
);
149149
});
150150
});
151+
152+
describe('string parsing', () => {
153+
it('should keep the percentage unit for translateX and translateY', () => {
154+
expect(processTransform('translateX(10%)')).toEqual([
155+
{translateX: '10%'},
156+
]);
157+
expect(processTransform('translateY(-25.5%)')).toEqual([
158+
{translateY: '-25.5%'},
159+
]);
160+
expect(processTransform('translateX(10px)')).toEqual([{translateX: 10}]);
161+
});
162+
163+
it('should keep the percentage unit for translate', () => {
164+
expect(processTransform('translate(10%, 20px)')).toEqual([
165+
{translate: ['10%', 20]},
166+
]);
167+
});
168+
169+
it('should parse numbers without a leading zero', () => {
170+
expect(processTransform('translateX(.5px)')).toEqual([{translateX: 0.5}]);
171+
expect(processTransform('translateY(-.25px)')).toEqual([
172+
{translateY: -0.25},
173+
]);
174+
expect(processTransform('translate(.5px, .25px)')).toEqual([
175+
{translate: [0.5, 0.25]},
176+
]);
177+
expect(processTransform('matrix(.5, 0, 0, .5, 10, 20, 0, 0, 1)')).toEqual(
178+
[{matrix: [0.5, 0, 0, 0.5, 10, 20, 0, 0, 1]}],
179+
);
180+
});
181+
});
151182
});

‎packages/react-native/Libraries/StyleSheet/processTransform.js‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,16 @@ const _getKeyAndValueFromCSSTransform: (
6060
key,
6161
args,
6262
) => {
63-
const argsWithUnitsRegex = new RegExp(/([+-]?\d+(\.\d+)?)([a-zA-Z]+|%)?/g);
63+
const argsWithUnitsRegex = new RegExp(
64+
/([+-]?(?:\d+(?:\.\d*)?|\.\d+))([a-zA-Z]+|%)?/g,
65+
);
6466

6567
switch (key) {
6668
case 'matrix':
67-
return {key, value: args.match(/[+-]?\d+(\.\d+)?/g)?.map(Number)};
69+
return {
70+
key,
71+
value: args.match(/[+-]?(?:\d+(?:\.\d*)?|\.\d+)/g)?.map(Number),
72+
};
6873
case 'translate':
6974
case 'translate3d':
7075
const parsedArgs = [];
@@ -73,7 +78,7 @@ const _getKeyAndValueFromCSSTransform: (
7378
let matches;
7479
while ((matches = argsWithUnitsRegex.exec(args))) {
7580
const value = Number(matches[1]);
76-
const unitOfMeasurement = matches[3];
81+
const unitOfMeasurement = matches[2];
7782

7883
if (value !== 0 && !unitOfMeasurement) {
7984
missingUnitOfMeasurement = true;
@@ -125,7 +130,7 @@ const _getKeyAndValueFromCSSTransform: (
125130
}
126131
127132
const value = Number(argMatches[1]);
128-
const unitOfMeasurement = argMatches[3];
133+
const unitOfMeasurement = argMatches[2];
129134
130135
if (__DEV__) {
131136
invariant(
@@ -135,7 +140,16 @@ const _getKeyAndValueFromCSSTransform: (
135140
);
136141
}
137142
138-
return {key, value};
143+
// The percentage unit is meaningful for translations, so keep it in the
144+
// value instead of dropping it. A bare number is interpreted as points.
145+
return {
146+
key,
147+
value:
148+
unitOfMeasurement === '%' &&
149+
(key === 'translateX' || key === 'translateY')
150+
? `${value}%`
151+
: value,
152+
};
139153

140154
default:
141155
return {key, value: !isNaN(args) ? Number(args) : args};

‎packages/react-native/ReactCommon/react/renderer/css/tests/CSSTransformTest.cpp‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,16 @@ TEST(CSSTransform, matrix_basic) {
2424
EXPECT_EQ(matrix.values[5], 6.0f);
2525
}
2626

27+
TEST(CSSTransform, matrix_leading_dot) {
28+
auto val =
29+
parseCSSProperty<CSSTransformFunction>("matrix(.5, 0, 0, -.5, 10, 20)");
30+
EXPECT_TRUE(std::holds_alternative<CSSMatrix>(val));
31+
auto& matrix = std::get<CSSMatrix>(val);
32+
33+
EXPECT_EQ(matrix.values[0], 0.5f);
34+
EXPECT_EQ(matrix.values[3], -0.5f);
35+
}
36+
2737
TEST(CSSTransform, matrix_funky) {
2838
auto val =
2939
parseCSSProperty<CSSTransformFunction>("mAtRiX( 1 , \n2,3, 4, \t5, 6)");
@@ -181,6 +191,16 @@ TEST(CSSTransform, translate_x_pct) {
181191
EXPECT_EQ(std::get<CSSPercentage>(translate.value).value, 420.0f);
182192
}
183193

194+
TEST(CSSTransform, translate_x_leading_dot) {
195+
auto val = parseCSSProperty<CSSTransformFunction>("translateX(.5px)");
196+
EXPECT_TRUE(std::holds_alternative<CSSTranslateX>(val));
197+
auto& translate = std::get<CSSTranslateX>(val);
198+
199+
EXPECT_TRUE(std::holds_alternative<CSSLength>(translate.value));
200+
EXPECT_EQ(std::get<CSSLength>(translate.value).value, 0.5f);
201+
EXPECT_EQ(std::get<CSSLength>(translate.value).unit, CSSLengthUnit::Px);
202+
}
203+
184204
TEST(CSSTransform, translate_x_funky) {
185205
auto val = parseCSSProperty<CSSTransformFunction>("transLaTeX(420%)");
186206
EXPECT_TRUE(std::holds_alternative<CSSTranslateX>(val));
@@ -224,6 +244,15 @@ TEST(CSSTransform, translate_y_pct) {
224244
EXPECT_EQ(std::get<CSSPercentage>(translate.value).value, 420.0f);
225245
}
226246

247+
TEST(CSSTransform, translate_y_leading_dot) {
248+
auto val = parseCSSProperty<CSSTransformFunction>("translateY(-.25%)");
249+
EXPECT_TRUE(std::holds_alternative<CSSTranslateY>(val));
250+
auto& translate = std::get<CSSTranslateY>(val);
251+
252+
EXPECT_TRUE(std::holds_alternative<CSSPercentage>(translate.value));
253+
EXPECT_EQ(std::get<CSSPercentage>(translate.value).value, -0.25f);
254+
}
255+
227256
TEST(CSSTransform, translate_y_funky) {
228257
auto val = parseCSSProperty<CSSTransformFunction>("transLaTeY(420%)");
229258
EXPECT_TRUE(std::holds_alternative<CSSTranslateY>(val));

0 commit comments

Comments
 (0)