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

Fix setting keyboardType from breaking autoCapitalize #27523

Closed
wants to merge 6 commits into from
Closed

Fix setting keyboardType from breaking autoCapitalize #27523

wants to merge 6 commits into from

Conversation

safaiyeh
Copy link
Contributor

@safaiyeh safaiyeh commented Dec 16, 2019

Summary

Fix for #27510.

Setting the InputType.TYPE_CLASS_TEXT flag when keyboardType is null or default breaks autoCapitalize. Handle the case when keyboardType is null, default, or invalid type.

Changelog

[Android] [Fixed] - Fix setting keyboardType from breaking autoCapitalize

Test Plan

Added keyboardType prop to RNTester as so

<TextInput autoCapitalize="words" keyboardType="default" style={styles.default} />

fixedKeyboardType

@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 Dec 16, 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.

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

@safaiyeh
Copy link
Contributor Author

Any action items from me for the Facebook Internal - Linter?

@elicwhite
Copy link
Member

I think this needs someone internal (likely @JoshuaGross) to run the formatter internally and reland.

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.

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

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @safaiyeh in 233fdfc.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Jan 7, 2020
facebook-github-bot pushed a commit that referenced this pull request Jan 23, 2020
Summary:
A previous PR broke toggling between visible and non-visible password (basically once a password field was made visible, future updates to the keyboardType were effectively ignored, so the password would always be visible).

Original PR: #27523

Changelog: [Internal]

Reviewed By: mdvacca, rodrigos-facebook

Differential Revision: D19527245

fbshipit-source-id: a5ab343c8a0c6a608171dbfa5afc7536ff241826
@JoshuaGross
Copy link
Contributor

@safaiyeh, please see this commit and verify that autoCapitalize still works for your test case here.

26d5faf

@safaiyeh
Copy link
Contributor Author

I'll check this out tomorrow @JoshuaGross, would adding an E2E test for it be helpful?

@safaiyeh
Copy link
Contributor Author

@JoshuaGross Just tested and confirm it works for 26d5faf.

@JoshuaGross
Copy link
Contributor

@safaiyeh E2E tests would be great!

alloy pushed a commit that referenced this pull request Mar 5, 2020
Summary:
Fix for #27510.

Setting the `InputType.TYPE_CLASS_TEXT` flag when `keyboardType` is null or default breaks autoCapitalize. Handle the case when `keyboardType` is null, default, or invalid type.

## Changelog

[Android] [Fixed] - Fix setting keyboardType from breaking autoCapitalize
Pull Request resolved: #27523

Test Plan:
Added keyboardType prop to RNTester as so
```
<TextInput autoCapitalize="words" keyboardType="default" style={styles.default} />
```
![fixedKeyboardType](https://user-images.githubusercontent.com/8675043/70872892-c96dec80-1f5f-11ea-8e33-714a67eff581.gif)

Reviewed By: makovkastar

Differential Revision: D19132261

Pulled By: JoshuaGross

fbshipit-source-id: be66f0317ed305425ebcff32046ad4bff06d367f
@vongohren
Copy link

@safaiyeh @JoshuaGross when is it worth thinking this fix will make it into the codebase? I dont see it with the 0.62.x RC

@safaiyeh
Copy link
Contributor Author

safaiyeh commented Mar 6, 2020

@vongohren @alloy is managing the release, the RC discussion will determine that.

Also if this is picked, this commit: 26d5faf also must be picked due to a regression this caused.

@vongohren
Copy link

@safaiyeh I tried asking in the RC discussion, but I was ignored. So just asking here to find some more insight

@alloy
Copy link
Contributor

alloy commented Mar 6, 2020

233fdfc was identified by react-native-community/releases#157 (comment), which was picked.

@safaiyeh Are you saying that 26d5faf is essential for your change [that I picked] to work right?

@safaiyeh
Copy link
Contributor Author

safaiyeh commented Mar 7, 2020

@alloy yup! It fixes a regression caused by this PR

@alloy
Copy link
Contributor

alloy commented Mar 7, 2020

Sigh, so that means RC4 is broken :( Thanks for the info!

alloy pushed a commit to alloy/react-native that referenced this pull request Mar 7, 2020
Summary:
A previous PR broke toggling between visible and non-visible password (basically once a password field was made visible, future updates to the keyboardType were effectively ignored, so the password would always be visible).

Original PR: facebook#27523

Changelog: [Internal]

Reviewed By: mdvacca, rodrigos-facebook

Differential Revision: D19527245

fbshipit-source-id: a5ab343c8a0c6a608171dbfa5afc7536ff241826
osdnk pushed a commit to osdnk/react-native that referenced this pull request Mar 9, 2020
Summary:
Fix for facebook#27510.

Setting the `InputType.TYPE_CLASS_TEXT` flag when `keyboardType` is null or default breaks autoCapitalize. Handle the case when `keyboardType` is null, default, or invalid type.

## Changelog

[Android] [Fixed] - Fix setting keyboardType from breaking autoCapitalize
Pull Request resolved: facebook#27523

Test Plan:
Added keyboardType prop to RNTester as so
```
<TextInput autoCapitalize="words" keyboardType="default" style={styles.default} />
```
![fixedKeyboardType](https://user-images.githubusercontent.com/8675043/70872892-c96dec80-1f5f-11ea-8e33-714a67eff581.gif)

Reviewed By: makovkastar

Differential Revision: D19132261

Pulled By: JoshuaGross

fbshipit-source-id: be66f0317ed305425ebcff32046ad4bff06d367f
osdnk pushed a commit to osdnk/react-native that referenced this pull request Mar 9, 2020
Summary:
A previous PR broke toggling between visible and non-visible password (basically once a password field was made visible, future updates to the keyboardType were effectively ignored, so the password would always be visible).

Original PR: facebook#27523

Changelog: [Internal]

Reviewed By: mdvacca, rodrigos-facebook

Differential Revision: D19527245

fbshipit-source-id: a5ab343c8a0c6a608171dbfa5afc7536ff241826
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API: Keyboard Bug 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. Platform: Android Android applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants