Add `letterSpacing` style property for `Text` #482

vkurchatkin · 2015-03-30T09:08:25Z

Fixes #457

frantic · 2015-03-30T20:17:50Z

Restarted the Travis build. @a2 / @nicklockwood - what do you think?

brentvatne · 2015-03-30T21:16:08Z

👍 - I'd love to see this get merged

a2 · 2015-03-30T21:29:43Z

Libraries/Text/RCTShadowText.m

@@ -105,6 +111,8 @@ - (NSAttributedString *)_attributedStringWithFontFamily:(NSString *)fontFamily
  [self _addAttribute:RCTReactTagAttributeName withValue:self.reactTag toAttributedString:attributedString];
  [self _setParagraphStyleOnAttributedString:attributedString];

+  [self _addAttribute:NSKernAttributeName withValue:@(letterSpacing) toAttributedString:attributedString];


This is a tiny nit, but I would move this to after the line that adds the NSFontAttributeName attribute to maintain consistency but that's really the only thing I have against this. Looks good otherwise! 👍

vkurchatkin · 2015-03-30T21:55:46Z

@a2 thank you, fixed the nit

vjeux · 2015-03-30T22:24:11Z

Hey, can you make sure it actually follows the same behavior as CSS.

Example, can you run this in React Native, take a screenshot and make sure it looks pixel perfect:
http://jsfiddle.net/y7h9j3uw/

If you want to try a different font: http://iosfonts.com/

vjeux · 2015-03-30T22:24:59Z

It is super important that any feature which has the same name as CSS does actually work exactly the same as CSS, otherwise it makes debugging a lot hard and confuses developers.

vkurchatkin · 2015-03-30T22:57:53Z

@vjeux here is what I get:

The code:

return (
      <View style={{ flex: 1, backgroundColor: 'white' }}>
        <View style={{ flex: 1, justifyContent: 'flex-end', alignItems: 'flex-start' }}>
            <Text style={{ fontFamily: 'Copperplate', letterSpacing: 5, fontSize: 10 }}>
              This is <Text style={{ letterSpacing: 0 }}>a text with a lot</Text> of spaces
            </Text>
        </View>

        <View style={{ flex: 1, backgroundColor: 'blue' }}>
          <WebView url="http://localhost:8082/" renderLoading={() => <View/>} renderError={() => <View/>}/>
        </View>

      </View>
    );
  }

Html:

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <title></title>
</head>
<body style="padding: 0; margin: 0">
  <span style="font-family: Copperplate; letter-spacing: 5px; font-size: 10px;">
      This is <span style="letter-spacing:0">a text with a lot</span> of spaces
  </span>
</body>
</html>

vjeux · 2015-03-30T23:17:52Z

Can you also check what's the behavior for negative spacing? Thanks a lot for making sure it's the same

vkurchatkin · 2015-03-30T23:32:03Z

For -3px I get the following:

Pretty close

vkurchatkin · 2015-03-31T12:42:38Z

There is the last nasty problem: width calculation. One the web letter spacing affects width, both positive and negative. Negative letter spacing makes width less than required to display text, but it is not clipped according to this width (even with all kinds of overflow and text-overflow properties). On iOS, on the contrary, it is clipped.

vkurchatkin · 2015-03-31T12:56:01Z

Pushed a fix for this problem, now behaves exactly the same way

vjeux · 2015-04-01T02:16:20Z

Libraries/Text/TextStylePropTypes.js

