forked from facebook/react-native
-
Notifications
You must be signed in to change notification settings - Fork 126
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
[iOS/macOS] [TextInput] Implement ghost text #1897
Merged
Merged
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
Ghost text is hint text that is inserted inline as a hint to the user (for example predictive text). Unlike autocomplete for combo boxes, which is "visible" to the model, ghost text is "invisible" or transparent to the model (i.e. not observable in any callback or imperative method). Ghost text isn't selectable by the user. Ideally we do ghost text only in the display layer, meaning it isn't in the model. Unfortunately it doesn't look like we can render ghost text and still have Apple flow text for us (ghost text that is in the middle of regular text will want to reflow the regular text to make room for ghost text). Overriding all text box rendering seems like substantial rework (we need to deal with all drawing, including but not limited to the text box background, text, insertion pointer\selection, ... as well as other ancillary things around positioning related input UI such as IME's), so here we DO introduce it in the model, but try to keep awareness on the native side, and away from Application logic. Since ghost text is presumed dependent on immediate user context (e.g. text and selection), we reset ghost text automatically when either of these change. We could *not* do that, but that requires us to deal with any possible edits around\on the ghost text, as well as actually ensure we "hide" ghost text from any callbacks made to the app (something we are getting away with right now since we remove ghost text on pretty much any user interaction). Plumbing for new imperative method to set ghost text. * Libraries/Components/TextInput/RCTMultilineTextInputNativeComponent.js * Libraries/Components/TextInput/RCTSingelineTextInputNativeComponent.js * Libraries/Components/TextInput/TextInput.flow.js * Libraries/Components/TextInput/TextInput.js * Libraries/Components/TextInput/TextInputNativeCommands.js * Libraries/Text/TextInput/RCTBaseTextInputViewManager.m Introduce a property on the `RCTUITextView` and `RCTUITextField` to store state on whether we're currently in the midst of setting\clearing ghost text. We use this to suppress various callbacks to the Application that may be triggered as a result of us changing the ghost text\selection to compensate for addition\removal of ghost text. * Libraries/Text/TextInput/Multiline/RCTUITextView.h * Libraries/Text/TextInput/RCTBackedTextInputDelegateAdapter.m * Libraries/Text/TextInput/RCTBackedTextInputViewProtocol.h * Libraries/Text/TextInput/Singleline/RCTUITextField.h Actual ghost text management logic. I placed as much as I reasonably could here since that allows us to share most of the functionality between single and multi line text inputs. * Libraries/Text/TextInput/RCTBaseTextInputView.h * Libraries/Text/TextInput/RCTBaseTextInputView.m Reintroduce `onTextInput` into Flow typing. This was removed in commit 3f7e0a2, but it looks like it wasn't ever removed on the native side, so the callback still "works" for now. I am using this callback to determine ghost text behavior correctness, and since it is fired, I need it back. When we truly remove this from the native side, we can stop caring about it. * Libraries/Components/TextInput/TextInput.flow.js * Libraries/Components/TextInput/TextInput.js Test page for ghost text. Shows predictive text example. * packages/rn-tester/js/examples/GhostText/GhostText.js * packages/rn-tester/js/utils/RNTesterList.ios.js Testing (macOS\iOS x multi\single line): * Type predictable text (see test page for list), see that we get ghost text and that isn't observable in the callbacks * Type non-predictable text, check ordering of events consistent with predictable text case * Log calls to `setGhostText:` on native. Ensure we clear ghost text (due to text chagnes) * Change selection with ghost text, check that we remove ghost text (due to selection change) * Hit Esc in text box with ghost text, check that we remove ghost text (due to focus loss)
|
Saadnajmi
approved these changes
Aug 2, 2023
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.
Please select one of the following
Summary
Ghost text is hint text that is inserted inline as a hint to the user (for example predictive text). Unlike autocomplete for combo boxes, which is "visible" to the model, ghost text is "invisible" or transparent to the model (i.e. not observable in any callback or imperative method). Ghost text isn't selectable by the user.
Ideally we do ghost text only in the display layer, meaning it isn't in the model. Unfortunately it doesn't look like we can render ghost text and still have Apple flow text for us (ghost text that is in the middle of regular text will want to reflow the regular text to make room for ghost text). Overriding all text box rendering seems like substantial rework (we need to deal with all drawing, including but not limited to the text box background, text, insertion pointer\selection, ... as well as other ancillary things around positioning related input UI such as IME's), so here we DO introduce it in the model, but try to keep awareness on the native side, and away from Application logic.
Since ghost text is presumed dependent on immediate user context (e.g. text and selection), we reset ghost text automatically when either of these change. We could not do that, but that requires us to deal with any possible edits around\on the ghost text, as well as actually ensure we "hide" ghost text from any callbacks made to the app (something we are getting away with right now since we remove ghost text on pretty much any user interaction).
Plumbing for new imperative method to set ghost text.
Introduce a property on the
RCTUITextView
andRCTUITextField
to store state on whether we're currently in the midst of setting\clearing ghost text. We use this to suppress various callbacks to the Application that may be triggered as a result of us changing the ghost text\selection to compensate for addition\removal of ghost text.Actual ghost text management logic. I placed as much as I reasonably could here since that allows us to share most of the functionality between single and multi line text inputs.
Test page for ghost text. Shows predictive text example.
Changelog
General Changed - Initial commit
Test Plan
Testing (macOS\iOS x multi\single line):
setGhostText:
on native. Ensure we clear ghost text (due to text chagnes)Test page example video: https://github.com/microsoft/react-native-macos/assets/72474613/38f466c3-695b-4526-bbb2-a5afb8e333d8