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

Fixes a bug where the spell check menu could overflow.

Author: justinmc

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

@@ -12,26 +12,30 @@ import 'text_selection_toolbar.dart';
 import 'text_selection_toolbar_button.dart';
 
 /// iOS only shows 3 spell check suggestions in the toolbar.
-const int _maxSuggestions = 3;
+const int _kMaxSuggestions = 3;
 
 /// The default spell check suggestions toolbar for iOS.
 ///
 /// Tries to position itself below the [anchors], but if it doesn't fit, then it
 /// readjusts to fit above bottom view insets.
 class CupertinoSpellCheckSuggestionsToolbar extends StatelessWidget {
   /// Constructs a [CupertinoSpellCheckSuggestionsToolbar].
+  ///
+  /// [buttonItems] must not contain more than three items.
   const CupertinoSpellCheckSuggestionsToolbar({
     super.key,
     required this.anchors,
     required this.buttonItems,
-  });
+  }) : assert(buttonItems.length <= _kMaxSuggestions);
 
   /// The location on which to anchor the menu.
   final TextSelectionToolbarAnchors anchors;
 
   /// The [ContextMenuButtonItem]s that will be turned into the correct button
   /// widgets and displayed in the spell check suggestions toolbar.
   ///
+  /// Must not contain more than three items.
+  ///
   /// See also:
   ///
   ///  * [AdaptiveTextSelectionToolbar.buttonItems], the list of
