Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Android textTransform style support #20572

Closed
wants to merge 2 commits into from
Closed

Android textTransform style support #20572

wants to merge 2 commits into from

Conversation

stephencookdev
Copy link

@stephencookdev stephencookdev commented Aug 7, 2018

Issue #2088 (closed, but a bit pre-emptively imo, since Android support was skipped)

Related (merged) iOS PR #18387

Related documentation PR facebook/react-native-website#500

The basic desire is to have a declarative mechanism to transform text content to uppercase or lowercase or titlecase ("capitalized").

Test Plan:

My test plan involves having copied exactly the iOS test-case, i.e. adding a test-case to the RNTester app within the component area. I then manually verified that the rendered content met my expectation.

The test itself is:

<RNTesterBlock title="Text transform">
  <Text style={{textTransform: 'uppercase'}}>
    This text should be uppercased.
  </Text>
  <Text style={{textTransform: 'lowercase'}}>
    This TEXT SHOULD be lowercased.
  </Text>
  <Text style={{textTransform: 'capitalize'}}>
    This text should be CAPITALIZED.
  </Text>
  <Text style={{textTransform: 'capitalize'}}>
    Mixed: <Text style={{textTransform: 'uppercase'}}>uppercase </Text>
    <Text style={{textTransform: 'lowercase'}}>LoWeRcAsE </Text>
    <Text style={{textTransform: 'capitalize'}}>
      capitalize each word
    </Text>
  </Text>
  <Text>
    Should be "ABC":
    <Text style={{textTransform: 'uppercase'}}>
      a<Text>b</Text>c
    </Text>
  </Text>
  <Text>
    Should be "AbC":
    <Text style={{textTransform: 'uppercase'}}>
      a<Text style={{textTransform: 'none'}}>b</Text>c
    </Text>
  </Text>
</RNTesterBlock>

And here is what that looks like:
screen shot 2018-08-07 at 23 38 55

Release Notes:

[ANDROID] [ENHANCEMENT] [Text] - added textTransform style property enabling declarative casing transformations

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 7, 2018
Copy link
Contributor

@janicduplessis janicduplessis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this!

Looks good, we just need to improve the capitalize implementation to be more robust and consistent with iOS.

return transformed;
}

private String capitalize(String rawString) {
Copy link
Contributor

@janicduplessis janicduplessis Aug 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will need to be a bit more robust to match iOS's [NSString capitalizedString]. I did some testing and something like this seems to work well and produce the same result as iOS.

import java.text.BreakIterator;

...

String text = ".aa\nbb\tcc  dd EE \r\nZZ I like to eat apples. 中文éé 我喜欢吃苹果。awdawd   ";

BreakIterator wordIterator = BreakIterator.getWordInstance();
wordIterator.setText(text);

StringBuilder res = new StringBuilder(text.length());
int start = wordIterator.first();
for (int end = wordIterator.next(); end != BreakIterator.DONE; end = wordIterator.next()) {
  String word = text.substring(start, end);
  if (Character.isLetterOrDigit(word.charAt(0))) {
    res.append(Character.toUpperCase(word.charAt(0)));
    res.append(word.substring(1).toLowerCase());
  } else {
    res.append(word);
  }
  start = end;
}
Log.d("test", res.toString());

(based on https://stackoverflow.com/a/42219474)

Might be nice to also add some more complex test cases like this. Could be a good candidate for some unit tests too.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks for this! I've used your suggestion in c580a50 and included your test string in the test view, as well as writing some UTs.

Although, I'm struggling to run them and I've noticed this in the UTs BUCK file:

rn_robolectric_test(
    name = "views",
    # TODO Disabled temporarily until Yoga linking is fixed t14964130
    # srcs = glob(['**/*.java']),
    srcs = glob(["image/*.java"]),

I can't see that issue, or run the Text UTs without hitting the linking issue mentioned 🤔 (so my UTs... probably work? 😅)

Not sure if anyone knows a way around this?

@janicduplessis
Copy link
Contributor

@facebook-github-bot shipit

@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Aug 14, 2018
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@react-native-bot
Copy link
Collaborator

This pull request was closed by Stephen Cook in 22cf5dc.

Once this commit is added to a release, you will see the corresponding version tag below the description at 22cf5dc. If the commit has a single master tag, it is not yet part of a release.

@facebook facebook locked as resolved and limited conversation to collaborators Aug 14, 2018
@react-native-bot react-native-bot added the Merged This PR has been merged. label Aug 14, 2018
kelset pushed a commit that referenced this pull request Oct 23, 2018
Summary:
Issue #2088 (closed, but a bit pre-emptively imo, since Android support was skipped)

Related (merged) iOS PR #18387

Related documentation PR facebook/react-native-website#500

The basic desire is to have a declarative mechanism to transform text content to uppercase or lowercase or titlecase ("capitalized").
Pull Request resolved: #20572

Differential Revision: D9311716

Pulled By: hramos

fbshipit-source-id: dfbb855117196958e7ae5e980700d31be07a448d
@hramos hramos removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Feb 6, 2019
rozele pushed a commit to microsoft/react-native-windows that referenced this pull request Apr 14, 2019
Summary:
Issue facebook/react-native#2088 (closed, but a bit pre-emptively imo, since Android support was skipped)

Related (merged) iOS PR facebook/react-native#18387

Related documentation PR facebook/react-native-website#500

The basic desire is to have a declarative mechanism to transform text content to uppercase or lowercase or titlecase ("capitalized").
Pull Request resolved: facebook/react-native#20572

Differential Revision: D9311716

Pulled By: hramos

fbshipit-source-id: dfbb855117196958e7ae5e980700d31be07a448d
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Platform: Android Android applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants