Don't capitalize words starting by a number (fabric renderer)#33629
Closed
MaeIg wants to merge 1 commit into
Closed
Don't capitalize words starting by a number (fabric renderer)#33629MaeIg wants to merge 1 commit into
MaeIg wants to merge 1 commit into
Conversation
Contributor
|
@GijsWeterings has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Base commit: e48a580 |
Base commit: e48a580 |
Collaborator
|
This pull request was successfully merged by @MaeIg in db284ae. When will my fix make it into a release? | Upcoming Releases |
Saadnajmi
pushed a commit
to Saadnajmi/react-native-macos
that referenced
this pull request
Jan 15, 2023
…ok#33629) Summary: <!-- Explain the **motivation** for making this change. What existing problem does the pull request solve? --> Few month ago, I created a [pull request](facebook#32774) to unify the behavior of capitalize style between Android and IOS. But, I found out that it doesn't work when fabric is enabled. We have the old behavior : | Android | IOS | | ------------- | ------------- | | <img width="458" alt="capitalize_android_fabric" src="https://user-images.githubusercontent.com/40902940/163182082-4061003c-230b-46f7-9e93-c34b66dbf3d2.png"> | <img width="476" alt="capitalize_ios_fabric" src="https://user-images.githubusercontent.com/40902940/163182124-b6dee450-46e3-41a3-b5bb-553d7c2662e6.png"> | (source: rn-tester, last commit: dac56ce) As fabric is now live since v0.68, we should fix this behavior for fabric aswell. I don't know why there is so much duplicated code between `ReactCommon/react/renderer/textlayoutmanager/platform/ios/RCTAttributedTextUtils.mm` and `Libraries/Text/RCTTextAttributes.m` because I don't know the architecture of the project very well. But if you see missing tests or some refacto to do I'm open to suggestions! ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://github.com/facebook/react-native/wiki/Changelog --> [iOS] [Fixed] - Don't capitalize the first letter of a word that is starting by a number (Fabric renderer) Pull Request resolved: facebook#33629 Test Plan: I manually tested these changes on rn-tester (react-native `main` branch): | Fabric ? | Android | IOS | | ------------- | ------------- | ------------- | | YES | <img width="458" alt="capitalize_android_fabric" src="https://user-images.githubusercontent.com/40902940/163182082-4061003c-230b-46f7-9e93-c34b66dbf3d2.png"> | <img width="476" alt="capitalize_ios_fabric_after" src="https://user-images.githubusercontent.com/40902940/163184277-6c58c117-7144-4f6b-98ea-0c1db654f27b.png"> | | NO | <img width="458" alt="android_nf_after" src="https://user-images.githubusercontent.com/40902940/163190263-9d801f6a-09c2-4ec6-a841-3dca115a5ef7.png"> | <img width="476" alt="ios_nf_after" src="https://user-images.githubusercontent.com/40902940/163190333-7e9eac6a-3f28-4826-8ef9-bcf45bf870a9.png"> | Reviewed By: cortinico Differential Revision: D35611086 Pulled By: GijsWeterings fbshipit-source-id: 0c43807dcddb30e65921eb1525c0fe440162ec32
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
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
Few month ago, I created a pull request to unify the behavior of capitalize style between Android and IOS.
But, I found out that it doesn't work when fabric is enabled. We have the old behavior :
As fabric is now live since v0.68, we should fix this behavior for fabric aswell.
I don't know why there is so much duplicated code between
ReactCommon/react/renderer/textlayoutmanager/platform/ios/RCTAttributedTextUtils.mmandLibraries/Text/RCTTextAttributes.mbecause I don't know the architecture of the project very well. But if you see missing tests or some refacto to do I'm open to suggestions!Changelog
[iOS] [Fixed] - Don't capitalize the first letter of a word that is starting by a number (Fabric renderer)
Test Plan
I manually tested these changes on rn-tester (react-native
mainbranch):