Skip to content

Commit

Permalink
Implement letterSpacing on Android >= 5.0
Browse files Browse the repository at this point in the history
Summary:
`letterSpacing` is completely missing from RN Android at the moment.

I've reviewed the `letterSpacing` implementations in #13199, #13877 and #16801 (that all seem to have stalled) and managed to put together an improved one based on #13199, updated to merge cleanly post 6114f86, that resolves the [issues](#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](#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 #17398

Reviewed By: mdvacca

Differential Revision: D6837718

Pulled By: hramos

fbshipit-source-id: 5c9d49e9cf4af6457b636416ce5fe15315aab72c
  • Loading branch information
motiz88 authored and facebook-github-bot committed Feb 27, 2018
1 parent dbafc29 commit 5898817
Show file tree
Hide file tree
Showing 8 changed files with 176 additions and 0 deletions.
30 changes: 30 additions & 0 deletions RNTester/js/TextExample.android.js
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,36 @@ class TextExample extends React.Component<{}> {
Holisticly formulate inexpensive ideas before best-of-breed benefits. <Text style={{fontSize: 20}}>Continually</Text> expedite magnetic potentialities rather than client-focused interfaces.
</Text>
</RNTesterBlock>
<RNTesterBlock title="Letter Spacing">
<View>
<Text style={{letterSpacing: 0}}>
letterSpacing = 0
</Text>
<Text style={{letterSpacing: 2, marginTop: 5}}>
letterSpacing = 2
</Text>
<Text style={{letterSpacing: 9, marginTop: 5}}>
letterSpacing = 9
</Text>
<View style={{flexDirection: 'row'}}>
<Text style={{fontSize: 12, letterSpacing: 2, backgroundColor: 'fuchsia', marginTop: 5}}>
With size and background color
</Text>
</View>
<Text style={{letterSpacing: -1, marginTop: 5}}>
letterSpacing = -1
</Text>
<Text style={{letterSpacing: 3, backgroundColor: '#dddddd', marginTop: 5}}>
[letterSpacing = 3]
<Text style={{letterSpacing: 0, backgroundColor: '#bbbbbb'}}>
[Nested letterSpacing = 0]
</Text>
<Text style={{letterSpacing: 6, backgroundColor: '#eeeeee'}}>
[Nested letterSpacing = 6]
</Text>
</Text>
</View>
</RNTesterBlock>
<RNTesterBlock title="Empty Text">
<Text />
</RNTesterBlock>
Expand Down
14 changes: 14 additions & 0 deletions RNTester/js/TextExample.ios.js
Original file line number Diff line number Diff line change
Expand Up @@ -575,9 +575,23 @@ exports.examples = [
<Text style={{letterSpacing: 9, marginTop: 5}}>
letterSpacing = 9
</Text>
<View style={{flexDirection: 'row'}}>
<Text style={{fontSize: 12, letterSpacing: 2, backgroundColor: 'fuchsia', marginTop: 5}}>
With size and background color
</Text>
</View>
<Text style={{letterSpacing: -1, marginTop: 5}}>
letterSpacing = -1
</Text>
<Text style={{letterSpacing: 3, backgroundColor: '#dddddd', marginTop: 5}}>
[letterSpacing = 3]
<Text style={{letterSpacing: 0, backgroundColor: '#bbbbbb'}}>
[Nested letterSpacing = 0]
</Text>
<Text style={{letterSpacing: 6, backgroundColor: '#eeeeee'}}>
[Nested letterSpacing = 6]
</Text>
</Text>
</View>
);
},
Expand Down
25 changes: 25 additions & 0 deletions RNTester/js/TextInputExample.android.js
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,31 @@ exports.examples = [
);
}
},
{
title: 'letterSpacing',
render: function() {
return (
<View>
<TextInput
style={[styles.singleLine, {letterSpacing: 0}]}
placeholder="letterSpacing = 0"
/>
<TextInput
style={[styles.singleLine, {letterSpacing: 2}]}
placeholder="letterSpacing = 2"
/>
<TextInput
style={[styles.singleLine, {letterSpacing: 9}]}
placeholder="letterSpacing = 9"
/>
<TextInput
style={[styles.singleLine, {letterSpacing: -1}]}
placeholder="letterSpacing = -1"
/>
</View>
);
}
},
{
title: 'Passwords',
render: function() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ public class ViewProps {
public static final String FONT_STYLE = "fontStyle";
public static final String FONT_FAMILY = "fontFamily";
public static final String LINE_HEIGHT = "lineHeight";
public static final String LETTER_SPACING = "letterSpacing";
public static final String NEEDS_OFFSCREEN_ALPHA_COMPOSITING = "needsOffscreenAlphaCompositing";
public static final String NUMBER_OF_LINES = "numberOfLines";
public static final String ELLIPSIZE_MODE = "ellipsizeMode";
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/**
* Copyright (c) 2015-present, Facebook, Inc.
* All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*/

package com.facebook.react.views.text;

import android.annotation.TargetApi;
import android.os.Build;
import android.text.TextPaint;
import android.text.style.MetricAffectingSpan;

import com.facebook.infer.annotation.Assertions;

/**
* A {@link MetricAffectingSpan} that allows to set the letter spacing
* on the selected text span.
*
* The letter spacing is specified in pixels, which are converted to
* ems at paint time; this span must therefore be applied after any
* spans affecting font size.
*/
@TargetApi(Build.VERSION_CODES.LOLLIPOP)
public class CustomLetterSpacingSpan extends MetricAffectingSpan {

private final float mLetterSpacing;

public CustomLetterSpacingSpan(float letterSpacing) {
mLetterSpacing = letterSpacing;
}

@Override
public void updateDrawState(TextPaint paint) {
apply(paint);
}

@Override
public void updateMeasureState(TextPaint paint) {
apply(paint);
}

private void apply(TextPaint paint) {
// mLetterSpacing and paint.getTextSize() are both in pixels,
// yielding an accurate em value.
if (!Float.isNaN(mLetterSpacing)) {
paint.setLetterSpacing(mLetterSpacing / paint.getTextSize());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,14 @@ private static void buildSpannedFromShadowNode(
new SetSpanOperation(
start, end, new BackgroundColorSpan(textShadowNode.mBackgroundColor)));
}
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
if (textShadowNode.mLetterSpacing != Float.NaN) {
ops.add(new SetSpanOperation(
start,
end,
new CustomLetterSpacingSpan(textShadowNode.mLetterSpacing)));
}
}
if (textShadowNode.mFontSize != UNSET) {
ops.add(new SetSpanOperation(start, end, new AbsoluteSizeSpan(textShadowNode.mFontSize)));
}
Expand Down Expand Up @@ -228,6 +236,7 @@ private static int parseNumericFontWeight(String fontWeightString) {
}

protected float mLineHeight = Float.NaN;
protected float mLetterSpacing = Float.NaN;
protected boolean mIsColorSet = false;
protected boolean mAllowFontScaling = true;
protected int mColor;
Expand All @@ -238,6 +247,7 @@ private static int parseNumericFontWeight(String fontWeightString) {
protected int mFontSize = UNSET;
protected float mFontSizeInput = UNSET;
protected float mLineHeightInput = UNSET;
protected float mLetterSpacingInput = Float.NaN;
protected int mTextAlign = Gravity.NO_GRAVITY;
protected int mTextBreakStrategy =
(Build.VERSION.SDK_INT < Build.VERSION_CODES.M) ? 0 : Layout.BREAK_STRATEGY_HIGH_QUALITY;
Expand Down Expand Up @@ -356,12 +366,22 @@ public void setLineHeight(float lineHeight) {
markUpdated();
}

@ReactProp(name = ViewProps.LETTER_SPACING, defaultFloat = Float.NaN)
public void setLetterSpacing(float letterSpacing) {
mLetterSpacingInput = letterSpacing;
mLetterSpacing = mAllowFontScaling
? PixelUtil.toPixelFromSP(mLetterSpacingInput)
: PixelUtil.toPixelFromDIP(mLetterSpacingInput);
markUpdated();
}

@ReactProp(name = ViewProps.ALLOW_FONT_SCALING, defaultBoolean = true)
public void setAllowFontScaling(boolean allowFontScaling) {
if (allowFontScaling != mAllowFontScaling) {
mAllowFontScaling = allowFontScaling;
setFontSize(mFontSizeInput);
setLineHeight(mLineHeightInput);
setLetterSpacing(mLetterSpacingInput);
markUpdated();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import android.widget.EditText;
import com.facebook.infer.annotation.Assertions;
import com.facebook.react.bridge.ReactContext;
import com.facebook.react.uimanager.PixelUtil;
import com.facebook.react.uimanager.UIManagerModule;
import com.facebook.react.views.text.CustomStyleSpan;
import com.facebook.react.views.text.ReactTagSpan;
Expand Down Expand Up @@ -80,6 +81,7 @@ public class ReactEditText extends EditText {
private @Nullable ScrollWatcher mScrollWatcher;
private final InternalKeyListener mKeyListener;
private boolean mDetectScrollMovement = false;
private float mLetterSpacingPt = 0;

private ReactViewBackgroundManager mReactBackgroundManager;

Expand Down Expand Up @@ -627,6 +629,29 @@ public void setBorderStyle(@Nullable String style) {
mReactBackgroundManager.setBorderStyle(style);
}

public void setLetterSpacingPt(float letterSpacingPt) {
mLetterSpacingPt = letterSpacingPt;
updateLetterSpacing();
}

@Override
public void setTextSize (float size) {
super.setTextSize(size);
updateLetterSpacing();
}

@Override
public void setTextSize (int unit, float size) {
super.setTextSize(unit, size);
updateLetterSpacing();
}

protected void updateLetterSpacing() {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
setLetterSpacing(PixelUtil.toPixelFromSP(mLetterSpacingPt) / getTextSize());
}
}

/**
* This class will redirect *TextChanged calls to the listeners only in the case where the text
* is changed by the user, and not explicitly set by JS.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,14 @@ public void setOnScroll(final ReactEditText view, boolean onScroll) {
}
}

// Sets the letter spacing as an absolute point size.
// This extra handling, on top of what ReactBaseTextShadowNode already does, is required for the
// correct display of spacing in placeholder (hint) text.
@ReactProp(name = ViewProps.LETTER_SPACING, defaultFloat = 0)
public void setLetterSpacing(ReactEditText view, float letterSpacing) {
view.setLetterSpacingPt(letterSpacing);
}

@ReactProp(name = "placeholder")
public void setPlaceholder(ReactEditText view, @Nullable String placeholder) {
view.setHint(placeholder);
Expand Down

0 comments on commit 5898817

Please sign in to comment.