Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 2 commits into from

Conversation

dchersey
Copy link
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
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
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

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

@matthargett
Copy link
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
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
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
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
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
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
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
matt-oakes pushed a commit to matt-oakes/react-native that referenced this pull request Feb 7, 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
@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
8 participants