Add `letterSpacing` style property for `Text` by vkurchatkin · Pull Request #482 · react/react-native

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

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

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(<Viewstyle={{flex: 1,backgroundColor: 'white'}}><Viewstyle={{flex: 1,justifyContent: 'flex-end',alignItems: 'flex-start'}}><Textstyle={{fontFamily: 'Copperplate',letterSpacing: 5,fontSize: 10}}>
This is <Textstyle={{letterSpacing: 0}}>a text with a lot</Text> of spaces
</Text></View><Viewstyle={{flex: 1,backgroundColor: 'blue'}}><WebViewurl="http://localhost:8082/"renderLoading={()=><View/>}renderError={()=><View/>}/></View></View>);}

Html:

<!DOCTYPE html><htmllang="en"><head><metacharset="UTF-8"><title></title></head><bodystyle="padding: 0; margin: 0"><spanstyle="font-family: Copperplate; letter-spacing: 5px; font-size: 10px;">
This is <spanstyle="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

nit: can you add a trailing comma

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

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

Fixesreact#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: Fixesreact#457Closesreact#482 Github Author: Vladimir Kurchatkin <vladimir.kurchatkin@gmail.com> Test Plan: Imported from GitHub, without a `Test Plan:` line.

…eact#482)

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 d1451feCompare March 30, 2015 21:54

vjeux reviewed Apr 1, 2015
View reviewed changes

Comment threadLibraries/Text/TextStylePropTypes.js Outdated
vjeuxApr 1, 2015
Copy link
Copy Markdown
Contributor
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: can you add a trailing comma

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

Add letterSpacing style property for Text
f3f0828
Fixesreact#457

vkurchatkin force-pushed the text-letter-spacing branch from 869bdb7 to f3f0828Compare 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 a142ed5May 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 (r…
11621a3
…eact#482)

Uh oh!

Conversation

vkurchatkin commented Mar 30, 2015

Uh oh!

frantic commented Mar 30, 2015

Uh oh!

brentvatne commented Mar 30, 2015

Uh oh!

a2Mar 30, 2015

Choose a reason for hiding this comment

Uh oh!

vkurchatkin commented Mar 30, 2015

Uh oh!

vjeux commented Mar 30, 2015

Uh oh!

vjeux commented Mar 30, 2015

Uh oh!

vkurchatkin commented Mar 30, 2015

Uh oh!

vjeux commented Mar 30, 2015

Uh oh!

vkurchatkin commented Mar 30, 2015

Uh oh!

vkurchatkin commented Mar 31, 2015

Uh oh!

vkurchatkin commented Mar 31, 2015

Uh oh!

vjeuxApr 1, 2015

Choose a reason for hiding this comment

Uh oh!

brentvatne commented Apr 1, 2015

Uh oh!

vkurchatkin commented Apr 1, 2015

Uh oh!

chirag04 commented Apr 2, 2015

Uh oh!

vkurchatkin commented Apr 9, 2015

Uh oh!

nicklockwood commented Apr 9, 2015

Uh oh!

ide commented May 8, 2015

Uh oh!

brentvatne commented May 11, 2015

Uh oh!

nicklockwood commented May 12, 2015

Uh oh!

brentvatne commented May 12, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants