Skip to content

Commit 602365b

Browse files
motiz88facebook-github-bot
motiz88
authored and
facebook-github-bot
committed
Implement letterSpacing on Android >= 5.0
Summary: `letterSpacing` is completely missing from RN Android at the moment. I've reviewed the `letterSpacing` implementations in microsoft#13199, microsoft#13877 and #16801 (that all seem to have stalled) and managed to put together an improved one based on microsoft#13199, updated to merge cleanly post facebook/react-native@6114f86, that resolves the [issues](facebook/react-native#13199 (comment)) I've identified with that code. I believe this is the closest PR yet to a correct implementation of this feature, with a few caveats: - As with the other PRs, this only works on Android >= 5.0 (silently falling back to no letter spacing on older versions). Is this acceptable for a RN feature, in general? Would a dev mode warning be desirable? - The other PRs seem to have explored the space of potential solutions to the layout issue ([Android renders space _around_ glyphs](https://issuetracker.google.com/issues/37079859), iOS to the _right_ of each one) and come up empty, so I've opted to merely document the difference. - I have neither updated nor tested the "Flat" UI implementation - everything compiles but I've taken [this comment](facebook/react-native#12770 (comment)) to mean there's no point in trying to wade through it on my own right now; I'm happy to tackle it if given some pointers. - The implementation in `ReactEditText` is only there to handle the placeholder text, as `ReactBaseTextShadowNode` already affects the input control's contents correctly. - I'm not sure whether `<TextInput>` is meant to respect `allowFontScaling`; I've taken my cue here from `ReactTextInputManager.setFontSize()`, and used the same units (SP) to interpret the value in `ReactEditText.setLetterSpacingPt()`. - I'm not sure whether `<TextInput>` is even meant to support `letterSpacing` - it doesn't actually work on iOS. I'm not going to be able to handle the Objective-C side of this, not as part of this PR at least. - I have not added unit tests to `ReactTextTest` - is this desirable? I see that some other props such as `lineHeight` aren't covered there (unless I'm not looking in the right place). - Overall, I'm new to this codebase, so it's likely I've missed something not mentioned here. Note comment re: unit tests above; RNTester screenshots follow. | iOS (existing functionality, amended test) | Android (new functionality & test) | | - | - | | <img src=https://user-images.githubusercontent.com/2246565/34458459-c8d59498-edcb-11e7-8c8f-e7426f723886.png width=300> | <img src=https://user-images.githubusercontent.com/2246565/34458473-2a1ca368-edcc-11e7-9ce6-30c6d3a48660.png width=300> | | iOS _(not implemented, test not in this branch)_ | Android (new functionality & test) | | - | - | | <img src=https://user-images.githubusercontent.com/2246565/34458481-6c60a36e-edcc-11e7-9af5-9734dd722ced.png width=300> | <img src=https://user-images.githubusercontent.com/2246565/34458486-8b3cdcf8-edcc-11e7-974b-25c6085fa674.png width=300> | | iOS _(not implemented, test not in this branch)_ | Android (new functionality & test) | | - | - | | <img src=https://user-images.githubusercontent.com/2246565/34458492-d69a77be-edcc-11e7-896f-21212621dbee.png width=300> | <img src=https://user-images.githubusercontent.com/2246565/34458490-b3a1139e-edcc-11e7-88c8-79d4430d1514.png width=300> | facebook/react-native-website#105 - this docs PR is edited slightly from what's in `TextStylePropTypes` here; happy to align either one to the other after a review. [ANDROID] [FEATURE] [Text] - Implemented letterSpacing Closes facebook/react-native#17398 Reviewed By: mdvacca Differential Revision: D6837718 Pulled By: hramos fbshipit-source-id: 5c9d49e9cf4af6457b636416ce5fe15315aab72c
1 parent 4ffb70a commit 602365b

File tree

3 files changed

+69
-0
lines changed

3 files changed

+69
-0
lines changed

‎js/TextExample.android.js

+30
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,36 @@ class TextExample extends React.Component<{}> {
341341
Holisticly formulate inexpensive ideas before best-of-breed benefits. <Text style={{fontSize: 20}}>Continually</Text> expedite magnetic potentialities rather than client-focused interfaces.
342342
</Text>
343343
</RNTesterBlock>
344+
<RNTesterBlock title="Letter Spacing">
345+
<View>
346+
<Text style={{letterSpacing: 0}}>
347+
letterSpacing = 0
348+
</Text>
349+
<Text style={{letterSpacing: 2, marginTop: 5}}>
350+
letterSpacing = 2
351+
</Text>
352+
<Text style={{letterSpacing: 9, marginTop: 5}}>
353+
letterSpacing = 9
354+
</Text>
355+
<View style={{flexDirection: 'row'}}>
356+
<Text style={{fontSize: 12, letterSpacing: 2, backgroundColor: 'fuchsia', marginTop: 5}}>
357+
With size and background color
358+
</Text>
359+
</View>
360+
<Text style={{letterSpacing: -1, marginTop: 5}}>
361+
letterSpacing = -1
362+
</Text>
363+
<Text style={{letterSpacing: 3, backgroundColor: '#dddddd', marginTop: 5}}>
364+
[letterSpacing = 3]
365+
<Text style={{letterSpacing: 0, backgroundColor: '#bbbbbb'}}>
366+
[Nested letterSpacing = 0]
367+
</Text>
368+
<Text style={{letterSpacing: 6, backgroundColor: '#eeeeee'}}>
369+
[Nested letterSpacing = 6]
370+
</Text>
371+
</Text>
372+
</View>
373+
</RNTesterBlock>
344374
<RNTesterBlock title="Empty Text">
345375
<Text />
346376
</RNTesterBlock>

‎js/TextExample.ios.js

+14
Original file line numberDiff line numberDiff line change
@@ -575,9 +575,23 @@ exports.examples = [
575575
<Text style={{letterSpacing: 9, marginTop: 5}}>
576576
letterSpacing = 9
577577
</Text>
578+
<View style={{flexDirection: 'row'}}>
579+
<Text style={{fontSize: 12, letterSpacing: 2, backgroundColor: 'fuchsia', marginTop: 5}}>
580+
With size and background color
581+
</Text>
582+
</View>
578583
<Text style={{letterSpacing: -1, marginTop: 5}}>
579584
letterSpacing = -1
580585
</Text>
586+
<Text style={{letterSpacing: 3, backgroundColor: '#dddddd', marginTop: 5}}>
587+
[letterSpacing = 3]
588+
<Text style={{letterSpacing: 0, backgroundColor: '#bbbbbb'}}>
589+
[Nested letterSpacing = 0]
590+
</Text>
591+
<Text style={{letterSpacing: 6, backgroundColor: '#eeeeee'}}>
592+
[Nested letterSpacing = 6]
593+
</Text>
594+
</Text>
581595
</View>
582596
);
583597
},

‎js/TextInputExample.android.js

+25
Original file line numberDiff line numberDiff line change
@@ -567,6 +567,31 @@ exports.examples = [
567567
);
568568
}
569569
},
570+
{
571+
title: 'letterSpacing',
572+
render: function() {
573+
return (
574+
<View>
575+
<TextInput
576+
style={[styles.singleLine, {letterSpacing: 0}]}
577+
placeholder="letterSpacing = 0"
578+
/>
579+
<TextInput
580+
style={[styles.singleLine, {letterSpacing: 2}]}
581+
placeholder="letterSpacing = 2"
582+
/>
583+
<TextInput
584+
style={[styles.singleLine, {letterSpacing: 9}]}
585+
placeholder="letterSpacing = 9"
586+
/>
587+
<TextInput
588+
style={[styles.singleLine, {letterSpacing: -1}]}
589+
placeholder="letterSpacing = -1"
590+
/>
591+
</View>
592+
);
593+
}
594+
},
570595
{
571596
title: 'Passwords',
572597
render: function() {

0 commit comments

Comments
 (0)