[Style] "letterSpacing" and "padding" is not effective on <Text />

[LeoJiaXin]

platform: Android4.3
simulator: Genymotion,Google Nexus 4, Android 4.3
react native version: 0.12.0
react-native-cli: 0.1.5

var React = require('react-native');

var {
 StyleSheet,
 Text
} = React;

var Test = React.createClass({
 render: function() {
 return (
 <Text style={styles.text}>123123</Text>
 );
 }
});

var styles = StyleSheet.create({
 text: {
 letterSpacing: 20,
 padding: 10,
 backgroundColor: '#f00'
 }
});

but result is:

seems letterSpacing and padding is not supported

[brentvatne]

Confirmed in this example: https://rnplay.org/apps/5M_0Nw

cc @kmagiera @foghina

[charliermarsh]

Seems like the letter spacing attribute was only added in API 21, but RN's minimum supported version is API 16.

[ide]

@crm416 thanks for the research.

@mkonicek @vjeux what do you want to do here if old Android doesn't support a feature? Looks like AppCompatTextView doesn't backport letterSpacing (https://developer.android.com/reference/android/support/v7/widget/AppCompatTextView.html) so we may need to reimplement text rendering if we wanted this. Or we could get it working on modern Android (if it doesn't work already) and mention that in the RN docs.

[kmagiera]

We don't expose this property yet on android (even on API 21+)

On API <21 we could potentially implement it using ReplacementSpan, but I'd rather only support it whenever available as using ReplacementSpan may not be the most efficient way to do that. One can always use custom font to achieve similar results although it won't be possible to tweak the value just by changing a single param. We already have a number of props that we support only for certain android release (e.g. some of the accessibility properties or borderless ripple params) so I think it would be fine to do the same here.

Anyone fancy sending PR?

[ide]

We already have a number of props that we support only for certain android release (e.g. some of the accessibility properties or borderless ripple params) so I think it would be fine to do the same here.

Sounds like a good plan. I've marked this as "Help Wanted" for enthusiastic contributors.

[charliermarsh]

@kmagiera I'd be happy to give it a shot.

[PhilippKrone]

@crm416 This seems to be connected to #3233 - are you working on it? If yes, those two issues could be merged. @ide

[ide]

Most of the convo in this issue is about letterSpacing and #3233 is more about padding so let's keep them separate for now. If both issues are fixed in the same PR, just write "Fixes #3485, fixes #3233".

[PhilippKrone]

@crm416 Did you already have a chance to take a shot here? :)

[charliermarsh]

@PhilippKrone not yet! been a little busy. feel free to take it over if you're interested.

[charliermarsh]

I started working on this tonight, but it's turning out to be non-trivial. The issue is that bumping up the letter spacing can in turn increase the number of lines in the text, and so ReactTextShadowNode has to account for that possibility (else, if you bump up the letter spacing and your text now spans an additional line or two, those lines will get cutoff). There isn't really a simple way to do that, since the final number of lines will be based on the font size and the actual contents of the text. (Aside: letter spacing is specified in em, unlike almost any other property in Android, which doesn't help.) So, I think that the letter spacing value will have to be passed all the way down to the StaticLayout constructor, but the code in that file is pretty complex, and I wanted to check back here before diving in.

On that note: please let me know if I'm missing something obvious! Very new to this codebase.