Skip to content

Conversation

@justinmc
Copy link
Contributor

@justinmc justinmc commented Apr 14, 2023

Previously, the Android-style spell check menu was shown by default for TextField, even on iOS. Instead, this should behave like AdaptiveTextSelectionToolbar:

  1. By default, TextField should show the correct menu for the current platform.
  2. Cupertino should always show a Cupertino menu, because it has no access to the Material library.
  3. Application developers should be able to override this behavior.
Before After
Screenshot from 2023-04-14 14-55-47 Screenshot 2023-04-20 at 11 15 36 AM

Partial fix for: #124882
Waiting on: #124254

The overflow will be handled in another PR.

@justinmc justinmc self-assigned this Apr 14, 2023
@flutter-dashboard flutter-dashboard bot added a: text input Entering text in a text field or keyboard related problems f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Apr 14, 2023
@justinmc justinmc marked this pull request as draft April 14, 2023 21:50
@justinmc justinmc marked this pull request as ready for review April 18, 2023 18:05
@justinmc justinmc requested a review from camsim99 April 18, 2023 18:06
Copy link
Contributor

@camsim99 camsim99 left a comment

Choose a reason for hiding this comment

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

Looks good to me! Left one nit and a question about the old toolbar constructors.


if (buttonItems == null){
return const SizedBox.shrink();
switch (defaultTargetPlatform) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you update the documentation on this method to explain the adaptiveness of the toolbar implemented here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, good catch, it's out of date now.

/// See also:
/// * [SpellCheckSuggestionsToolbar.editableText], which is similar but
/// builds an Android-style toolbar.
CupertinoSpellCheckSuggestionsToolbar.editableText({
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious -- with the editableText named constructors, do we need the other ones?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • A children-based constructor - Could be useful for advanced customization.
  • A plain editable constructor - Could be useful for custom text editors.
  • selectable or selectableRegion - Unlikely to be useful because users probably don't want to spell check non-editable text?

I think let's wait and see if anyone opens some issues asking for these. I would be up for it if they did though.

@justinmc justinmc merged commit 13f4fe4 into flutter:master Apr 20, 2023
@justinmc justinmc deleted the spell-check-ios-material branch April 20, 2023 18:09
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 20, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 20, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 20, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 21, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 21, 2023
@reidbaker reidbaker mentioned this pull request Apr 21, 2023
8 tasks
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 22, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 22, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 23, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 23, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 24, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 24, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 24, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 24, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 24, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 25, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 25, 2023
justinmc added a commit to justinmc/flutter that referenced this pull request Apr 25, 2023
Even in TextField using the Material library, iOS devices will still show the iOS-style spell check toolbar by default when using spell check.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: text input Entering text in a text field or keyboard related problems f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants