Fix textTransform when used with other text styles on Android by janicduplessis · Pull Request #22670 · react/react-native

janicduplessis · 2018-12-16T01:26:59Z

On Android textTransform breaks other styles applied to the text. It seems related to the usage of ReplacementSpan which allows drawing the text manually but seems to throw away some changes made by other span applied to the text.

To fix it I removed the usage of ReplacementSpan and simply transform the text before appending it to the Spannable string. To make sure textTransform is inherited correctly I added it to TextAttributes which handles this.

Test Plan:

I added a test case to RNTester. It should render turquoise text on a blue background. It also adds line height and letter spacing to make sure the text is properly measured and all styles are applied.

Before the fix:

After the fix:

Also made sure all existing text transform examples produce the expected result.

Changelog:

[Android] [Fixed] - Fix textTransform when used with other text styles on Android

analysis-bot

Code analysis results:

eslint found some issues.

kristerkari · 2018-12-16T10:18:32Z

This will most likely fix #21966, right?

janicduplessis · 2018-12-16T18:41:53Z

@kristerkari I think so, I can test with an example from the issue

facebook-github-bot

@hramos has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

rigdern · 2018-12-17T23:29:30Z

@janicduplessis I have a question about setTextTransform (I know you didn't change it in this PR).

The default for mTextTransform is UNSET but setTextTransform maps null to NONE. Is this a bug? Should setTextTransform be mapping null to UNSET instead?

janicduplessis · 2018-12-18T01:56:18Z

@rigdern Quite possible, I just tested on snack but textTransform on android doesn't work on latest expo yet.

This should print aB

<Textstyle={{textTransform: 'uppercase'}}><Textstyle={{textTransform: 'none'}}>a</Text><Textstyle={{textTransform: null}}>b</Text></Text>

I can test it when I get on a computer.

janicduplessis · 2018-12-18T22:01:35Z

@rigdern you were right #22694

hramos · 2019-01-11T23:57:10Z

@rigdern's changes landed in f6f8b09. @janicduplessis do you still want your changes to be incorporated? If so, let me know if you can resolve the conflicts.

Thanks to both of you for working on getting these issues addressed!

janicduplessis · 2019-01-12T16:43:01Z

@hramos Looks like #22917 will also conflict with this. I think we should try to land #22917 first and then I can rebase this on top of the improvements to text inheritance that @rigdern implemented.

rigdern · 2019-01-15T00:50:42Z

@janicduplessis #22917 has been merged.

analysis-bot

Code analysis results:

eslint found some issues.

janicduplessis · 2019-01-19T05:49:28Z

@hramos @rigdern Rebased on top of #22917 and re-tested to make sure everything still works. I also included the little fix in #22694 since I ended up changing these lines.

analysis-bot

Code analysis results:

eslint found some issues.

analysis-bot

Code analysis results:

eslint found some issues.

analysis-bot · 2019-01-24T21:56:24Z

 }
 </Text>
+ <Text
+ style={{


react-native/no-inline-styles: Inline style: { textTransform: 'uppercase',
fontSize: 16,
color: 'turquoise',
backgroundColor: 'blue',
lineHeight: 32,
letterSpacing: 2,
alignSelf: 'flex-start' }

rigdern · 2019-01-24T22:04:52Z

@janicduplessis The PR description says:

To do that I can pass the parent's textTransform when recursing through text shadow nodes.

This sentence seems to need to be updated now that this information is passed around via TextAttributes.java

facebook-github-bot

@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

react-native-bot · 2019-01-25T14:30:05Z

@janicduplessis merged commit 3a33e75 into facebook:master.

Summary: On Android `textTransform` breaks other styles applied to the text. It seems related to the usage of `ReplacementSpan` which allows drawing the text manually but seems to throw away some changes made by other span applied to the text. To fix it I removed the usage of `ReplacementSpan` and simply transform the text before appending it to the `Spannable` string. To make sure textTransform is inherited correctly I added it to TextAttributes which handles this. Pull Request resolved: react/react-native#22670 Differential Revision: D13494819 Pulled By: cpojer fbshipit-source-id: 1c69591084aa906c2d3b10153b354d39c0936340

facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Tests This PR adds or edits a test case. labels Dec 16, 2018

analysis-bot suggested changes Dec 16, 2018
View reviewed changes

Comment threadRNTester/js/TextExample.android.js Outdated

react-native-bot added 🔶Components Component: Text Platform: Android Android applications. labels Dec 16, 2018

facebook-github-bot reviewed Dec 17, 2018
View reviewed changes

janicduplessis mentioned this pull request Dec 17, 2018
Android TextInput: Improve application of styles for value prop #22461
Closed

kristerkari mentioned this pull request Dec 20, 2018
[0.57.4-0.57.8/0.58.6] using textTransform causes text to disappear on Android, but works on iOS #21966
Closed
3 tasks

react-native-bot added ✅Changelog labels Jan 15, 2019

janicduplessis added 2 commits January 19, 2019 00:01

Fix textTransform when used with other text styles on Android
c70df42

Rebase on top of text inheritance changes
ca640db

janicduplessis force-pushed the fix-android-text-transform branch from 15768b5 to ca640dbCompare January 19, 2019 05:45

analysis-bot suggested changes Jan 19, 2019
View reviewed changes

Comment threadRNTester/js/TextExample.android.js

janicduplessis mentioned this pull request Jan 19, 2019
Fix textTransform inheritance on Android #22694
Closed

Update ReactBaseTextShadowNode.java
9180e14

analysis-bot suggested changes Jan 19, 2019
View reviewed changes

Comment threadRNTester/js/TextExample.android.js

Merge branch 'master' into fix-android-text-transform
22fdcf2

analysis-bot suggested changes Jan 24, 2019
View reviewed changes

rigdern approved these changes Jan 24, 2019
View reviewed changes

hramos removed the 🔶Components label Jan 24, 2019

cpojer approved these changes Jan 25, 2019
View reviewed changes

facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Jan 25, 2019

facebook-github-bot approved these changes Jan 25, 2019
View reviewed changes

facebook-github-bot closed this in 3a33e75Jan 25, 2019

react locked as resolved and limited conversation to collaborators Jan 25, 2019

react-native-bot added the Merged This PR has been merged. label Jan 25, 2019

hramos removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Feb 6, 2019

react-native-bot added the Bug label May 3, 2019

facebook-github-bot added the Contributor A React Native contributor. label Apr 15, 2024

Uh oh!

Conversation

janicduplessis commented Dec 16, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Plan:

Changelog:

Uh oh!

analysis-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kristerkari commented Dec 16, 2018

Uh oh!

janicduplessis commented Dec 16, 2018

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

rigdern commented Dec 17, 2018

Uh oh!

janicduplessis commented Dec 18, 2018

Uh oh!

janicduplessis commented Dec 18, 2018

Uh oh!

hramos commented Jan 11, 2019

Uh oh!

janicduplessis commented Jan 12, 2019

Uh oh!

rigdern commented Jan 15, 2019

Uh oh!

analysis-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

janicduplessis commented Jan 19, 2019

Uh oh!

analysis-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

analysis-bot left a comment

Choose a reason for hiding this comment

Uh oh!

analysis-botJan 24, 2019

Choose a reason for hiding this comment

Uh oh!

rigdern commented Jan 24, 2019

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

react-native-bot commented Jan 25, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

janicduplessis commented Dec 16, 2018•
edited
Loading