Revert the removal of isNative #1205
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Preventing native text input events completely breaks autocorrect, autocomplete, auto-capitalization, predictive text, and double-space period insertion on iOS. In order to allow iOS to perform these kinds of operations, we have to avoid preventing the default event behavior and allow native input handling whenever possible. Fixes ianstormtaylor#1176 Fixes ianstormtaylor#1177 See also: ianstormtaylor#1088
YurkaninRyan
reviewed
Oct 5, 2017
|
|
||
| // If the state has been changed natively, never re-render, or else we'll | ||
| // end up duplicating content. | ||
| if (props.state.isNative) return false |
There was a problem hiding this comment.
This never appears to be true. From what I can tell the change.state.isNative gets set to true, but this doesn't get updated in the props before it checks here.
YurkaninRyan
reviewed
Oct 5, 2017
| @@ -21,6 +21,7 @@ const DEFAULTS = { | |||
| selection: Selection.create(), | |||
| history: History.create(), | |||
| data: new Map(), | |||
| isNative: false, | |||
There was a problem hiding this comment.
Curious if this should go as a flag on the change object instead along with merge and save
|
Thanks for the feedback, @YurkaninRyan! I'm going to close this PR in favor of an alternative approach (discussed in #1177). I'll keep your suggestions in mind as I work on that. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Preventing native text input events completely breaks autocorrect, autocomplete, auto-capitalization, predictive text, and double-space period insertion on iOS. In order to allow iOS to perform these kinds of operations, we have to avoid preventing the default event behavior and allow native input handling whenever possible.
This is mostly a reversion of the changes introduced in #1088, but I've tried to maintain the cleanup that was done in
packages/slate-react/src/plugins/core.js'sonBeforeInputmethod since then.That method also includes one additional difference, which is the use of
change.deleteCharBackward()to avoid double input whenisNativeistrue. Restoring the previously removedisNativecode throughout the codebase no longer seems to be enough to prevent rendering double input since focus-related state changes cause theisNativestate to get reset tofalsebefore rendering happens. Deleting the inserted char seems like a bit of a hacky workaround to me, but it does work, and I wasn't able to come up with a better solution with my limited knowledge of the codebase. I'm open to suggestions.Fixes #1176
Fixes #1177