Skip to content

Commit 2edc0ee

Browse files
motiz88guyca
authored andcommitted
Implement letterSpacing on Android >= 5.0
Summary: `letterSpacing` is completely missing from RN Android at the moment. I've reviewed the `letterSpacing` implementations in facebook#13199, facebook#13877 and facebook#16801 (that all seem to have stalled) and managed to put together an improved one based on facebook#13199, updated to merge cleanly post facebook@6114f86, that resolves the [issues](facebook#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#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#17398 Reviewed By: mdvacca Differential Revision: D6837718 Pulled By: hramos fbshipit-source-id: 5c9d49e9cf4af6457b636416ce5fe15315aab72c
1 parent 4f5c5c7 commit 2edc0ee

File tree

8 files changed

+176
-0
lines changed

8 files changed

+176
-0
lines changed

RNTester/js/TextExample.android.js

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

RNTester/js/TextExample.ios.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,9 +513,23 @@ exports.examples = [
513513
<Text style={{letterSpacing: 9, marginTop: 5}}>
514514
letterSpacing = 9
515515
</Text>
516+
<View style={{flexDirection: 'row'}}>
517+
<Text style={{fontSize: 12, letterSpacing: 2, backgroundColor: 'fuchsia', marginTop: 5}}>
518+
With size and background color
519+
</Text>
520+
</View>
516521
<Text style={{letterSpacing: -1, marginTop: 5}}>
517522
letterSpacing = -1
518523
</Text>
524+
<Text style={{letterSpacing: 3, backgroundColor: '#dddddd', marginTop: 5}}>
525+
[letterSpacing = 3]
526+
<Text style={{letterSpacing: 0, backgroundColor: '#bbbbbb'}}>
527+
[Nested letterSpacing = 0]
528+
</Text>
529+
<Text style={{letterSpacing: 6, backgroundColor: '#eeeeee'}}>
530+
[Nested letterSpacing = 6]
531+
</Text>
532+
</Text>
519533
</View>
520534
);
521535
},

