Fix textTransform when used with other text styles on Android #22670

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.

RNTester/js/TextExample.android.js

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

<Text style={{textTransform: 'uppercase'}}>
  <Text style={{textTransform: 'none'}}>a</Text>
  <Text style={{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.

RNTester/js/TextExample.android.js

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.

RNTester/js/TextExample.android.js

analysis-bot

Code analysis results:

eslint found some issues.

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

RNTester/js/TextExample.android.js

@@ -600,6 +600,18 @@ class TextExample extends React.Component<{}> {
              '.aa\tbb\t\tcc  dd EE \r\nZZ I like to eat apples. \n中文éé 我喜欢吃苹果。awdawd   '
            }
          </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.

…ok#22670) 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: facebook#22670 Differential Revision: D13494819 Pulled By: cpojer fbshipit-source-id: 1c69591084aa906c2d3b10153b354d39c0936340

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: facebook/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

RNTester/js/TextExample.android.js Show resolved Hide resolved

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 ca640db Compare January 19, 2019 05:45

analysis-bot suggested changes Jan 19, 2019

View reviewed changes

RNTester/js/TextExample.android.js Show resolved Hide resolved

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

RNTester/js/TextExample.android.js Show resolved Hide resolved

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 3a33e75 Jan 25, 2019

facebook 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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix textTransform when used with other text styles on Android #22670

Fix textTransform when used with other text styles on Android #22670

janicduplessis commented Dec 16, 2018 •

edited

Loading

analysis-bot left a comment

kristerkari commented Dec 16, 2018

janicduplessis commented Dec 16, 2018

facebook-github-bot left a comment

rigdern commented Dec 17, 2018

janicduplessis commented Dec 18, 2018

janicduplessis commented Dec 18, 2018

hramos commented Jan 11, 2019

janicduplessis commented Jan 12, 2019

rigdern commented Jan 15, 2019

analysis-bot left a comment

janicduplessis commented Jan 19, 2019

analysis-bot left a comment

analysis-bot left a comment

analysis-bot Jan 24, 2019

rigdern commented Jan 24, 2019

facebook-github-bot left a comment

react-native-bot commented Jan 25, 2019

Fix textTransform when used with other text styles on Android #22670

Fix textTransform when used with other text styles on Android #22670

Conversation

janicduplessis commented Dec 16, 2018 • edited Loading

Test Plan:

Changelog:

analysis-bot left a comment

Choose a reason for hiding this comment

kristerkari commented Dec 16, 2018

janicduplessis commented Dec 16, 2018

facebook-github-bot left a comment

Choose a reason for hiding this comment

rigdern commented Dec 17, 2018

janicduplessis commented Dec 18, 2018

janicduplessis commented Dec 18, 2018

hramos commented Jan 11, 2019

janicduplessis commented Jan 12, 2019

rigdern commented Jan 15, 2019

analysis-bot left a comment

Choose a reason for hiding this comment

janicduplessis commented Jan 19, 2019

analysis-bot left a comment

Choose a reason for hiding this comment

analysis-bot left a comment

Choose a reason for hiding this comment

analysis-bot Jan 24, 2019

Choose a reason for hiding this comment

rigdern commented Jan 24, 2019

facebook-github-bot left a comment

Choose a reason for hiding this comment

react-native-bot commented Jan 25, 2019

janicduplessis commented Dec 16, 2018 •

edited

Loading