Skip to content

iOS: Introduce spellCheck prop to TextInput#11056

Closed
rigdern wants to merge 2 commits into
react:masterfrom
rigdern:rigdern/iosSpellCheck
Closed

iOS: Introduce spellCheck prop to TextInput#11056
rigdern wants to merge 2 commits into
react:masterfrom
rigdern:rigdern/iosSpellCheck

Conversation

@rigdern

@rigdern rigdern commented Nov 22, 2016

Copy link
Copy Markdown
Contributor

This exposes iOS's spellCheckingType functionality to JavaScript. The native functionality is a three state enum. It gets exposed to JavaScript as a boolean. The initial value and JS null map to the third state.

An alternative design for this API would have been to expose a three state enum to JavaScript:

  • "on" which maps to UITextSpellCheckingTypeYes
  • "off" which maps to UITextSpellCheckingTypeNo
  • "auto" (default) which maps to UITextSpellCheckingTypeDefault

For consistency, I decided to use the same API design as spellCheck. We don't have many options for fixing spellCheck in #11055 without introducing a breaking change.

Test plan (required)

Verified that switching spellCheck between true, false, and null all work correctly in single line and multiline TextInputs.

@facebook-github-bot

Copy link
Copy Markdown
Contributor

By analyzing the blame information on this pull request, we identified @janicduplessis and @javache to be potential reviewers.

@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Nov 22, 2016
Comment thread Libraries/Text/RCTTextFieldManager.m Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this be UITextSpellCheckingType instead of NSString?

Comment thread Libraries/Text/RCTTextViewManager.m Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Instead of repeating the logic here, extract this logic to an RCTConvert category.

@mkonicek mkonicek changed the title iOS: Introduce spellCheck prop iOS: Introduce spellCheck prop to TextInput Nov 22, 2016
@rigdern

rigdern commented Nov 23, 2016

Copy link
Copy Markdown
Contributor Author

I'm going to wait to see what happens with #11055 since this one will use the same pattern.

Adam Comella added 2 commits November 23, 2016 13:42
This exposes iOS's spellCheckingType functionality to JavaScript. The native functionality is a three state enum. It gets exposed to JavaScript as a boolean. The initial value and JS null map to the third state.

An alternative design for this API would have been to expose a three state enum to JavaScript:
  - "on" which maps to UITextSpellCheckingTypeYes
  - "off" which maps to UITextSpellCheckingTypeNo
  - "auto" (default) which maps to UITextSpellCheckingTypeDefault

For consistency, I decided to use the same API design as spellCheck. We don't have many options for fixing spellCheck in react#11055 without introducing a breaking change.
@rigdern

rigdern commented Nov 23, 2016

Copy link
Copy Markdown
Contributor Author

@javache I pushed a change to address your feedback and rebased my branch on master.

@facebook-github-bot

Copy link
Copy Markdown
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants