Implement letterSpacing on Android >= 5.0 #17398

motiz88 · 2017-12-31T02:11:15Z

Motivation

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 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, 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 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.

Test Plan

Note comment re: unit tests above; RNTester screenshots follow.

RNTester - `<Text>`

iOS (existing functionality, amended test)	Android (new functionality & test)

RNTester - `<TextInput placeholder>`

iOS (not implemented, test not in this branch)	Android (new functionality & test)

RNTester - `<TextInput value>`

iOS (not implemented, test not in this branch)	Android (new functionality & test)

Related PRs

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.

Release Notes

[ANDROID] [FEATURE] [Text] - Implemented letterSpacing

janicduplessis · 2018-01-20T06:55:11Z

Thanks for the PR, going to try it in my RN fork to see if it works properly in some apps.

@hramos Could you have someone review this, not sure who to ping for android these days :S

janicduplessis · 2018-01-20T07:37:27Z

@motiz88 Tested this and there is one major issue. Text isn't measured properly when using letterSpacing. It can be made very noticeable by adding a background color.

Letter spacing 2.0:

No letter spacing:

motiz88 · 2018-01-20T08:16:39Z

@janicduplessis Ooh, that's going to be interesting to track down. On it, will report back soon.

motiz88 · 2018-01-20T09:06:33Z

Can you share some code to reproduce the issue you're seeing, @janicduplessis? Trying this in RNTester (see code in 19df006) seems to render correctly.

iOS	Android

Note: The bottom row isn't wrapped with flexDirection: row so gets full width. As I understand, this is per the standard RN layout algorithm for text.

janicduplessis · 2018-01-20T09:25:19Z

@motiz88 Oh, looks like it only happens when setting fontSize + letterSpacing.

This repros it for me:

      <View style={{ flex: 1, alignItems: 'flex-start' }}>
        <Text
          style={{
            backgroundColor: 'red',
            letterSpacing: 2,
            fontSize: 12,
          }}
        >
          TEST TEST TEST
        </Text>
      </View>

motiz88 · 2018-01-20T10:03:35Z

Fixed in 6408a25. Apparently, spans end up being evaluated in reverse order to the one in which they're added to the ops list in buildSpannedFromShadowNode - probably due to this line increasing each subsequent one's priority, if I'm reading the code correctly. This in turn matters to CustomLetterSpacingSpan, as it must read the current font size from the TextPaint instance to perform an appropriate unit conversion.

Quite an omission in my testing approach 😨 especially as I was very much aware of the dependency between fontSize and letterSpacing. But looks good now!

iOS	Android

janicduplessis · 2018-01-20T21:04:28Z

@motiz88 Nice just tested again and everything seems fine! Will try to have someone from fb have a look.

hramos · 2018-01-29T21:12:44Z

Libraries/Text/TextStylePropTypes.js