RNTester/js/TextInputExample.android.js

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -562,6 +562,31 @@ exports.examples = [
562562
);
563563
}
564564
},
565+
{
566+
title: 'letterSpacing',
567+
render: function() {
568+
return (
569+
<View>
570+
<TextInput
571+
style={[styles.singleLine, {letterSpacing: 0}]}
572+
placeholder="letterSpacing = 0"
573+
/>
574+
<TextInput
575+
style={[styles.singleLine, {letterSpacing: 2}]}
576+
placeholder="letterSpacing = 2"
577+
/>
578+
<TextInput
579+
style={[styles.singleLine, {letterSpacing: 9}]}
580+
placeholder="letterSpacing = 9"
581+
/>
582+
<TextInput
583+
style={[styles.singleLine, {letterSpacing: -1}]}
584+
placeholder="letterSpacing = -1"
585+
/>
586+
</View>
587+
);
588+
}
589+
},
565590
{
566591
title: 'Passwords',
567592
render: function() {

ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewProps.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ public class ViewProps {
8686
public static final String FONT_STYLE = "fontStyle";
8787
public static final String FONT_FAMILY = "fontFamily";
8888
public static final String LINE_HEIGHT = "lineHeight";
89+
public static final String LETTER_SPACING = "letterSpacing";
8990
public static final String NEEDS_OFFSCREEN_ALPHA_COMPOSITING = "needsOffscreenAlphaCompositing";
9091
public static final String NUMBER_OF_LINES = "numberOfLines";
9192
public static final String ELLIPSIZE_MODE = "ellipsizeMode";
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/**
2+
* Copyright (c) 2015-present, Facebook, Inc.
3+
* All rights reserved.
4+
*
5+
* This source code is licensed under the BSD-style license found in the
6+
* LICENSE file in the root directory of this source tree. An additional grant
7+
* of patent rights can be found in the PATENTS file in the same directory.
8+
*/
9+
10+
package com.facebook.react.views.text;
11+
12+
import android.annotation.TargetApi;
13+
import android.os.Build;
14+
import android.text.TextPaint;
15+
import android.text.style.MetricAffectingSpan;
16+
17+
import com.facebook.infer.annotation.Assertions;
18+
19+
/**
20+
* A {@link MetricAffectingSpan} that allows to set the letter spacing
21+
* on the selected text span.
22+
*
23+
* The letter spacing is specified in pixels, which are converted to
24+
* ems at paint time; this span must therefore be applied after any
25+
* spans affecting font size.
26+
*/
27+
@TargetApi(Build.VERSION_CODES.LOLLIPOP)
28+
public class CustomLetterSpacingSpan extends MetricAffectingSpan {
29+
30+
private final float mLetterSpacing;
31+
32+
public CustomLetterSpacingSpan(float letterSpacing) {
33+
mLetterSpacing = letterSpacing;
34+
}
35+
36+
@Override
37+
public void updateDrawState(TextPaint paint) {
38+
apply(paint);
39+
}
40+
41+
@Override
42+
public void updateMeasureState(TextPaint paint) {
43+
apply(paint);
44+
}
45+
46+
private void apply(TextPaint paint) {
47+
// mLetterSpacing and paint.getTextSize() are both in pixels,
48+
// yielding an accurate em value.
49+
if (!Float.isNaN(mLetterSpacing)) {
50+
paint.setLetterSpacing(mLetterSpacing / paint.getTextSize());
51+
}
52+
}
53+
}

ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,14 @@ private static void buildSpannedFromShadowNode(
119119
new SetSpanOperation(
120120
start, end, new BackgroundColorSpan(textShadowNode.mBackgroundColor)));
121121
}
122+
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
123+
if (textShadowNode.mLetterSpacing != Float.NaN) {
124+
ops.add(new SetSpanOperation(
125+
start,
126+
end,
127+
new CustomLetterSpacingSpan(textShadowNode.mLetterSpacing)));
128+
}
129+
}
122130
if (textShadowNode.mFontSize != UNSET) {
123131
ops.add(new SetSpanOperation(start, end, new AbsoluteSizeSpan(textShadowNode.mFontSize)));
124132
}
@@ -229,6 +237,7 @@ private static int parseNumericFontWeight(String fontWeightString) {
229237
}
230238

231239
protected float mLineHeight = Float.NaN;
240+
protected float mLetterSpacing = Float.NaN;
232241
protected boolean mIsColorSet = false;
233242
protected boolean mAllowFontScaling = true;
234243
protected int mColor;
@@ -239,6 +248,7 @@ private static int parseNumericFontWeight(String fontWeightString) {
239248
protected int mFontSize = UNSET;
240249
protected float mFontSizeInput = UNSET;
241250
protected float mLineHeightInput = UNSET;
251+
protected float mLetterSpacingInput = Float.NaN;
242252
protected int mTextAlign = Gravity.NO_GRAVITY;
243253
protected int mTextBreakStrategy =
244254
(Build.VERSION.SDK_INT < Build.VERSION_CODES.M) ? 0 : Layout.BREAK_STRATEGY_HIGH_QUALITY;
@@ -324,12 +334,22 @@ public void setLineHeight(float lineHeight) {
324334
markUpdated();
325335
}
326336

337+
@ReactProp(name = ViewProps.LETTER_SPACING, defaultFloat = Float.NaN)
338+
public void setLetterSpacing(float letterSpacing) {
339+
mLetterSpacingInput = letterSpacing;
340+
mLetterSpacing = mAllowFontScaling
341+
? PixelUtil.toPixelFromSP(mLetterSpacingInput)
342+
: PixelUtil.toPixelFromDIP(mLetterSpacingInput);
343+
markUpdated();
344+
}
345+
327346
@ReactProp(name = ViewProps.ALLOW_FONT_SCALING, defaultBoolean = true)
328347
public void setAllowFontScaling(boolean allowFontScaling) {
329348
if (allowFontScaling != mAllowFontScaling) {
330349
mAllowFontScaling = allowFontScaling;
331350
setFontSize(mFontSizeInput);
332351
setLineHeight(mLineHeightInput);
352+
setLetterSpacing(mLetterSpacingInput);
333353
markUpdated();
334354
}
335355
}

ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import android.widget.EditText;
3535
import com.facebook.infer.annotation.Assertions;
3636
import com.facebook.react.bridge.ReactContext;
37+
import com.facebook.react.uimanager.PixelUtil;
3738
import com.facebook.react.uimanager.UIManagerModule;
3839
import com.facebook.react.views.text.CustomStyleSpan;
3940
import com.facebook.react.views.text.ReactTagSpan;
@@ -81,6 +82,7 @@ public class ReactEditText extends EditText {
8182
private @Nullable ScrollWatcher mScrollWatcher;
8283
private final InternalKeyListener mKeyListener;
8384
private boolean mDetectScrollMovement = false;
85+
private float mLetterSpacingPt = 0;
8486

8587
private ReactViewBackgroundManager mReactBackgroundManager;
8688

@@ -607,6 +609,29 @@ public void setBorderStyle(@Nullable String style) {
607609
mReactBackgroundManager.setBorderStyle(style);
608610
}
609611

612+
public void setLetterSpacingPt(float letterSpacingPt) {
613+
mLetterSpacingPt = letterSpacingPt;
614+
updateLetterSpacing();
615+
}
616+
617+
@Override
618+
public void setTextSize (float size) {
619+
super.setTextSize(size);
620+
updateLetterSpacing();
621+
}
622+
623+
@Override
624+
public void setTextSize (int unit, float size) {
625+
super.setTextSize(unit, size);
626+
updateLetterSpacing();
627+
}
628+
629+
protected void updateLetterSpacing() {
630+
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
631+
setLetterSpacing(PixelUtil.toPixelFromSP(mLetterSpacingPt) / getTextSize());
632+
}
633+
}
634+
610635
/**
611636
* This class will redirect *TextChanged calls to the listeners only in the case where the text
612637
* is changed by the user, and not explicitly set by JS.

ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,14 @@ public void setOnScroll(final ReactEditText view, boolean onScroll) {
297297
}
298298
}
299299

300+
// Sets the letter spacing as an absolute point size.
301+
// This extra handling, on top of what ReactBaseTextShadowNode already does, is required for the
302+
// correct display of spacing in placeholder (hint) text.
303+
@ReactProp(name = ViewProps.LETTER_SPACING, defaultFloat = 0)
304+
public void setLetterSpacing(ReactEditText view, float letterSpacing) {
305+
view.setLetterSpacingPt(letterSpacing);
306+
}
307+
300308
@ReactProp(name = "placeholder")
301309
public void setPlaceholder(ReactEditText view, @Nullable String placeholder) {
302310
view.setHint(placeholder);

0 commit comments

Comments
 (0)