Repository navigation
Conversation
`processTransform` dropped the `%` unit when parsing `translateX(...)` and
`translateY(...)` from a CSS transform string, so `'translateX(10%)'` became
`[{translateX: 10}]`, which native interprets as 10 points. `translate(...)`,
the array form, and the native CSS parser already preserve percentages.
The parser also required a digit before the decimal point, so
`translateX(.5px)` and `matrix(.5, ...)` parsed `.5` as `5`.
Adds Fantom coverage for the JS parser and a View layout test, plus native
CSS parser tests for numbers without a leading zero.
meta-cla
Bot
added
the
CLA Signed
facebook-github-tools
Bot
added
the
Shared with Meta
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
processTransformdrops the%unit when it parsestranslateX(...)ortranslateY(...)from a CSS transform string: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 thetranslateX/translateYbranch of the JS string parser discards it.The same parser also required a digit before the decimal point, so
translateX(.5px)andmatrix(.5, …)parsed.5as5. The array form and the native CSS parser both accept these. #57487 fixed the same problem inprocessTransformOrigin.Changelog:
[GENERAL] [FIXED] - Keep percentages in
translateX/translateYstring transforms and parse numbers without a leading zeroTest 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}, andtranslateX(.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>withtransform: 'translateX(10%) translateY(20%)'and checksgetBoundingClientRect(). Without the fix, the 2enableNativeCSSParsing🛑 variants fail withExpected 20 but received 10. The 2 ✅ variants already passed because the native parser handles it.CSSTransformTest.cpp: addedmatrix_leading_dot,translate_x_leading_dotandtranslate_y_leading_dotto 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 to5.0fmakestranslate_x_leading_dotfail withWhich is: 0.5, so the assertion is checking the parsed value.yarn flow-check: 0 errors. ESLint--max-warnings 0and Prettier on the changed JS files are clean, andnode scripts/clang-format.js --checkon 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.