@@ -49,7 +49,16 @@ const TextStylePropTypes = {
  textShadowRadius: ReactPropTypes.number,
  textShadowColor: ColorPropType,
  /**
-   * @platform ios
+   * Increase or decrease the spacing between characters. The default value is 0, for no extra


Can you add these docs to the docs at https://github.com/facebook/react-native-website instead? We'll be slowly pruning out these comment blocks over the next few months.

(It's fine to wait for this PR to be merged first)

hramos · 2018-01-29T21:14:37Z

Importing for internal review.

facebook-github-bot

@hramos has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

hramos

We'll need the PR to be rebased before we can cleanly import it. Thanks.

hramos

Needs rebase

hramos · 2018-02-03T00:00:16Z

Sorry about the conflicting statements above - this still needs to be reviewed internally, but we'll need this to be rebased first in order to be able to import the PR cleanly.

motiz88 · 2018-02-03T13:44:07Z

@hramos I've rebased this on top of master as requested. UPDATE: Also reverted the commit which added the doc comments. There already is a docs PR waiting on this one: facebook/react-native-website#105

hramos · 2018-02-27T21:20:06Z

The PR has already been imported, which is more than can be said for most PRs. It's now waiting for review from an employee. We'll sync back here if any changes are required.

janicduplessis · 2018-02-27T23:19:48Z

Thanks @motiz88 @hramos @mdvacca !! 🎉

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 facebook/react-native@6114f86, that resolves the [issues](facebook/react-native#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/react-native#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/react-native#17398 Reviewed By: mdvacca Differential Revision: D6837718 Pulled By: hramos fbshipit-source-id: 5c9d49e9cf4af6457b636416ce5fe15315aab72c

Summary: `<Image resizeMode="repeat" />` for Android, matching the iOS implementation (#7968). (Non-goal: changing the component's API for finer-grained control / feature parity with CSS - this would be nice in the future) As requested in e.g. #14158. Given facebook/fresco#1575, and lacking the context to follow the specific recommendations in facebook/fresco#1575 (comment), I've opted for a minimal change within RN itself. It's likely that performance can be improved by offloading this work to Fresco in some clever way; but I'm assuming that the present naive approach is still an improvement over a userland implementation with `onLayout` and multiple `<Image>` instances. - Picking up on a TODO note in the existing code, I implemented `MultiPostprocessor` to allow arbitrary chaining of Fresco-compatible postprocessors inside `ReactImageView`. - Rather than extensively refactor `ImageResizeMode`, `ReactImageManager` and `ReactImageView`, I mostly preserved the existing API that maps `resizeMode` values to [`ScaleType`](http://frescolib.org/javadoc/reference/com/facebook/drawee/drawable/ScalingUtils.ScaleType.html) instances, and simply added a second mapping, to [`TileMode`](https://developer.android.com/reference/android/graphics/Shader.TileMode.html). - To match the iOS rendering exactly for oversized images, I found that scaling with a custom `ScaleType` was required - a kind of combination of `CENTER_INSIDE` and `FIT_START` which Fresco doesn't provide - so I implemented that as `ScaleTypeStartInside`. (This is, frankly, questionable as the default behaviour on iOS to begin with - but I am aiming for parity here) - `resizeMode="repeat"` is therefore unpacked by the view manager to the effect of: ```js view.setScaleType(ScaleTypeStartInside.INSTANCE); view.setTileMode(Shader.TileMode.REPEAT); ``` And the added postprocessing in the view (in case of a non-`CLAMP` tile mode) consists of waiting for layout, allocating a destination bitmap and painting the source bitmap with the requested tile mode and scale type. Note that as in #17398 (comment), 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. Also, I'm happy to address any code style issues or other feedback; I'm new to this codebase and a very infrequent Android/Java coder. Tested by enabling the relevant case in RNTester on Android. | iOS | Android | |-|-| | <img src=https://user-images.githubusercontent.com/2246565/34461897-4e12008e-ee2f-11e7-8581-1dc0cc8f2779.png width=300>| <img src=https://user-images.githubusercontent.com/2246565/34461894-40b2c8ec-ee2f-11e7-8a8f-96704f3c8caa.png width=300> | Docs update: facebook/react-native-website#106 [ANDROID] [FEATURE] [Image] - Implement resizeMode=repeat Closes #17404 Reviewed By: achen1 Differential Revision: D7070329 Pulled By: mdvacca fbshipit-source-id: 6a72fcbdcc7c7c2daf293dc1d8b6728f54ad0249

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

Summary: `<Image resizeMode="repeat" />` for Android, matching the iOS implementation (facebook#7968). (Non-goal: changing the component's API for finer-grained control / feature parity with CSS - this would be nice in the future) As requested in e.g. facebook#14158. Given facebook/fresco#1575, and lacking the context to follow the specific recommendations in facebook/fresco#1575 (comment), I've opted for a minimal change within RN itself. It's likely that performance can be improved by offloading this work to Fresco in some clever way; but I'm assuming that the present naive approach is still an improvement over a userland implementation with `onLayout` and multiple `<Image>` instances. - Picking up on a TODO note in the existing code, I implemented `MultiPostprocessor` to allow arbitrary chaining of Fresco-compatible postprocessors inside `ReactImageView`. - Rather than extensively refactor `ImageResizeMode`, `ReactImageManager` and `ReactImageView`, I mostly preserved the existing API that maps `resizeMode` values to [`ScaleType`](http://frescolib.org/javadoc/reference/com/facebook/drawee/drawable/ScalingUtils.ScaleType.html) instances, and simply added a second mapping, to [`TileMode`](https://developer.android.com/reference/android/graphics/Shader.TileMode.html). - To match the iOS rendering exactly for oversized images, I found that scaling with a custom `ScaleType` was required - a kind of combination of `CENTER_INSIDE` and `FIT_START` which Fresco doesn't provide - so I implemented that as `ScaleTypeStartInside`. (This is, frankly, questionable as the default behaviour on iOS to begin with - but I am aiming for parity here) - `resizeMode="repeat"` is therefore unpacked by the view manager to the effect of: ```js view.setScaleType(ScaleTypeStartInside.INSTANCE); view.setTileMode(Shader.TileMode.REPEAT); ``` And the added postprocessing in the view (in case of a non-`CLAMP` tile mode) consists of waiting for layout, allocating a destination bitmap and painting the source bitmap with the requested tile mode and scale type. Note that as in facebook#17398 (comment), 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. Also, I'm happy to address any code style issues or other feedback; I'm new to this codebase and a very infrequent Android/Java coder. Tested by enabling the relevant case in RNTester on Android. | iOS | Android | |-|-| | <img src=https://user-images.githubusercontent.com/2246565/34461897-4e12008e-ee2f-11e7-8581-1dc0cc8f2779.png width=300>| <img src=https://user-images.githubusercontent.com/2246565/34461894-40b2c8ec-ee2f-11e7-8a8f-96704f3c8caa.png width=300> | Docs update: facebook/react-native-website#106 [ANDROID] [FEATURE] [Image] - Implement resizeMode=repeat Closes facebook#17404 Reviewed By: achen1 Differential Revision: D7070329 Pulled By: mdvacca fbshipit-source-id: 6a72fcbdcc7c7c2daf293dc1d8b6728f54ad0249

LouisJS · 2018-05-15T08:29:14Z

Any news on this ?

motiz88 · 2018-05-15T08:50:09Z

It's in 0.55: https://github.com/react-native-community/react-native-releases/blob/master/CHANGELOG.md#055

…

On Tue, 15 May 2018, 09:31 Louis Lecocq, ***@***.***> wrote: Any news on this ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#17398 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACJHpU7TcPcMrsNXFyp85FxIWaKUU6T9ks5typJQgaJpZM4RPv26> .

Summary: `<Image resizeMode="repeat" />` for Android, matching the iOS implementation (facebook#7968). (Non-goal: changing the component's API for finer-grained control / feature parity with CSS - this would be nice in the future) As requested in e.g. facebook#14158. Given facebook/fresco#1575, and lacking the context to follow the specific recommendations in facebook/fresco#1575 (comment), I've opted for a minimal change within RN itself. It's likely that performance can be improved by offloading this work to Fresco in some clever way; but I'm assuming that the present naive approach is still an improvement over a userland implementation with `onLayout` and multiple `<Image>` instances. - Picking up on a TODO note in the existing code, I implemented `MultiPostprocessor` to allow arbitrary chaining of Fresco-compatible postprocessors inside `ReactImageView`. - Rather than extensively refactor `ImageResizeMode`, `ReactImageManager` and `ReactImageView`, I mostly preserved the existing API that maps `resizeMode` values to [`ScaleType`](http://frescolib.org/javadoc/reference/com/facebook/drawee/drawable/ScalingUtils.ScaleType.html) instances, and simply added a second mapping, to [`TileMode`](https://developer.android.com/reference/android/graphics/Shader.TileMode.html). - To match the iOS rendering exactly for oversized images, I found that scaling with a custom `ScaleType` was required - a kind of combination of `CENTER_INSIDE` and `FIT_START` which Fresco doesn't provide - so I implemented that as `ScaleTypeStartInside`. (This is, frankly, questionable as the default behaviour on iOS to begin with - but I am aiming for parity here) - `resizeMode="repeat"` is therefore unpacked by the view manager to the effect of: ```js view.setScaleType(ScaleTypeStartInside.INSTANCE); view.setTileMode(Shader.TileMode.REPEAT); ``` And the added postprocessing in the view (in case of a non-`CLAMP` tile mode) consists of waiting for layout, allocating a destination bitmap and painting the source bitmap with the requested tile mode and scale type. Note that as in facebook#17398 (comment), 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. Also, I'm happy to address any code style issues or other feedback; I'm new to this codebase and a very infrequent Android/Java coder. Tested by enabling the relevant case in RNTester on Android. | iOS | Android | |-|-| | <img src=https://user-images.githubusercontent.com/2246565/34461897-4e12008e-ee2f-11e7-8581-1dc0cc8f2779.png width=300>| <img src=https://user-images.githubusercontent.com/2246565/34461894-40b2c8ec-ee2f-11e7-8a8f-96704f3c8caa.png width=300> | Docs update: facebook/react-native-website#106 [ANDROID] [FEATURE] [Image] - Implement resizeMode=repeat Closes facebook#17404 Reviewed By: achen1 Differential Revision: D7070329 Pulled By: mdvacca fbshipit-source-id: 6a72fcbdcc7c7c2daf293dc1d8b6728f54ad0249 (cherry picked from commit 0459e4f)

Summary: `<Image resizeMode="repeat" />` for Android, matching the iOS implementation (facebook#7968). (Non-goal: changing the component's API for finer-grained control / feature parity with CSS - this would be nice in the future) As requested in e.g. facebook#14158. Given facebook/fresco#1575, and lacking the context to follow the specific recommendations in facebook/fresco#1575 (comment), I've opted for a minimal change within RN itself. It's likely that performance can be improved by offloading this work to Fresco in some clever way; but I'm assuming that the present naive approach is still an improvement over a userland implementation with `onLayout` and multiple `<Image>` instances. - Picking up on a TODO note in the existing code, I implemented `MultiPostprocessor` to allow arbitrary chaining of Fresco-compatible postprocessors inside `ReactImageView`. - Rather than extensively refactor `ImageResizeMode`, `ReactImageManager` and `ReactImageView`, I mostly preserved the existing API that maps `resizeMode` values to [`ScaleType`](http://frescolib.org/javadoc/reference/com/facebook/drawee/drawable/ScalingUtils.ScaleType.html) instances, and simply added a second mapping, to [`TileMode`](https://developer.android.com/reference/android/graphics/Shader.TileMode.html). - To match the iOS rendering exactly for oversized images, I found that scaling with a custom `ScaleType` was required - a kind of combination of `CENTER_INSIDE` and `FIT_START` which Fresco doesn't provide - so I implemented that as `ScaleTypeStartInside`. (This is, frankly, questionable as the default behaviour on iOS to begin with - but I am aiming for parity here) - `resizeMode="repeat"` is therefore unpacked by the view manager to the effect of: ```js view.setScaleType(ScaleTypeStartInside.INSTANCE); view.setTileMode(Shader.TileMode.REPEAT); ``` And the added postprocessing in the view (in case of a non-`CLAMP` tile mode) consists of waiting for layout, allocating a destination bitmap and painting the source bitmap with the requested tile mode and scale type. Note that as in facebook#17398 (comment), 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. Also, I'm happy to address any code style issues or other feedback; I'm new to this codebase and a very infrequent Android/Java coder. Tested by enabling the relevant case in RNTester on Android. | iOS | Android | |-|-| | <img src=https://user-images.githubusercontent.com/2246565/34461897-4e12008e-ee2f-11e7-8581-1dc0cc8f2779.png width=300>| <img src=https://user-images.githubusercontent.com/2246565/34461894-40b2c8ec-ee2f-11e7-8a8f-96704f3c8caa.png width=300> | Docs update: facebook/react-native-website#106 [ANDROID] [FEATURE] [Image] - Implement resizeMode=repeat Closes facebook#17404 Reviewed By: achen1 Differential Revision: D7070329 Pulled By: mdvacca fbshipit-source-id: 6a72fcbdcc7c7c2daf293dc1d8b6728f54ad0249

Because NaN is special, the `!=` version of this condition will always be true -- even if `mLetterSpacing` is also `Float.NaN`, which is its default value. It should instead be `!Float.isNaN(...)`. The effect of the broken condition is that every text shadow node will create a `CustomLetterSpacingSpan` around its contents, even when the `letterSpacing` prop was never set. Empirically, that in turn causes facebook#19126: in a controlled TextInput after some text is first added, then deleted, further interaction with the TextInput becomes extremely slow. Perhaps we're somehow ending up with large numbers of these shadow nodes (that sounds like a bug in itself, but I guess it's normally low-impact); then this code would have caused them each to generate more work to do. In any case, fixing this condition causes that issue to disappear. The affected logic was introduced between v0.54 and v0.55, in facebook#17398 aka 5898817 "Implement letterSpacing on Android >= 5.0". Fixes facebook#19126.

Summary: `<Image resizeMode="repeat" />` for Android, matching the iOS implementation (facebook#7968). (Non-goal: changing the component's API for finer-grained control / feature parity with CSS - this would be nice in the future) As requested in e.g. facebook#14158. Given facebook/fresco#1575, and lacking the context to follow the specific recommendations in facebook/fresco#1575 (comment), I've opted for a minimal change within RN itself. It's likely that performance can be improved by offloading this work to Fresco in some clever way; but I'm assuming that the present naive approach is still an improvement over a userland implementation with `onLayout` and multiple `<Image>` instances. - Picking up on a TODO note in the existing code, I implemented `MultiPostprocessor` to allow arbitrary chaining of Fresco-compatible postprocessors inside `ReactImageView`. - Rather than extensively refactor `ImageResizeMode`, `ReactImageManager` and `ReactImageView`, I mostly preserved the existing API that maps `resizeMode` values to [`ScaleType`](http://frescolib.org/javadoc/reference/com/facebook/drawee/drawable/ScalingUtils.ScaleType.html) instances, and simply added a second mapping, to [`TileMode`](https://developer.android.com/reference/android/graphics/Shader.TileMode.html). - To match the iOS rendering exactly for oversized images, I found that scaling with a custom `ScaleType` was required - a kind of combination of `CENTER_INSIDE` and `FIT_START` which Fresco doesn't provide - so I implemented that as `ScaleTypeStartInside`. (This is, frankly, questionable as the default behaviour on iOS to begin with - but I am aiming for parity here) - `resizeMode="repeat"` is therefore unpacked by the view manager to the effect of: ```js view.setScaleType(ScaleTypeStartInside.INSTANCE); view.setTileMode(Shader.TileMode.REPEAT); ``` And the added postprocessing in the view (in case of a non-`CLAMP` tile mode) consists of waiting for layout, allocating a destination bitmap and painting the source bitmap with the requested tile mode and scale type. Note that as in facebook#17398 (comment), 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. Also, I'm happy to address any code style issues or other feedback; I'm new to this codebase and a very infrequent Android/Java coder. Tested by enabling the relevant case in RNTester on Android. | iOS | Android | |-|-| | <img src=https://user-images.githubusercontent.com/2246565/34461897-4e12008e-ee2f-11e7-8581-1dc0cc8f2779.png width=300>| <img src=https://user-images.githubusercontent.com/2246565/34461894-40b2c8ec-ee2f-11e7-8a8f-96704f3c8caa.png width=300> | Docs update: facebook/react-native-website#106 [ANDROID] [FEATURE] [Image] - Implement resizeMode=repeat Closes facebook#17404 Reviewed By: achen1 Differential Revision: D7070329 Pulled By: mdvacca fbshipit-source-id: 6a72fcbdcc7c7c2daf293dc1d8b6728f54ad0249

Because NaN is special, the `!=` version of this condition will always be true -- even if `mLetterSpacing` is also `Float.NaN`, which is its default value. It should instead be `!Float.isNaN(...)`. The effect of the broken condition is that every text shadow node will create a `CustomLetterSpacingSpan` around its contents, even when the `letterSpacing` prop was never set. Empirically, that in turn causes facebook#19126: in a controlled TextInput after some text is first added, then deleted, further interaction with the TextInput becomes extremely slow. Perhaps we're somehow ending up with large numbers of these shadow nodes (that sounds like a bug in itself, but I guess it's normally low-impact); then this code would have caused them each to generate more work to do. In any case, fixing this condition causes that issue to disappear. The affected logic was introduced between v0.54 and v0.55, in facebook#17398 aka 5898817 "Implement letterSpacing on Android >= 5.0". Fixes facebook#19126.

Summary: `<Image resizeMode="repeat" />` for Android, matching the iOS implementation (#7968). (Non-goal: changing the component's API for finer-grained control / feature parity with CSS - this would be nice in the future) As requested in e.g. #14158. Given facebook/fresco#1575, and lacking the context to follow the specific recommendations in facebook/fresco#1575 (comment), I've opted for a minimal change within RN itself. It's likely that performance can be improved by offloading this work to Fresco in some clever way; but I'm assuming that the present naive approach is still an improvement over a userland implementation with `onLayout` and multiple `<Image>` instances. - Picking up on a TODO note in the existing code, I implemented `MultiPostprocessor` to allow arbitrary chaining of Fresco-compatible postprocessors inside `ReactImageView`. - Rather than extensively refactor `ImageResizeMode`, `ReactImageManager` and `ReactImageView`, I mostly preserved the existing API that maps `resizeMode` values to [`ScaleType`](http://frescolib.org/javadoc/reference/com/facebook/drawee/drawable/ScalingUtils.ScaleType.html) instances, and simply added a second mapping, to [`TileMode`](https://developer.android.com/reference/android/graphics/Shader.TileMode.html). - To match the iOS rendering exactly for oversized images, I found that scaling with a custom `ScaleType` was required - a kind of combination of `CENTER_INSIDE` and `FIT_START` which Fresco doesn't provide - so I implemented that as `ScaleTypeStartInside`. (This is, frankly, questionable as the default behaviour on iOS to begin with - but I am aiming for parity here) - `resizeMode="repeat"` is therefore unpacked by the view manager to the effect of: ```js view.setScaleType(ScaleTypeStartInside.INSTANCE); view.setTileMode(Shader.TileMode.REPEAT); ``` And the added postprocessing in the view (in case of a non-`CLAMP` tile mode) consists of waiting for layout, allocating a destination bitmap and painting the source bitmap with the requested tile mode and scale type. Note that as in facebook/react-native#17398 (comment), I have neither updated nor tested the "Flat" UI implementation - everything compiles but I've taken [this comment](facebook/react-native#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. Also, I'm happy to address any code style issues or other feedback; I'm new to this codebase and a very infrequent Android/Java coder. Tested by enabling the relevant case in RNTester on Android. | iOS | Android | |-|-| | <img src=https://user-images.githubusercontent.com/2246565/34461897-4e12008e-ee2f-11e7-8581-1dc0cc8f2779.png width=300>| <img src=https://user-images.githubusercontent.com/2246565/34461894-40b2c8ec-ee2f-11e7-8a8f-96704f3c8caa.png width=300> | Docs update: facebook/react-native-website#106 [ANDROID] [FEATURE] [Image] - Implement resizeMode=repeat Closes facebook/react-native#17404 Reviewed By: achen1 Differential Revision: D7070329 Pulled By: mdvacca fbshipit-source-id: 6a72fcbdcc7c7c2daf293dc1d8b6728f54ad0249

Cherry pick from: facebook@5017b86 Because NaN is special, the `!=` version of this condition will always be true -- even if `mLetterSpacing` is also `Float.NaN`, which is its default value. It should instead be `!Float.isNaN(...)`. The effect of the broken condition is that every text shadow node will create a `CustomLetterSpacingSpan` around its contents, even when the `letterSpacing` prop was never set. Empirically, that in turn causes facebook#19126: in a controlled TextInput after some text is first added, then deleted, further interaction with the TextInput becomes extremely slow. Perhaps we're somehow ending up with large numbers of these shadow nodes (that sounds like a bug in itself, but I guess it's normally low-impact); then this code would have caused them each to generate more work to do. In any case, fixing this condition causes that issue to disappear. The affected logic was introduced between v0.54 and v0.55, in facebook#17398 aka 5898817 "Implement letterSpacing on Android >= 5.0". Fixes facebook#19126.

Summary: This sync includes the following changes: - **[19f6fe170](facebook/react@19f6fe170 )**: Revert "Revert "Dispatch commands to both UIManagers from both renderers (#17211)" (#17232)" (#17799) //<Eli White>// - **[6250462be](facebook/react@6250462be )**: Renamed "ReactDOM-fb" imports to "ReactDOM" in www shims (#17797) //<Brian Vaughn>// - **[59f21f1b2](facebook/react@59f21f1b2 )**: HostText needs to copy over from current if it is unchanged in persistent mode (#17538) //<Sebastian Markbåge>// - **[ccc6100d7](facebook/react@ccc6100d7 )**: Fix comments typos (#17550) //<Nick S. Plekhanov>// - **[1b9328cd9](facebook/react@1b9328cd9 )**: Null stateNode after unmount (#17666) //<Brian Vaughn>// - **[897976600](facebook/react@897976600 )**: [ESLint] Allow partial matches for custom Effect Hooks (#17663) //<Dan Abramov>// - **[72592310a](facebook/react@72592310a )**: Create packages/dom-event-testing-library (#17660) //<Nicolas Gallagher>// - **[a5e951d4c](facebook/react@a5e951d4c )**: [react-interactions] Event testing library improvements (#17614) //<Nicolas Gallagher>// - **[7dc974542](facebook/react@7dc974542 )**: [Flight] Chunks API (#17398) //<Sebastian Markbåge>// - **[9354dd275](facebook/react@9354dd275 )**: Make HostComponent inexact (#17412) //<Eli White>// - **[4c270375e](facebook/react@4c270375e )**: Favor fallthrough switch instead of case statements for work tags (#17648) //<Sebastian Markbåge>// - **[6fef7c47a](facebook/react@6fef7c47a )**: Add a regression test for switching from Fragment to a component (#17647) //<Dan Abramov>// - **[9fe103124](facebook/react@9fe103124 )**: [react-interactions] Rename Flare APIs to deprecated and remove from RN (#17644) //<Dominic Gannaway>// - **[4b0cdf29a](facebook/react@4b0cdf29a )**: Build FB RN targets only in experimental mode (#17641) //<Dan Abramov>// - **[7309c5f93](facebook/react@7309c5f93 )**: Use zero-fill right shift instead of Math.floor (#17616) //<伊撒尔>// - **[3c54df091](facebook/react@3c54df091 )**: Fix missing stacks in WWW warnings (#17638) //<Dan Abramov>// - **[b66e86d95](facebook/react@b66e86d95 )**: react-refresh@0.7.1 //<Dan Abramov>// - **[c2d1561c6](facebook/react@c2d1561c6 )**: [Fast Refresh] Support injecting runtime after renderer executes (#17633) //<Dan Abramov>// - **[f42431abe](facebook/react@f42431abe )**: Revert "Remove renderPhaseUpdates Map (#17484)" (#17623) //<Dan Abramov>// - **[0b5a26a48](facebook/react@0b5a26a48 )**: Rename toWarnDev -> toErrorDev, toLowPriorityWarnDev -> toWarnDev (#17605) //<Dan Abramov>// - **[0cf22a56a](facebook/react@0cf22a56a )**: Use console directly instead of warning() modules (#17599) //<Dan Abramov>// - **[b6c423daa](facebook/react@b6c423daa )**: Use matching test command for equivalence tests (#17604) //<Dan Abramov>// - **[8a347ed02](facebook/react@8a347ed02 )**: Remove renderPhaseUpdates Map (#17484) //<Sebastian Markbåge>// - **[be603f5a5](facebook/react@be603f5a5 )**: [react-events] Remove lastNativeEvent in favor of SystemFlags (#17585) //<Dominic Gannaway>// - **[b15bf3675](facebook/react@b15bf3675 )**: Add component stacks to (almost) all warnings (#17586) //<Dan Abramov>// - **[2afeebdcc](facebook/react@2afeebdcc )**: [react-interactions] Remove responder root event types + revert commit phase change (#17577) //<Dominic Gannaway>// - **[9ac42dd07](facebook/react@9ac42dd07 )**: Remove the condition argument from warning() (#17568) //<Laura buns>// - **[7bf40e1cf](facebook/react@7bf40e1cf )**: Initialize update queue object on mount (#17560) //<Andrew Clark>// - **[e039e690b](facebook/react@e039e690b )**: Revert Update Queue Refactor //<Andrew Clark>// - **[b617db3d9](facebook/react@b617db3d9 )**: Refactor Update Queues to Fix Rebasing Bug //<Andrew Clark>// - **[b43eec7ea](facebook/react@b43eec7ea )**: Replace `wrap-warning-with-env-check` with an eslint plugin (#17540) //<Laura buns>// - **[acfe4b21b](facebook/react@acfe4b21b )**: [react-interactions] Upgrade passive event listeners to active listeners (#17513) //<Dominic Gannaway>// - **[5064c7f6a](facebook/react@5064c7f6a )**: Revert Rerender Error Check (#17519) //<Sebastian Markbåge>// - **[6d105ad3f](facebook/react@6d105ad3f )**: [react-interactions] Move Flare event registration to commit phase (#17518) //<Dominic Gannaway>// - **[dc18b8b8d](facebook/react@dc18b8b8d )**: Don't group Idle/Offscreen work with other work (#17456) //<Sebastian Markbåge>// - **[f523b2e0d](facebook/react@f523b2e0d )**: Use fewer global variables in Hooks (#17480) //<Sebastian Markbåge>// - **[d75323f65](facebook/react@d75323f65 )**: Remove case that only exists for createBatch (#17506) //<Sebastian Markbåge>// - **[79572e34d](facebook/react@79572e34d )**: Adjust SuspenseList CPU bound heuristic (#17455) //<Sebastian Markbåge>// - **[969f4b5bb](facebook/react@969f4b5bb )**: Change DevTools hook warning message (#17478) //<Dan Abramov>// - **[6470e0f16](facebook/react@6470e0f16 )**: [Fresh] Make all errors recoverable (#17438) //<Dan Abramov>// Changelog: [General][Changed] - React sync for revisions 6cff70a...19f6fe1 Reviewed By: TheSavior Differential Revision: D19318286 fbshipit-source-id: acaa5224f7162a274c395a62e54da82199001005

Summary: This sync includes the following changes: - **[19f6fe170](facebook/react@19f6fe170 )**: Revert "Revert "Dispatch commands to both UIManagers from both renderers (facebook#17211)" (facebook#17232)" (facebook#17799) //<Eli White>// - **[6250462be](facebook/react@6250462be )**: Renamed "ReactDOM-fb" imports to "ReactDOM" in www shims (facebook#17797) //<Brian Vaughn>// - **[59f21f1b2](facebook/react@59f21f1b2 )**: HostText needs to copy over from current if it is unchanged in persistent mode (facebook#17538) //<Sebastian Markbåge>// - **[ccc6100d7](facebook/react@ccc6100d7 )**: Fix comments typos (facebook#17550) //<Nick S. Plekhanov>// - **[1b9328cd9](facebook/react@1b9328cd9 )**: Null stateNode after unmount (facebook#17666) //<Brian Vaughn>// - **[897976600](facebook/react@897976600 )**: [ESLint] Allow partial matches for custom Effect Hooks (facebook#17663) //<Dan Abramov>// - **[72592310a](facebook/react@72592310a )**: Create packages/dom-event-testing-library (facebook#17660) //<Nicolas Gallagher>// - **[a5e951d4c](facebook/react@a5e951d4c )**: [react-interactions] Event testing library improvements (facebook#17614) //<Nicolas Gallagher>// - **[7dc974542](facebook/react@7dc974542 )**: [Flight] Chunks API (facebook#17398) //<Sebastian Markbåge>// - **[9354dd275](facebook/react@9354dd275 )**: Make HostComponent inexact (facebook#17412) //<Eli White>// - **[4c270375e](facebook/react@4c270375e )**: Favor fallthrough switch instead of case statements for work tags (facebook#17648) //<Sebastian Markbåge>// - **[6fef7c47a](facebook/react@6fef7c47a )**: Add a regression test for switching from Fragment to a component (facebook#17647) //<Dan Abramov>// - **[9fe103124](facebook/react@9fe103124 )**: [react-interactions] Rename Flare APIs to deprecated and remove from RN (facebook#17644) //<Dominic Gannaway>// - **[4b0cdf29a](facebook/react@4b0cdf29a )**: Build FB RN targets only in experimental mode (facebook#17641) //<Dan Abramov>// - **[7309c5f93](facebook/react@7309c5f93 )**: Use zero-fill right shift instead of Math.floor (facebook#17616) //<伊撒尔>// - **[3c54df091](facebook/react@3c54df091 )**: Fix missing stacks in WWW warnings (facebook#17638) //<Dan Abramov>// - **[b66e86d95](facebook/react@b66e86d95 )**: react-refresh@0.7.1 //<Dan Abramov>// - **[c2d1561c6](facebook/react@c2d1561c6 )**: [Fast Refresh] Support injecting runtime after renderer executes (facebook#17633) //<Dan Abramov>// - **[f42431abe](facebook/react@f42431abe )**: Revert "Remove renderPhaseUpdates Map (facebook#17484)" (facebook#17623) //<Dan Abramov>// - **[0b5a26a48](facebook/react@0b5a26a48 )**: Rename toWarnDev -> toErrorDev, toLowPriorityWarnDev -> toWarnDev (facebook#17605) //<Dan Abramov>// - **[0cf22a56a](facebook/react@0cf22a56a )**: Use console directly instead of warning() modules (facebook#17599) //<Dan Abramov>// - **[b6c423daa](facebook/react@b6c423daa )**: Use matching test command for equivalence tests (facebook#17604) //<Dan Abramov>// - **[8a347ed02](facebook/react@8a347ed02 )**: Remove renderPhaseUpdates Map (facebook#17484) //<Sebastian Markbåge>// - **[be603f5a5](facebook/react@be603f5a5 )**: [react-events] Remove lastNativeEvent in favor of SystemFlags (facebook#17585) //<Dominic Gannaway>// - **[b15bf3675](facebook/react@b15bf3675 )**: Add component stacks to (almost) all warnings (facebook#17586) //<Dan Abramov>// - **[2afeebdcc](facebook/react@2afeebdcc )**: [react-interactions] Remove responder root event types + revert commit phase change (facebook#17577) //<Dominic Gannaway>// - **[9ac42dd07](facebook/react@9ac42dd07 )**: Remove the condition argument from warning() (facebook#17568) //<Laura buns>// - **[7bf40e1cf](facebook/react@7bf40e1cf )**: Initialize update queue object on mount (facebook#17560) //<Andrew Clark>// - **[e039e690b](facebook/react@e039e690b )**: Revert Update Queue Refactor //<Andrew Clark>// - **[b617db3d9](facebook/react@b617db3d9 )**: Refactor Update Queues to Fix Rebasing Bug //<Andrew Clark>// - **[b43eec7ea](facebook/react@b43eec7ea )**: Replace `wrap-warning-with-env-check` with an eslint plugin (facebook#17540) //<Laura buns>// - **[acfe4b21b](facebook/react@acfe4b21b )**: [react-interactions] Upgrade passive event listeners to active listeners (facebook#17513) //<Dominic Gannaway>// - **[5064c7f6a](facebook/react@5064c7f6a )**: Revert Rerender Error Check (facebook#17519) //<Sebastian Markbåge>// - **[6d105ad3f](facebook/react@6d105ad3f )**: [react-interactions] Move Flare event registration to commit phase (facebook#17518) //<Dominic Gannaway>// - **[dc18b8b8d](facebook/react@dc18b8b8d )**: Don't group Idle/Offscreen work with other work (facebook#17456) //<Sebastian Markbåge>// - **[f523b2e0d](facebook/react@f523b2e0d )**: Use fewer global variables in Hooks (facebook#17480) //<Sebastian Markbåge>// - **[d75323f65](facebook/react@d75323f65 )**: Remove case that only exists for createBatch (facebook#17506) //<Sebastian Markbåge>// - **[79572e34d](facebook/react@79572e34d )**: Adjust SuspenseList CPU bound heuristic (facebook#17455) //<Sebastian Markbåge>// - **[969f4b5bb](facebook/react@969f4b5bb )**: Change DevTools hook warning message (facebook#17478) //<Dan Abramov>// - **[6470e0f16](facebook/react@6470e0f16 )**: [Fresh] Make all errors recoverable (facebook#17438) //<Dan Abramov>// Changelog: [General][Changed] - React sync for revisions 6cff70a...19f6fe1 Reviewed By: TheSavior Differential Revision: D19318286 fbshipit-source-id: acaa5224f7162a274c395a62e54da82199001005

motiz88 requested a review from shergin as a code owner December 31, 2017 02:11

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 Dec 31, 2017

This was referenced Dec 31, 2017

Document letterSpacing, including on Android facebook/react-native-website#105

Merged

Support Image resizeMode=repeat on Android #17404

Closed

yenda mentioned this pull request Jan 19, 2018

Letter-spacing on Android status-im/status-mobile#1879

Closed

motiz88 force-pushed the letter-spacing-android-dec2017 branch from 58611cd to 6408a25 Compare January 20, 2018 09:44

hramos reviewed Jan 29, 2018

View reviewed changes

facebook-github-bot reviewed Jan 29, 2018

View reviewed changes

hramos approved these changes Feb 2, 2018

View reviewed changes

hramos suggested changes Feb 2, 2018

View reviewed changes

jerolimov and others added 9 commits February 3, 2018 13:32

Implement letterSpacing option on Android

b20f8d8

Add letter spacing text example for Android.

7eb1d04

Update documentation for Text style attribute ‘letterSpacing’

a736983

Add letterSpacing TextInput example on Android

78451ac

Implement placeholder letterSpacing in TextInput on Android

ba65661

Add RNTester cases for nested letterSpacing

c92342e

Add background color cases to RNTester Text examples

ecb66af

Update RNTester letterSpacing cases to set font size

1e3a83b

Apply letter spacing span after font size on Android

79da4b2

motiz88 force-pushed the letter-spacing-android-dec2017 branch from 6408a25 to 79da4b2 Compare February 3, 2018 13:33

This was referenced Feb 20, 2018

Add letter spacing for Android versions >= Lollipop (5.0). #13877

Closed

[WIP] LetterSpacing support for Android Lollipop and above (21) #16801

Closed

facebook-github-bot closed this in 5898817 Feb 27, 2018

zeh mentioned this pull request Mar 5, 2018

[Android] TextView doesn't support letter spacing and padding #3233

Closed

aronshamash mentioned this pull request Mar 7, 2018

Feat/video label newsuk/times-components#704

Merged

gnprice mentioned this pull request Jun 11, 2018

Fix extreme TextInput slowness on Android #19645

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement letterSpacing on Android >= 5.0 #17398

Implement letterSpacing on Android >= 5.0 #17398

motiz88 commented Dec 31, 2017 •

edited

Loading

janicduplessis commented Jan 20, 2018

janicduplessis commented Jan 20, 2018 •

edited

Loading

motiz88 commented Jan 20, 2018

motiz88 commented Jan 20, 2018 •

edited

Loading

janicduplessis commented Jan 20, 2018 •

edited

Loading

motiz88 commented Jan 20, 2018 •

edited

Loading

janicduplessis commented Jan 20, 2018

hramos Jan 29, 2018

hramos Jan 29, 2018

hramos commented Jan 29, 2018

facebook-github-bot left a comment

hramos left a comment •

edited

Loading

hramos left a comment

hramos commented Feb 3, 2018

motiz88 commented Feb 3, 2018 •

edited

Loading

hramos commented Feb 27, 2018

janicduplessis commented Feb 27, 2018

LouisJS commented May 15, 2018

motiz88 commented May 15, 2018 via email

Implement letterSpacing on Android >= 5.0 #17398

Implement letterSpacing on Android >= 5.0 #17398

Conversation

motiz88 commented Dec 31, 2017 • edited Loading

Motivation

Test Plan

RNTester - <Text>

RNTester - <TextInput placeholder>

RNTester - <TextInput value>

Related PRs

Release Notes

janicduplessis commented Jan 20, 2018

janicduplessis commented Jan 20, 2018 • edited Loading

motiz88 commented Jan 20, 2018

motiz88 commented Jan 20, 2018 • edited Loading

janicduplessis commented Jan 20, 2018 • edited Loading

motiz88 commented Jan 20, 2018 • edited Loading

janicduplessis commented Jan 20, 2018

hramos Jan 29, 2018

Choose a reason for hiding this comment

hramos Jan 29, 2018

Choose a reason for hiding this comment

hramos commented Jan 29, 2018

facebook-github-bot left a comment

Choose a reason for hiding this comment

hramos left a comment • edited Loading

Choose a reason for hiding this comment

hramos left a comment

Choose a reason for hiding this comment

hramos commented Feb 3, 2018

motiz88 commented Feb 3, 2018 • edited Loading

hramos commented Feb 27, 2018

janicduplessis commented Feb 27, 2018

LouisJS commented May 15, 2018

motiz88 commented May 15, 2018 via email

motiz88 commented Dec 31, 2017 •

edited

Loading

RNTester - `<Text>`

RNTester - `<TextInput placeholder>`

RNTester - `<TextInput value>`

janicduplessis commented Jan 20, 2018 •

edited

Loading

motiz88 commented Jan 20, 2018 •

edited

Loading

janicduplessis commented Jan 20, 2018 •

edited

Loading

motiz88 commented Jan 20, 2018 •

edited

Loading

hramos left a comment •

edited

Loading

motiz88 commented Feb 3, 2018 •

edited

Loading