Skip to content

Fixes capitalized I's when emojiis are present after the text being edited.#21951

Closed
dchersey wants to merge 2 commits intofacebook:masterfrom
Leap-Forward:fix-textinput-emojiis
Closed

Fixes capitalized I's when emojiis are present after the text being edited.#21951
dchersey wants to merge 2 commits intofacebook:masterfrom
Leap-Forward:fix-textinput-emojiis

Conversation

@dchersey
Copy link
Copy Markdown
Contributor

@dchersey dchersey commented Oct 25, 2018

Fixes #21243.
Fixes #20908.

Credit goes to @superandrew213 who provided the patch based on 0.56; this commit merges and resolved the conflict introduced in 0.57.

Test Plan:

When you have an emoji as the last character of your text input and then move the text cursor back (still with emoji as last character) and enter new letters somewhere in the middle of the string, the TextInput begins to capitalize all letter I's without this PR.

Without this PR in place:

  1. Create a TextInput on a View
  2. Run it on IOS in development or release build in a simulator or on a device
  3. Type "Test with emoji 😃"
  4. Move the curser back before the emojii
  5. Type "edited". The text will appear as "edIted".
  6. Apply this PR
  7. Run it again
  8. Type "Test with emoji 😃"
  9. Move the curser back before the emojii
  10. Type "edited". The text will now appear as "edited".

Release Notes:

Help reviewers and the release process by writing your own release notes. See below for an example.

[IOS] [BUGFIX] [TextInput] - Fixes bug where letter I's are permanently capitalized when editing text that appears before emojiis

…t after the text being edited.

# Conflicts:
#	Libraries/Text/TextInput/RCTBaseTextInputView.m
@facebook-github-bot 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 Oct 25, 2018
Copy link
Copy Markdown
Contributor

@matthargett matthargett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add an e2e test for this?

@dchersey
Copy link
Copy Markdown
Contributor Author

@matthargett I'm game to do so; I've been able to build and run RNTester and I see the TextInputExample ... I guess I just need to add an example showing my emojii bug & testing the fix.

This is my first contribution to RN and I haven't been able to figure out how to actually add the test ... the doc is not super-helpful unfortunately. Can you point me in a direction?

@mikefogg
Copy link
Copy Markdown

Hey guys, any update on getting this fix in there?

@matthargett
Copy link
Copy Markdown
Contributor

I tried extending the current RNTester e2e tests to exercise things in this way, but had some trouble once the keyboard changes locales. GOing to mark this as approve and follow up with other folks. Thanks!

@kelset
Copy link
Copy Markdown
Contributor

kelset commented Feb 6, 2019

@facebook-github-bot shipit

@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Feb 6, 2019
Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@facebook-github-bot facebook-github-bot added Import Failed and removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Feb 7, 2019
@facebook-github-bot
Copy link
Copy Markdown
Contributor

I tried to merge this pull request into the Facebook internal repo but some checks failed. To unblock yourself please check the following: Does this pull request pass all open source tests on GitHub? If not please fix those. Does the code still apply cleanly on top of GitHub master? If not can please rebase. In all other cases this means some internal test failed, for example a part of a fb app won't work with this pull request. I've added the Import Failed label to this pull request so it is easy for someone at fb to find the pull request and check what failed. If you don't see anyone comment in a few days feel free to comment mentioning one of the core contributors to the project so they get a notification.

Copy link
Copy Markdown
Contributor

@cpojer cpojer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Feb 7, 2019
Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@react-native-bot
Copy link
Copy Markdown
Collaborator

@dchersey merged commit f307ac7 into facebook:master.

@facebook facebook locked as resolved and limited conversation to collaborators Feb 7, 2019
@react-native-bot react-native-bot added the Merged This PR has been merged. label Feb 7, 2019
@hramos hramos removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Feb 14, 2019
grabbou pushed a commit that referenced this pull request Feb 18, 2019
…dited. (#21951)

Summary:
Fixes #21243.
Fixes #20908.

Credit goes to superandrew213 who provided the patch based on 0.56; this commit merges and resolved the conflict introduced in 0.57.
Pull Request resolved: #21951

Differential Revision: D13980799

Pulled By: cpojer

fbshipit-source-id: 6b9f1a1ae54ad9dba043005d683d6a221472c729
t-nanava pushed a commit to microsoft/react-native-macos that referenced this pull request Jun 17, 2019
…dited. (facebook#21951)

Summary:
Fixes facebook#21243.
Fixes facebook#20908.

Credit goes to superandrew213 who provided the patch based on 0.56; this commit merges and resolved the conflict introduced in 0.57.
Pull Request resolved: facebook#21951

Differential Revision: D13980799

Pulled By: cpojer

fbshipit-source-id: 6b9f1a1ae54ad9dba043005d683d6a221472c729
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[IOS] TextInput with emojis capitalizes text and causes other issues TextInput capitalized all letter i's when there's an emoji before the text

8 participants