[iOS] Fix textAttributes not applied when typing text #23585

zhongwuzw · 2019-02-22T03:56:19Z

Summary

Currently, if we has defaultValue, textAttributes like letterSpacing can works, but if textinput has not default text, when we typing the text, some attributes not applied.

Changelog

[iOS] [Fixed] - Fix textAttributes not applied when typing text

Test Plan

We can use code like below, letterSpacing not work previously.

<TextInput
  style={{ letterSpacing: 10, color: 'red' }}
  placeholder="I will not respect letterSpacing value on iOS"
/>

Before:

If we have defaultValue, letterSpacing works:

<TextInput
  style={{ letterSpacing: 10 , color: 'red'}}
  placeholder="I will not respect letterSpacing value on iOS"
  defaultValue="s"
/>

After:

janicduplessis

Looks like some snapshot tests are failling, can you check if its related to this pr and fix those? Probably just need to update them if the changes are expected.

zhongwuzw · 2019-02-23T06:49:36Z

@janicduplessis Thanks for review, I scan the failed tests, seems the failed snapshot tests not related to TextInput, they don't use this module.

So I think it needs to fix by another PR? and TBO 😂 , I don't familiar how to do it, maybe I need remove all snapshot images and re-generate it?

janicduplessis · 2019-02-24T23:16:12Z

👍 I just double checked and you are right it is for other components. I think you have to use xcode to update the snapshots but I've never done it either haha.

janicduplessis · 2019-02-24T23:19:45Z

Libraries/Text/TextInput/Multiline/RCTUITextView.m

@@ -88,6 +91,20 @@ - (void)setPlaceholderColor:(UIColor *)placeholderColor
  _placeholderView.textColor = _placeholderColor ?: defaultPlaceholderColor();
 }

+- (void)setReactTextAttributes:(RCTTextAttributes *)reactTextAttributes
+{
+  if (!reactTextAttributes.effectiveTextAttributes || [reactTextAttributes isEqual:_reactTextAttributes]) {


Can you add a variable for effectiveTextAttributes to avoid computing it twice, looks like a pretty big getter

react-native/Libraries/Text/RCTTextAttributes.m

Line 82 in 9d16ddf

- (NSDictionary<NSAttributedStringKey, id> *)effectiveTextAttributes

(I hate properties :))

@janicduplessis 👍 I removed !reactTextAttributes.effectiveTextAttributes check because it cannot be nil.

janicduplessis · 2019-02-24T23:20:08Z

Libraries/Text/TextInput/Singleline/RCTUITextField.m

@@ -62,6 +65,20 @@ - (void)setPlaceholderColor:(UIColor *)placeholderColor
  [self _updatePlaceholder];
 }

