Skip to content

Limit the number of Material spell check suggestions to 3 #124899

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 8 commits into from
Apr 18, 2023
Merged

Limit the number of Material spell check suggestions to 3 #124899

merged 8 commits into from
Apr 18, 2023

Conversation

justinmc
Copy link
Contributor

The comments seemed to say that the limit was three suggestions:

flutter/packages/flutter/lib/src/material/spell_check_suggestions_toolbar.dart

Line 17 in 2f36100

// assumes there are the maximum number of spell check suggestions available, 3.

But it wasn't enforced anywhere for Material, only for Cupertino. This PR throws away suggestions beyond the first 3 for Material too.

Before After
Screenshot from 2023-04-14 16-24-08 Screenshot from 2023-04-14 15-44-40

Partial fix for: #124882

@flutter-dashboard flutter-dashboard bot added 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 requested a review from camsim99 April 14, 2023 23:42
@justinmc justinmc self-assigned this Apr 14, 2023
@justinmc justinmc marked this pull request as ready for review April 14, 2023 23:42
camsim99
camsim99 approved these changes Apr 18, 2023
View reviewed changes
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.

Nice!

@justinmc justinmc merged commit f04a5af into flutter:master Apr 18, 2023
@justinmc justinmc deleted the spell-check-suggestions-limit branch April 18, 2023 19:50
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 19, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 19, 2023
@engine-flutter-autoroll
f04a5af Limit the number of Material spell check suggestions to 3 (fl…
6da07b7 
…utter/flutter#124899)
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Apr 19, 2023
@engine-flutter-autoroll
Roll Flutter from 42fb0b2 to 3476b96 (20 revisions) (#3760)
88591be 
flutter/flutter@42fb0b2...3476b96

2023-04-19 [email protected] Update helper message for `--suppress-analytics` (flutter/flutter#124810)
2023-04-19 [email protected] Roll Flutter Engine from 8b7cdb02f7f3 to 609f9d536494 (1 revision) (flutter/flutter#125097)
2023-04-19 [email protected] Roll Flutter Engine from 099ed6c62d04 to 8b7cdb02f7f3 (6 revisions) (flutter/flutter#125094)
2023-04-19 [email protected] Roll Flutter Engine from 5fcc7b719029 to 099ed6c62d04 (3 revisions) (flutter/flutter#125078)
2023-04-19 [email protected] Disableable ContextMenuButtonItems (flutter/flutter#124253)
2023-04-18 [email protected] Adaptive alert dialog (flutter/flutter#124336)
2023-04-18 [email protected] Roll Flutter Engine from 6d263ea56a62 to 5fcc7b719029 (4 revisions) (flutter/flutter#125069)
2023-04-18 [email protected] Remove package:js/dart:js_interop conflicts (flutter/flutter#124879)
2023-04-18 [email protected] Remove double.fromEnvironment from dart-define doc (flutter/flutter#124102)
2023-04-18 [email protected] Update to add Kim Jiun to `AUTHORS` (flutter/flutter#125026)
2023-04-18 [email protected] Add controller argument to SubmenuButton (flutter/flutter#125000)
2023-04-18 [email protected] Roll Flutter Engine from 879308a52228 to 6d263ea56a62 (1 revision) (flutter/flutter#125060)
2023-04-18 [email protected] Limit the number of Material spell check suggestions to 3 (flutter/flutter#124899)
2023-04-18 [email protected] Remove impeller testowners (flutter/flutter#125056)
2023-04-18 [email protected] [web] - Clean up skipped tests (flutter/flutter#124981)
2023-04-18 [email protected] Roll Flutter Engine from 72b68622fffa to 879308a52228 (1 revision) (flutter/flutter#125057)
2023-04-18 [email protected] Remove unused getRootRenderObject and getSelectedRenderObject service extensions (flutter/flutter#124805)
2023-04-18 [email protected] l10n.yaml's nullable-getter option should default to true (flutter/flutter#124353)
2023-04-18 [email protected] Roll Flutter Engine from 55bb065c607b to 72b68622fffa (1 revision) (flutter/flutter#125053)
2023-04-18 [email protected] Add vmservice for android build options (flutter/flutter#123034)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
This was referenced Apr 20, 2023
Closed
Closed
@HansMuller HansMuller mentioned this pull request Apr 21, 2023
Closed
@reidbaker reidbaker mentioned this pull request Apr 21, 2023
Merged
8 tasks
justinmc added a commit to justinmc/flutter that referenced this pull request Apr 25, 2023
@justinmc
Limit the number of Material spell check suggestions to 3 (flutter#12…
4c194b0 
…4899)

Fixes a bug where the spell check menu could overflow.
@justinmc justinmc mentioned this pull request Apr 25, 2023
Merged
itsjustkevin pushed a commit that referenced this pull request Apr 26, 2023
@justinmc
Cherry Pick Spell Check Fixes (#125517)
69b6c94 
This is a cherry pick of 8 of my recent spell check bug fixes into the
beta branch.

  1. #124259
  2. #124875
  3. #124254
  4. #124899
  5. #124895
  6. #125162
  7. #124897
  8. #125432

This is the behavior of spell check with these changes:

| Screenshot | Video |
| --- | --- |
| <img
src="https://user-images.githubusercontent.com/389558/234087650-bcd62c89-03e7-427d-afc5-0fe8f96a5f80.png"
/> | <video
src="https://user-images.githubusercontent.com/389558/234087667-651b0fde-348c-467e-ba00-27b6b3966a27.mov"
/> |

CC @itsjustkevin @leighajarett
nploi pushed a commit to nploi/packages that referenced this pull request Jul 16, 2023
@engine-flutter-autoroll @nploi
Roll Flutter from 42fb0b2 to 3476b96 (20 revisions) (flutter#3760)
d5f3329 
flutter/flutter@42fb0b2...3476b96

2023-04-19 [email protected] Update helper message for `--suppress-analytics` (flutter/flutter#124810)
2023-04-19 [email protected] Roll Flutter Engine from 8b7cdb02f7f3 to 609f9d536494 (1 revision) (flutter/flutter#125097)
2023-04-19 [email protected] Roll Flutter Engine from 099ed6c62d04 to 8b7cdb02f7f3 (6 revisions) (flutter/flutter#125094)
2023-04-19 [email protected] Roll Flutter Engine from 5fcc7b719029 to 099ed6c62d04 (3 revisions) (flutter/flutter#125078)
2023-04-19 [email protected] Disableable ContextMenuButtonItems (flutter/flutter#124253)
2023-04-18 [email protected] Adaptive alert dialog (flutter/flutter#124336)
2023-04-18 [email protected] Roll Flutter Engine from 6d263ea56a62 to 5fcc7b719029 (4 revisions) (flutter/flutter#125069)
2023-04-18 [email protected] Remove package:js/dart:js_interop conflicts (flutter/flutter#124879)
2023-04-18 [email protected] Remove double.fromEnvironment from dart-define doc (flutter/flutter#124102)
2023-04-18 [email protected] Update to add Kim Jiun to `AUTHORS` (flutter/flutter#125026)
2023-04-18 [email protected] Add controller argument to SubmenuButton (flutter/flutter#125000)
2023-04-18 [email protected] Roll Flutter Engine from 879308a52228 to 6d263ea56a62 (1 revision) (flutter/flutter#125060)
2023-04-18 [email protected] Limit the number of Material spell check suggestions to 3 (flutter/flutter#124899)
2023-04-18 [email protected] Remove impeller testowners (flutter/flutter#125056)
2023-04-18 [email protected] [web] - Clean up skipped tests (flutter/flutter#124981)
2023-04-18 [email protected] Roll Flutter Engine from 72b68622fffa to 879308a52228 (1 revision) (flutter/flutter#125057)
2023-04-18 [email protected] Remove unused getRootRenderObject and getSelectedRenderObject service extensions (flutter/flutter#124805)
2023-04-18 [email protected] l10n.yaml's nullable-getter option should default to true (flutter/flutter#124353)
2023-04-18 [email protected] Roll Flutter Engine from 55bb065c607b to 72b68622fffa (1 revision) (flutter/flutter#125053)
2023-04-18 [email protected] Add vmservice for android build options (flutter/flutter#123034)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@camsim99 camsim99 camsim99 approved these changes

Assignees

@justinmc justinmc

Labels
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@justinmc @camsim99