@@ -32,6 +32,7 @@ var TextStylePropTypes = Object.assign(Object.create(ViewStylePropTypes), {
  writingDirection: ReactPropTypes.oneOf(
    ['auto' /*default*/, 'ltr', 'rtl']
  ),
+  letterSpacing: ReactPropTypes.number


nit: can you add a trailing comma

brentvatne · 2015-04-01T03:16:33Z

Great work @vkurchatkin - looking forward to putting this to use!

Fixes facebook#457

vkurchatkin · 2015-04-01T17:50:51Z

@vjeux all done

chirag04 · 2015-04-02T18:40:08Z

Awesome work @vkurchatkin - I badly want to see this getting merged.

vkurchatkin · 2015-04-09T12:45:50Z

@vjeux ping

nicklockwood · 2015-04-09T12:47:47Z

This looks good to me. Not sure what's holding it up. I'll check.

ide · 2015-05-08T00:41:30Z

+1

brentvatne · 2015-05-11T20:28:01Z

@nicklockwood any update?

nicklockwood · 2015-05-12T01:45:29Z

Sorry about the radio silence. There were a few issues merging - notably this broke the UIExplorer snapshot tests because the default letterSpacing value of 0 is not pixel-for-pixel identical to having no letterSpacing set at all (it disables iOS's per-character smart kerning).

Anyway, that's fixed now, and I've merged this internally so it should be appearing on GitHub real soon.

brentvatne · 2015-05-12T02:12:16Z

🌴 that's great, thanks!

Summary: Fixes facebook#457 Closes facebook#482 Github Author: Vladimir Kurchatkin <vladimir.kurchatkin@gmail.com> Test Plan: Imported from GitHub, without a `Test Plan:` line.

…acebook#482)

* View Properties accessibilityRole/States Added documentation for accessibilityRole and accessibilityStates on View * Update view.md * Update view.md

a2 reviewed Mar 30, 2015
View reviewed changes

frantic mentioned this pull request Mar 30, 2015

Letter spacing CSS property #457

Closed

vkurchatkin force-pushed the text-letter-spacing branch 2 times, most recently from 06e01e6 to d1451fe Compare March 30, 2015 21:54

vjeux reviewed Apr 1, 2015
View reviewed changes

vjeux added the GH review: accepted label Apr 1, 2015

Add letterSpacing style property for Text

f3f0828

Fixes facebook#457

vkurchatkin force-pushed the text-letter-spacing branch from 869bdb7 to f3f0828 Compare April 1, 2015 17:50

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 7, 2015

vkurchatkin mentioned this pull request Apr 9, 2015

how to put line through text ? #753

Closed

vjeux added Needs Review and removed Needs Review labels Apr 17, 2015

vjeux closed this in a142ed5 May 13, 2015

ide mentioned this pull request May 15, 2015

[Text] Fix <Text> letterSpacing right side padding #1295

Closed

jfrolich pushed a commit to jfrolich/react-native that referenced this pull request Apr 22, 2020

Replace unnecessary names with empty strings in external statements (f…

11621a3

…acebook#482)

ayushjainrksh referenced this pull request in MLH-Fellowship/react-native Jul 2, 2020

View Properties accessibilityRole/States (#482)

22270d5

* View Properties accessibilityRole/States Added documentation for accessibilityRole and accessibilityStates on View * Update view.md * Update view.md

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `letterSpacing` style property for `Text` #482

Add `letterSpacing` style property for `Text` #482

vkurchatkin commented Mar 30, 2015

frantic commented Mar 30, 2015

brentvatne commented Mar 30, 2015

a2 Mar 30, 2015

vkurchatkin commented Mar 30, 2015

vjeux commented Mar 30, 2015

vjeux commented Mar 30, 2015

vkurchatkin commented Mar 30, 2015

vjeux commented Mar 30, 2015

vkurchatkin commented Mar 30, 2015

vkurchatkin commented Mar 31, 2015

vkurchatkin commented Mar 31, 2015

vjeux Apr 1, 2015

brentvatne commented Apr 1, 2015

vkurchatkin commented Apr 1, 2015

chirag04 commented Apr 2, 2015

vkurchatkin commented Apr 9, 2015

nicklockwood commented Apr 9, 2015

ide commented May 8, 2015

brentvatne commented May 11, 2015

nicklockwood commented May 12, 2015

brentvatne commented May 12, 2015

Add letterSpacing style property for Text #482

Add letterSpacing style property for Text #482

Conversation

vkurchatkin commented Mar 30, 2015

frantic commented Mar 30, 2015

brentvatne commented Mar 30, 2015

a2 Mar 30, 2015

Choose a reason for hiding this comment

vkurchatkin commented Mar 30, 2015

vjeux commented Mar 30, 2015

vjeux commented Mar 30, 2015

vkurchatkin commented Mar 30, 2015

vjeux commented Mar 30, 2015

vkurchatkin commented Mar 30, 2015

vkurchatkin commented Mar 31, 2015

vkurchatkin commented Mar 31, 2015

vjeux Apr 1, 2015

Choose a reason for hiding this comment

brentvatne commented Apr 1, 2015

vkurchatkin commented Apr 1, 2015

chirag04 commented Apr 2, 2015

vkurchatkin commented Apr 9, 2015

nicklockwood commented Apr 9, 2015

ide commented May 8, 2015

brentvatne commented May 11, 2015

nicklockwood commented May 12, 2015

brentvatne commented May 12, 2015

Add `letterSpacing` style property for `Text` #482

Add `letterSpacing` style property for `Text` #482