+- (void)setReactTextAttributes:(RCTTextAttributes *)reactTextAttributes
+{
+  if (!reactTextAttributes.effectiveTextAttributes || [reactTextAttributes isEqual:_reactTextAttributes]) {


cpojer · 2019-02-25T02:55:13Z

@zhongwuzw can you update the screenshot examples?

zhongwuzw · 2019-02-25T04:06:45Z

@cpojer I don't know why here failed 🤔 , seems all other PRs like #23608 #23625 passes. So I don't ensure wether the code has issues, or I just re-generate the snapshots inside this PR? any instructions I can find to re-generate snapshots? I checked the snapshot tests code, seems I only need to remove snapshots

react-native/RNTester/RNTesterIntegrationTests/RNTesterIntegrationTests.m

Line 39 in 86d0611

_runner.recordMode = NO;

and set _runner.recordMode = YES?

cpojer · 2019-02-25T04:20:55Z

Yeah I think that's it.

janicduplessis · 2019-02-25T04:21:11Z

@zhongwuzw Looks like tests are passing on master, maybe you can just try rebasing see if it fixes it?

[iOS] Fix textAttributes not applied when typing text Remove dead code

zhongwuzw · 2019-02-25T06:53:48Z

@cpojer @janicduplessis CI passes! 🍻

cpojer

Awesome, thank you!

facebook-github-bot

@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Summary: This diff changes how we apply default text attributes to backed text input. The original change in #23585 that introduced the `reactTextAttributes` field in for RCTBackedTextInputViewProtocol was great! Thank you Wu zhongwuzw ! However, there is one detail that needs to be changed. RCTBackedTextInputViewProtocol is designed to only abstract complexity of iOS text input components (UITextView and UITextField); it intentionally does not have any React-specific fields or types. Adding a field `RCTTextAttributes *reactTextAttributes;` violates this principle and make it hard to reuse this functionality in the new Fabric-powered TextInput. This diff changes the type of this prop from `RCTTextAttributes` to `NSDictionary<NSAttributedStringKey,id> *` (exact same type that UITextView and UITextField use). Reviewed By: cpojer Differential Revision: D17408501 fbshipit-source-id: 65f2bba119ccc30f22e87c28d0f8ea6f731cd365

zhongwuzw requested a review from shergin as a code owner February 22, 2019 03:56

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 Feb 22, 2019

react-native-bot added Platform: iOS iOS applications. PR: Includes Changelog labels Feb 22, 2019

janicduplessis approved these changes Feb 22, 2019

View reviewed changes

janicduplessis suggested changes Feb 22, 2019

View reviewed changes

janicduplessis approved these changes Feb 24, 2019

View reviewed changes

janicduplessis reviewed Feb 24, 2019

View reviewed changes

janicduplessis approved these changes Feb 25, 2019

View reviewed changes

[iOS] Fix textAttributes not applied when typing text

ebaa9bb

[iOS] Fix textAttributes not applied when typing text Remove dead code

zhongwuzw force-pushed the fix_input_textAttributes branch from 4901a12 to ebaa9bb Compare February 25, 2019 05:00

Update snapshots on iOS12

0205241

cpojer approved these changes Feb 25, 2019

View reviewed changes

facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Feb 25, 2019

facebook-github-bot approved these changes Feb 25, 2019

View reviewed changes

facebook-github-bot closed this in 7a7eb11 Feb 25, 2019

hramos added the Merged This PR has been merged. label Mar 8, 2019

react-native-bot removed the Import Started This pull request has been imported. This does not imply the PR has been approved. label Mar 8, 2019

zhongwuzw mentioned this pull request Mar 22, 2019

[iOS] TextInput styles not applied with initial empty value #24087

Closed

RWOverdijk mentioned this pull request Mar 22, 2019

0.59.x Discussion react-native-community/releases#100

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[iOS] Fix textAttributes not applied when typing text #23585

[iOS] Fix textAttributes not applied when typing text #23585

zhongwuzw commented Feb 22, 2019

janicduplessis left a comment

zhongwuzw commented Feb 23, 2019

janicduplessis commented Feb 24, 2019

janicduplessis Feb 24, 2019

zhongwuzw Feb 25, 2019

janicduplessis Feb 24, 2019

cpojer commented Feb 25, 2019

zhongwuzw commented Feb 25, 2019

cpojer commented Feb 25, 2019

janicduplessis commented Feb 25, 2019

zhongwuzw commented Feb 25, 2019

cpojer left a comment

facebook-github-bot left a comment

[iOS] Fix textAttributes not applied when typing text #23585

[iOS] Fix textAttributes not applied when typing text #23585

Conversation

zhongwuzw commented Feb 22, 2019

Summary

Changelog

Test Plan

janicduplessis left a comment

Choose a reason for hiding this comment

zhongwuzw commented Feb 23, 2019

janicduplessis commented Feb 24, 2019

janicduplessis Feb 24, 2019

Choose a reason for hiding this comment

zhongwuzw Feb 25, 2019

Choose a reason for hiding this comment

janicduplessis Feb 24, 2019

Choose a reason for hiding this comment

cpojer commented Feb 25, 2019

zhongwuzw commented Feb 25, 2019

cpojer commented Feb 25, 2019

janicduplessis commented Feb 25, 2019

zhongwuzw commented Feb 25, 2019

cpojer left a comment

Choose a reason for hiding this comment

facebook-github-bot left a comment

Choose a reason for hiding this comment