Enable inline text prediction on iOS (issue #135221)#182728
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request enables inline text prediction on iOS 17 and later by implementing the setAttributedMarkedText:selectedRange: method in FlutterTextInputPlugin and TextInputSemanticsObject. The implementation forwards the call to the existing setMarkedText method after extracting the plain string. A new unit test is added to verify this functionality. The PR also includes a timing adjustment in vsync_waiter_ios to prevent a potential assertion failure, and unrelated Swift concurrency improvements for ResizeSynchronizer on macOS. Additionally, a new markdown file is added to document how to run the iOS text input tests.
- FlutterTextInputPlugin: support predictive text / inline predictions - TextInputSemanticsObject: semantics for inline prediction - vsync_waiter_ios: related timing adjustments - Add run_ios_text_input_tests.md - ResizeSynchronizer (macOS): unrelated sync fixes
- Framework: add enableInlinePrediction and composingStyle to TextInputConfiguration, EditableText, TextField, and CupertinoTextField for iOS 17+ inline suggestions. - Engine: set inlinePredictionType from config; implement setAttributedMarkedText for FlutterTextInputView and TextInputSemanticsObject (iOS 17+). - Fix FlutterTextInputPluginTest: assert marked range length 17 for 'inline prediction'. - Revert unrelated ResizeSynchronizer/ResizeSynchronizerTest (macOS) changes.
93f48ba to
b289c9e
Compare
Test run instructions are not needed in the engine tree.
There was a problem hiding this comment.
Code Review
This pull request enables inline text prediction on iOS 17+ by adding enableInlinePrediction and composingStyle properties to TextField and CupertinoTextField. The changes are well-implemented across the engine and framework, including necessary tests. Additionally, a fix in vsync_waiter_ios.mm addresses a potential crash related to CADisplayLink timestamps, which improves overall stability. I have one suggestion to improve code readability.
|
/gemeni review |
| self.isVisibleToAutofill = autofill || _secureTextEntry; | ||
|
|
||
| if (@available(iOS 17.0, *)) { | ||
| NSNumber* enableInlinePrediction = configuration[kEnableInlinePrediction]; |
There was a problem hiding this comment.
Is it supposed to be NSValue here?
There was a problem hiding this comment.
the config value is a boolean from the channel, hence NSNumber. Tested this flow locally as well to validate this behaviour
| NSNumber* enableInlinePrediction = configuration[kEnableInlinePrediction]; | ||
| BOOL enabled = enableInlinePrediction == nil || [enableInlinePrediction boolValue]; | ||
| self.inlinePredictionType = | ||
| enabled ? UITextInlinePredictionTypeYes : UITextInlinePredictionTypeNo; |
There was a problem hiding this comment.
What about default?
There was a problem hiding this comment.
platform default behaviour will be default - and null/no value will be considered as default
| required BuildContext context, | ||
| TextStyle? style, | ||
| required bool withComposing, | ||
| TextStyle? composingStyle, |
There was a problem hiding this comment.
This will likely be a breaking change (based on my past attempts to add new parameters to this method).
There was a problem hiding this comment.
If that's so we should solve this once and for all: Create a new data class class BuildTextSpanInfo that contains all these parameters, and use this class as the parameter instead. This way adding new parameters no longer breaks.
There was a problem hiding this comment.
Ah it looks like you're trying to expose the configuration on TextField/CupertinoTextField. The widget-level API makes sense but I'm still worried this change (on TextEditingController) will be breaking.
Since this is a separate / potentially breaking feature from inline prediction (and people can subclass TextEditingController to achieve the same thing, and interestingly this is going to be a breaking change for them), I feel we should hold off adding this argument if it's indeed breaking.
There was a problem hiding this comment.
I agree with @LongCatIsLooong's comment. Also I'm concerned that:
- Inline predictive text will have incorrect styling but will be on by default on iOS 17+.
- We're coupling the styling for inline predictive text and the composition string, even though these are styled differently on iOS.
I'd suggest that in this PR:
- We remove the APIs to customize the
composingStyle. - We make inline text prediction off by default on all platforms. iOS apps with
enablePredictiveText: nullhave inline predictive text disabled. iOS apps must useenablePredictiveText: trueto enable it.
Then in one or more subsequent PRs:
- Add APIs to customize the
composingStyleand/or theinlinePredictionStyle - Update
CupertinoTextFieldto customize these styles to match iOS's styling for inline predictive text - Make
enablePredictiveText: nulluse the platform default for inline text prediction on iOS 17+.
What do y'all think?
There was a problem hiding this comment.
makes sense @loic-sharma , I will split this PR and share
|
I will be adding these attributes to ThemeData class as well - so it can easily controlled centrally. |
dkwingsmt
left a comment
There was a problem hiding this comment.
Generally LGTM with minor comments.
| /// Optional style for the composing (and inline prediction) region. | ||
| /// | ||
| /// When set, applied to the composing range (IME and inline predictive text). | ||
| /// When null, the default is used ([TextDecoration.underline]). |
There was a problem hiding this comment.
How come you can use one style to represent both IME and predictive text? AFAI understand, IME text uses regular color with underline, while predictive text uses grey color and no underline.
There was a problem hiding this comment.
The iOS text input implementation does not distinguish these two. UX-wise I don't think it's too big of a deal since to users they both represent uncommitted changes suggested by iOS.
| required BuildContext context, | ||
| TextStyle? style, | ||
| required bool withComposing, | ||
| TextStyle? composingStyle, |
There was a problem hiding this comment.
If that's so we should solve this once and for all: Create a new data class class BuildTextSpanInfo that contains all these parameters, and use this class as the parameter instead. This way adding new parameters no longer breaks.
|
FYI the analyzer is complaining:
From my experience implementing the same feature, I was never able to trigger inline prediction on a custom Overall the approach LGTM, I have just a question regarding the current default and not sure about the (likely breaking) change. |
Co-authored-by: Tong Mu <dkwingsmt@users.noreply.github.com>
| Color? suffixIconColor, | ||
| BoxConstraints? suffixIconConstraints, | ||
| TextStyle? counterStyle, | ||
| TextStyle? composingStyle, |
There was a problem hiding this comment.
New properties should be added in InputDecorationThemeData only. Check #168981 for more context:) Eventually the xxxTheme class should only contain data and child fields and any other properties should only exist in xxxThemeData:)
| - (void)setAttributedMarkedText:(NSAttributedString*)attributedString | ||
| selectedRange:(NSRange)selectedRange API_AVAILABLE(ios(17.0)) { | ||
| NSString* markedText = attributedString ? [attributedString string] : @""; | ||
| [self setMarkedText:markedText selectedRange:selectedRange]; |
There was a problem hiding this comment.
From my understanding, this works because currently the iOS text input plugin considers any marked text as the composition string:
However, this forces the framework to use the same style for inline predictive text and the composition string. That is not desirable since these strings have different styling on iOS.
@hellohuanlin @LongCatIsLooong What do y'all think of introducing new values to the text input editing state to capture the inline predictive text's base / extent? Something like inlinePredictionBase and inlinePredictionExtent. This would let track the composition string and the inline predictive text separately, which would let us style these separately.
There was a problem hiding this comment.
Yes, reusing the composition text feels a bit too magical for me (#182728 (comment))
I prefer having a more explicit setup as you described, since they are semantically different concepts.
Co-authored-by: Loïc Sharma <737941+loic-sharma@users.noreply.github.com>
Co-authored-by: Loïc Sharma <737941+loic-sharma@users.noreply.github.com>
|
Thanks everyone for reviewing the PR |
|
Thanks! I'll convert this PR to a draft since it contains styling changes that aren't ready to be reviewed until after this lands. Please feel free to mark this as ready for review when ready! |
|
This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
Expose inline prediction configuration through Flutter text fields so apps can explicitly control iOS 17 inline predictive text behavior and ensure the engine handles attributed marked text correctly. This PR adds an `enableInlinePrediction` option to `TextInputConfiguration`, `EditableText`, `TextField`, and `CupertinoTextField`, and forwards that setting to the iOS text input plugin. On the engine side, the iOS text input implementation now: - maps `enableInlinePrediction` to `UITextInlinePredictionType` on iOS 17+ - defaults inline prediction to disabled unless the framework explicitly enables it - handles `setAttributedMarkedText:selectedRange:` by forwarding the plain string to the existing marked text flow so inline predictive text updates are processed correctly This keeps the feature opt-in, preserves existing behavior by default, and gives apps explicit control over inline predictive text on supported iOS versions. Fixes: #135221 Previous PR: #182728 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines --------- Co-authored-by: Loïc Sharma <737941+loic-sharma@users.noreply.github.com>
Expose inline prediction configuration through Flutter text fields so apps can explicitly control iOS 17 inline predictive text behavior and ensure the engine handles attributed marked text correctly. This PR adds an `enableInlinePrediction` option to `TextInputConfiguration`, `EditableText`, `TextField`, and `CupertinoTextField`, and forwards that setting to the iOS text input plugin. On the engine side, the iOS text input implementation now: - maps `enableInlinePrediction` to `UITextInlinePredictionType` on iOS 17+ - defaults inline prediction to disabled unless the framework explicitly enables it - handles `setAttributedMarkedText:selectedRange:` by forwarding the plain string to the existing marked text flow so inline predictive text updates are processed correctly This keeps the feature opt-in, preserves existing behavior by default, and gives apps explicit control over inline predictive text on supported iOS versions. Fixes: flutter#135221 Previous PR: flutter#182728 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines --------- Co-authored-by: Loïc Sharma <737941+loic-sharma@users.noreply.github.com>
Description
This PR adds support for inline predictive text on iOS 17+ (the gray suggestion that appears after the cursor as you type). It does two things:
Enable/disable inline prediction – Apps can turn inline prediction on or off per field via a new
enableInlinePredictionparameter onTextFieldandCupertinoTextField(default remainstrue). The setting is sent throughTextInputConfigurationto the engine, which setsUITextInlinePredictionTypeon the iOS text input view so the system shows or hides inline suggestions.Style the prediction/composing region – A new optional
composingStyleparameter lets apps control how the composing range (IME and inline prediction) is drawn. When set (e.g. gray with no underline), the suggestion can match the native iOS look instead of the default underline.Why: On iOS 17+, the system keyboard can show inline predictions; Flutter had no way to enable/disable this or style the suggestion text. This change adds that control and styling so Flutter apps can match platform behavior and design.
Technical notes:
TextInputConfiguration.enableInlinePrediction,EditableText/TextField/CupertinoTextFieldpass-through, andTextEditingController.buildTextSpan(composingStyle)for the composing region.enableInlinePredictioninconfigureWithDictionary:, setFlutterTextInputView.inlinePredictionType, and implementsetAttributedMarkedText:selectedRange:so the system can deliver inline prediction. Semantics objects updated for the same protocol.composingStyleonly affects how the composing range is drawn.Consider adding before/after screenshots: one with inline prediction off or default underline, one with
enableInlinePrediction: trueand a customcomposingStyle(e.g. gray, no underline) to show the new behavior.Fixes #135221
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.