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
mdcollab · Feb 27, 2018 · 5898817 · 5898817
1 parent dbafc29
commit 5898817
Show file tree

Hide file tree

Showing 8 changed files with 176 additions and 0 deletions.
diff --git a/RNTester/js/TextExample.android.js b/RNTester/js/TextExample.android.js
@@ -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>

diff --git a/RNTester/js/TextExample.ios.js b/RNTester/js/TextExample.ios.js
@@ -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>
       );
     },

diff --git a/RNTester/js/TextInputExample.android.js b/RNTester/js/TextInputExample.android.js
@@ -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() {

diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewProps.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewProps.java
@@ -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";

diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/text/CustomLetterSpacingSpan.java b/ReactAndroid/src/main/java/com/facebook/react/views/text/CustomLetterSpacingSpan.java
@@ -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());
+    }
+  }
+}
diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java
@@ -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)));
       }
@@ -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;
@@ -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;
@@ -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();
     }
   }

diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java
@@ -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;
@@ -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;
 
@@ -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.

diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java
@@ -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);