@@ -71,11 +75,7 @@ class CupertinoSpellCheckSuggestionsToolbar extends StatelessWidget {
     final List<ContextMenuButtonItem> buttonItems = <ContextMenuButtonItem>[];
 
     // Build suggestion buttons.
-    int suggestionCount = 0;
-    for (final String suggestion in spanAtCursorIndex.suggestions) {
-      if (suggestionCount >= _maxSuggestions) {
-        break;
-      }
+    for (final String suggestion in spanAtCursorIndex.suggestions.take(_kMaxSuggestions)) {
       buttonItems.add(ContextMenuButtonItem(
         onPressed: () {
           if (!editableTextState.mounted) {
@@ -89,7 +89,6 @@ class CupertinoSpellCheckSuggestionsToolbar extends StatelessWidget {
         },
         label: suggestion,
       ));
-      suggestionCount += 1;
     }
     return buttonItems;
   }

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

@@ -18,17 +18,23 @@ import 'text_selection_toolbar_text_button.dart';
 // Size eyeballed on Pixel 4 emulator running Android API 31.
 const double _kDefaultToolbarHeight = 193.0;
 
+/// The maximum number of suggestions in the toolbar is 3, plus a delete button.
+const int _kMaxSuggestions = 3;
+
 /// The default spell check suggestions toolbar for Android.
 ///
 /// Tries to position itself below the [anchor], but if it doesn't fit, then it
 /// readjusts to fit above bottom view insets.
 class SpellCheckSuggestionsToolbar extends StatelessWidget {
   /// Constructs a [SpellCheckSuggestionsToolbar].
+  ///
+  /// [buttonItems] must not contain more than four items, generally three
+  /// suggestions and one delete button.
   const SpellCheckSuggestionsToolbar({
     super.key,
     required this.anchor,
     required this.buttonItems,
-  });
+  }) : assert(buttonItems.length <= _kMaxSuggestions + 1);
 
   /// {@template flutter.material.SpellCheckSuggestionsToolbar.anchor}
   /// The focal point below which the toolbar attempts to position itself.
@@ -38,6 +44,9 @@ class SpellCheckSuggestionsToolbar extends StatelessWidget {
   /// The [ContextMenuButtonItem]s that will be turned into the correct button
   /// widgets and displayed in the spell check suggestions toolbar.
   ///
+  /// Must not contain more than four items, typically three suggestions and a
+  /// delete button.
+  ///
   /// See also:
   ///
   ///  * [AdaptiveTextSelectionToolbar.buttonItems], the list of
@@ -52,13 +61,6 @@ class SpellCheckSuggestionsToolbar extends StatelessWidget {
   /// running Android API 31.
   static const double kToolbarContentDistanceBelow = TextSelectionToolbar.kHandleSize - 3.0;
 
-  /// Builds the default Android Material spell check suggestions toolbar.
-  static Widget _spellCheckSuggestionsToolbarBuilder(BuildContext context, Widget child) {
-    return _SpellCheckSuggestionsToolbarContainer(
-      child: child,
-    );
-  }
-
   /// Builds the button items for the toolbar based on the available
   /// spell check suggestions.
   static List<ContextMenuButtonItem>? buildButtonItems(
@@ -77,7 +79,7 @@ class SpellCheckSuggestionsToolbar extends StatelessWidget {
     final List<ContextMenuButtonItem> buttonItems = <ContextMenuButtonItem>[];
 
     // Build suggestion buttons.
-    for (final String suggestion in spanAtCursorIndex.suggestions) {
+    for (final String suggestion in spanAtCursorIndex.suggestions.take(_kMaxSuggestions)) {
       buttonItems.add(ContextMenuButtonItem(
         onPressed: () {
           if (!editableTextState.mounted) {
@@ -190,10 +192,10 @@ class SpellCheckSuggestionsToolbar extends StatelessWidget {
           // This duration was eyeballed on a Pixel 2 emulator running Android
           // API 28 for the Material TextSelectionToolbar.
           duration: const Duration(milliseconds: 140),
-          child: _spellCheckSuggestionsToolbarBuilder(context, _SpellCheckSuggestsionsToolbarItemsLayout(
+          child: _SpellCheckSuggestionsToolbarContainer(
             height: spellCheckSuggestionsToolbarHeight,
             children: <Widget>[..._buildToolbarButtons(context)],
-          )),
+          ),
         ),
       ),
     );
@@ -204,46 +206,30 @@ class SpellCheckSuggestionsToolbar extends StatelessWidget {
 /// toolbar.
 class _SpellCheckSuggestionsToolbarContainer extends StatelessWidget {
   const _SpellCheckSuggestionsToolbarContainer({
-    required this.child,
-  });
-
-  final Widget child;
-
-  @override
-  Widget build(BuildContext context) {
-    return Material(
-      // This elevation was eyeballed on a Pixel 4 emulator running Android
-      // API 31 for the SpellCheckSuggestionsToolbar.
-      elevation: 2.0,
-      type: MaterialType.card,
-      child: child,
-    );
-  }
-}
-
-/// Renders the spell check suggestions toolbar items in the correct positions
-/// in the menu.
-class _SpellCheckSuggestsionsToolbarItemsLayout extends StatelessWidget {
-  const _SpellCheckSuggestsionsToolbarItemsLayout({
     required this.height,
     required this.children,
   });
 
   final double height;
-
   final List<Widget> children;
 
   @override
   Widget build(BuildContext context) {
-    return SizedBox(
-      // This width was eyeballed on a Pixel 4 emulator running Android
+    return Material(
+      // This elevation was eyeballed on a Pixel 4 emulator running Android
       // API 31 for the SpellCheckSuggestionsToolbar.
-      width: 165,
-      height: height,
-      child: Column(
-        mainAxisSize: MainAxisSize.min,
-        crossAxisAlignment: CrossAxisAlignment.stretch,
-        children: children,
+      elevation: 2.0,
+      type: MaterialType.card,
+      child: SizedBox(
+        // This width was eyeballed on a Pixel 4 emulator running Android
+        // API 31 for the SpellCheckSuggestionsToolbar.
+        width: 165.0,
+        height: height,
+        child: Column(
+          mainAxisSize: MainAxisSize.min,
+          crossAxisAlignment: CrossAxisAlignment.stretch,
+          children: children,
+        ),
       ),
     );
   }

packages/flutter/test/cupertino/spell_check_suggestions_toolbar_test.dart

@@ -3,13 +3,64 @@
 // found in the LICENSE file.
 
 import 'package:flutter/cupertino.dart';
+import 'package:flutter/foundation.dart';
 import 'package:flutter/services.dart';
 
 import 'package:flutter_test/flutter_test.dart';
 
 void main() {
   TestWidgetsFlutterBinding.ensureInitialized();
 
+  testWidgets('more than three suggestions throws an error', (WidgetTester tester) async {
+    Future<void> pumpToolbar(List<String> suggestions) async {
+      await tester.pumpWidget(
+        CupertinoApp(
+          home: Center(
+            child: CupertinoSpellCheckSuggestionsToolbar(
+              anchors: const TextSelectionToolbarAnchors(
+                primaryAnchor: Offset.zero,
+              ),
+              buttonItems: suggestions.map((String string) {
+                return ContextMenuButtonItem(
+                  onPressed: () {},
+                  label: string,
+                );
+              }).toList(),
+            ),
+          ),
+        ),
+      );
+    }
+    await pumpToolbar(<String>['hello', 'yellow', 'yell']);
+    expect(() async {
+      await pumpToolbar(<String>['hello', 'yellow', 'yell', 'yeller']);
+    }, throwsAssertionError);
+  },
+    skip: kIsWeb, // [intended]
+  );
+
+  test('buildSuggestionButtons only considers the first three suggestions', () {
+    final _FakeEditableTextState editableTextState = _FakeEditableTextState(
+      suggestions: <String>[
+        'hello',
+        'yellow',
+        'yell',
+        'yeller',
+      ],
+    );
+    final List<ContextMenuButtonItem>? buttonItems =
+        CupertinoSpellCheckSuggestionsToolbar.buildButtonItems(editableTextState);
+    expect(buttonItems, isNotNull);
+    final Iterable<String?> labels = buttonItems!.map((ContextMenuButtonItem buttonItem) {
+      return buttonItem.label;
+    });
+    expect(labels, hasLength(3));
+    expect(labels, contains('hello'));
+    expect(labels, contains('yellow'));
+    expect(labels, contains('yell'));
+    expect(labels, isNot(contains('yeller')));
+  });
+
   testWidgets('buildButtonItems builds a "No Replacements Found" button when no suggestions', (WidgetTester tester) async {
     await tester.pumpWidget(
       CupertinoApp(
@@ -41,17 +92,22 @@ class _FakeEditableText extends EditableText {
 }
 
 class _FakeEditableTextState extends EditableTextState {
+  _FakeEditableTextState({
+    this.suggestions,
+  });
+
+  final List<String>? suggestions;
   @override
   TextEditingValue get currentTextEditingValue => TextEditingValue.empty;
 
   @override
   SuggestionSpan? findSuggestionSpanAtCursorIndex(int cursorIndex) {
-    return const SuggestionSpan(
-      TextRange(
+    return SuggestionSpan(
+      const TextRange(
         start: 0,
         end: 0,
       ),
-      <String>[],
+      suggestions ?? <String>[],
     );
   }
 }

packages/flutter/test/material/spell_check_suggestions_toolbar_test.dart

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 import 'package:flutter/cupertino.dart' show CupertinoTextSelectionToolbar;
+import 'package:flutter/foundation.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter/services.dart';
 import 'package:flutter_test/flutter_test.dart';
@@ -87,6 +88,50 @@ void main() {
     expect(toolbarY, equals(expectedToolbarY));
   });
 
+  testWidgets('more than three suggestions throws an error', (WidgetTester tester) async {
+    Future<void> pumpToolbar(List<String> suggestions) async {
+      await tester.pumpWidget(
+        MaterialApp(
+          home: Scaffold(
+            body: SpellCheckSuggestionsToolbar(
+              anchor: const Offset(0.0, _kAnchor - _kTestToolbarOverlap),
+              buttonItems: buildSuggestionButtons(suggestions),
+            ),
+          ),
+        ),
+      );
+    }
+    await pumpToolbar(<String>['hello', 'yellow', 'yell']);
+    expect(() async {
+      await pumpToolbar(<String>['hello', 'yellow', 'yell', 'yeller']);
+    }, throwsAssertionError);
+  },
+    skip: kIsWeb, // [intended]
+  );
+
+  test('buildSuggestionButtons only considers the first three suggestions', () {
+    final _FakeEditableTextState editableTextState = _FakeEditableTextState(
+      suggestions: <String>[
+        'hello',
+        'yellow',
+        'yell',
+        'yeller',
+      ],
+    );
+    final List<ContextMenuButtonItem>? buttonItems =
+        SpellCheckSuggestionsToolbar.buildButtonItems(editableTextState);
+    expect(buttonItems, isNotNull);
+    final Iterable<String?> labels = buttonItems!.map((ContextMenuButtonItem buttonItem) {
+      return buttonItem.label;
+    });
+    expect(labels, hasLength(4));
+    expect(labels, contains('hello'));
+    expect(labels, contains('yellow'));
+    expect(labels, contains('yell'));
+    expect(labels, contains(null)); // For the delete button.
+    expect(labels, isNot(contains('yeller')));
+  });
+
   test('buildButtonItems builds only a delete button when no suggestions', () {
     final _FakeEditableTextState editableTextState = _FakeEditableTextState();
     final List<ContextMenuButtonItem>? buttonItems =
@@ -98,17 +143,23 @@ void main() {
 }
 
 class _FakeEditableTextState extends EditableTextState {
+  _FakeEditableTextState({
+    this.suggestions,
+  });
+
+  final List<String>? suggestions;
+
   @override
   TextEditingValue get currentTextEditingValue => TextEditingValue.empty;
 
   @override
   SuggestionSpan? findSuggestionSpanAtCursorIndex(int cursorIndex) {
-    return const SuggestionSpan(
-      TextRange(
+    return SuggestionSpan(
+      const TextRange(
         start: 0,
         end: 0,
       ),
-      <String>[],
+      suggestions ?? <String>[],
     );
   }
 }