Unverified Commit f04a5afb authored by Justin McCandless's avatar Justin McCandless Committed by GitHub

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

Fixes a bug where the spell check menu could overflow.
parent 0ea2f3b5
...@@ -12,7 +12,7 @@ import 'text_selection_toolbar.dart'; ...@@ -12,7 +12,7 @@ import 'text_selection_toolbar.dart';
import 'text_selection_toolbar_button.dart'; import 'text_selection_toolbar_button.dart';
/// iOS only shows 3 spell check suggestions in the toolbar. /// 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. /// The default spell check suggestions toolbar for iOS.
/// ///
...@@ -20,11 +20,13 @@ const int _maxSuggestions = 3; ...@@ -20,11 +20,13 @@ const int _maxSuggestions = 3;
/// readjusts to fit above bottom view insets. /// readjusts to fit above bottom view insets.
class CupertinoSpellCheckSuggestionsToolbar extends StatelessWidget { class CupertinoSpellCheckSuggestionsToolbar extends StatelessWidget {
/// Constructs a [CupertinoSpellCheckSuggestionsToolbar]. /// Constructs a [CupertinoSpellCheckSuggestionsToolbar].
///
/// [buttonItems] must not contain more than three items.
const CupertinoSpellCheckSuggestionsToolbar({ const CupertinoSpellCheckSuggestionsToolbar({
super.key, super.key,
required this.anchors, required this.anchors,
required this.buttonItems, required this.buttonItems,
}); }) : assert(buttonItems.length <= _kMaxSuggestions);
/// The location on which to anchor the menu. /// The location on which to anchor the menu.
final TextSelectionToolbarAnchors anchors; final TextSelectionToolbarAnchors anchors;
...@@ -32,6 +34,8 @@ class CupertinoSpellCheckSuggestionsToolbar extends StatelessWidget { ...@@ -32,6 +34,8 @@ class CupertinoSpellCheckSuggestionsToolbar extends StatelessWidget {
/// The [ContextMenuButtonItem]s that will be turned into the correct button /// The [ContextMenuButtonItem]s that will be turned into the correct button
/// widgets and displayed in the spell check suggestions toolbar. /// widgets and displayed in the spell check suggestions toolbar.
/// ///
/// Must not contain more than three items.
///
/// See also: /// See also:
/// ///
/// * [AdaptiveTextSelectionToolbar.buttonItems], the list of /// * [AdaptiveTextSelectionToolbar.buttonItems], the list of
...@@ -71,11 +75,7 @@ class CupertinoSpellCheckSuggestionsToolbar extends StatelessWidget { ...@@ -71,11 +75,7 @@ class CupertinoSpellCheckSuggestionsToolbar extends StatelessWidget {
final List<ContextMenuButtonItem> buttonItems = <ContextMenuButtonItem>[]; final List<ContextMenuButtonItem> buttonItems = <ContextMenuButtonItem>[];
// Build suggestion buttons. // Build suggestion buttons.
int suggestionCount = 0; for (final String suggestion in spanAtCursorIndex.suggestions.take(_kMaxSuggestions)) {
for (final String suggestion in spanAtCursorIndex.suggestions) {
if (suggestionCount >= _maxSuggestions) {
break;
}
buttonItems.add(ContextMenuButtonItem( buttonItems.add(ContextMenuButtonItem(
onPressed: () { onPressed: () {
if (!editableTextState.mounted) { if (!editableTextState.mounted) {
...@@ -89,7 +89,6 @@ class CupertinoSpellCheckSuggestionsToolbar extends StatelessWidget { ...@@ -89,7 +89,6 @@ class CupertinoSpellCheckSuggestionsToolbar extends StatelessWidget {
}, },
label: suggestion, label: suggestion,
)); ));
suggestionCount += 1;
} }
return buttonItems; return buttonItems;
} }
......
...@@ -18,17 +18,23 @@ import 'text_selection_toolbar_text_button.dart'; ...@@ -18,17 +18,23 @@ import 'text_selection_toolbar_text_button.dart';
// Size eyeballed on Pixel 4 emulator running Android API 31. // Size eyeballed on Pixel 4 emulator running Android API 31.
const double _kDefaultToolbarHeight = 193.0; 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. /// The default spell check suggestions toolbar for Android.
/// ///
/// Tries to position itself below the [anchor], but if it doesn't fit, then it /// Tries to position itself below the [anchor], but if it doesn't fit, then it
/// readjusts to fit above bottom view insets. /// readjusts to fit above bottom view insets.
class SpellCheckSuggestionsToolbar extends StatelessWidget { class SpellCheckSuggestionsToolbar extends StatelessWidget {
/// Constructs a [SpellCheckSuggestionsToolbar]. /// Constructs a [SpellCheckSuggestionsToolbar].
///
/// [buttonItems] must not contain more than four items, generally three
/// suggestions and one delete button.
const SpellCheckSuggestionsToolbar({ const SpellCheckSuggestionsToolbar({
super.key, super.key,
required this.anchor, required this.anchor,
required this.buttonItems, required this.buttonItems,
}); }) : assert(buttonItems.length <= _kMaxSuggestions + 1);
/// {@template flutter.material.SpellCheckSuggestionsToolbar.anchor} /// {@template flutter.material.SpellCheckSuggestionsToolbar.anchor}
/// The focal point below which the toolbar attempts to position itself. /// The focal point below which the toolbar attempts to position itself.
...@@ -38,6 +44,9 @@ class SpellCheckSuggestionsToolbar extends StatelessWidget { ...@@ -38,6 +44,9 @@ class SpellCheckSuggestionsToolbar extends StatelessWidget {
/// The [ContextMenuButtonItem]s that will be turned into the correct button /// The [ContextMenuButtonItem]s that will be turned into the correct button
/// widgets and displayed in the spell check suggestions toolbar. /// 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: /// See also:
/// ///
/// * [AdaptiveTextSelectionToolbar.buttonItems], the list of /// * [AdaptiveTextSelectionToolbar.buttonItems], the list of
...@@ -52,13 +61,6 @@ class SpellCheckSuggestionsToolbar extends StatelessWidget { ...@@ -52,13 +61,6 @@ class SpellCheckSuggestionsToolbar extends StatelessWidget {
/// running Android API 31. /// running Android API 31.
static const double kToolbarContentDistanceBelow = TextSelectionToolbar.kHandleSize - 3.0; 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 /// Builds the button items for the toolbar based on the available
/// spell check suggestions. /// spell check suggestions.
static List<ContextMenuButtonItem>? buildButtonItems( static List<ContextMenuButtonItem>? buildButtonItems(
...@@ -77,7 +79,7 @@ class SpellCheckSuggestionsToolbar extends StatelessWidget { ...@@ -77,7 +79,7 @@ class SpellCheckSuggestionsToolbar extends StatelessWidget {
final List<ContextMenuButtonItem> buttonItems = <ContextMenuButtonItem>[]; final List<ContextMenuButtonItem> buttonItems = <ContextMenuButtonItem>[];
// Build suggestion buttons. // Build suggestion buttons.
for (final String suggestion in spanAtCursorIndex.suggestions) { for (final String suggestion in spanAtCursorIndex.suggestions.take(_kMaxSuggestions)) {
buttonItems.add(ContextMenuButtonItem( buttonItems.add(ContextMenuButtonItem(
onPressed: () { onPressed: () {
if (!editableTextState.mounted) { if (!editableTextState.mounted) {
...@@ -190,10 +192,10 @@ class SpellCheckSuggestionsToolbar extends StatelessWidget { ...@@ -190,10 +192,10 @@ class SpellCheckSuggestionsToolbar extends StatelessWidget {
// This duration was eyeballed on a Pixel 2 emulator running Android // This duration was eyeballed on a Pixel 2 emulator running Android
// API 28 for the Material TextSelectionToolbar. // API 28 for the Material TextSelectionToolbar.
duration: const Duration(milliseconds: 140), duration: const Duration(milliseconds: 140),
child: _spellCheckSuggestionsToolbarBuilder(context, _SpellCheckSuggestsionsToolbarItemsLayout( child: _SpellCheckSuggestionsToolbarContainer(
height: spellCheckSuggestionsToolbarHeight, height: spellCheckSuggestionsToolbarHeight,
children: <Widget>[..._buildToolbarButtons(context)], children: <Widget>[..._buildToolbarButtons(context)],
)), ),
), ),
), ),
); );
...@@ -204,46 +206,30 @@ class SpellCheckSuggestionsToolbar extends StatelessWidget { ...@@ -204,46 +206,30 @@ class SpellCheckSuggestionsToolbar extends StatelessWidget {
/// toolbar. /// toolbar.
class _SpellCheckSuggestionsToolbarContainer extends StatelessWidget { class _SpellCheckSuggestionsToolbarContainer extends StatelessWidget {
const _SpellCheckSuggestionsToolbarContainer({ 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.height,
required this.children, required this.children,
}); });
final double height; final double height;
final List<Widget> children; final List<Widget> children;
@override @override
Widget build(BuildContext context) { Widget build(BuildContext context) {
return SizedBox( return Material(
// This width was eyeballed on a Pixel 4 emulator running Android // This elevation was eyeballed on a Pixel 4 emulator running Android
// API 31 for the SpellCheckSuggestionsToolbar. // API 31 for the SpellCheckSuggestionsToolbar.
width: 165, elevation: 2.0,
height: height, type: MaterialType.card,
child: Column( child: SizedBox(
mainAxisSize: MainAxisSize.min, // This width was eyeballed on a Pixel 4 emulator running Android
crossAxisAlignment: CrossAxisAlignment.stretch, // API 31 for the SpellCheckSuggestionsToolbar.
children: children, width: 165.0,
height: height,
child: Column(
mainAxisSize: MainAxisSize.min,
crossAxisAlignment: CrossAxisAlignment.stretch,
children: children,
),
), ),
); );
} }
......
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
// found in the LICENSE file. // found in the LICENSE file.
import 'package:flutter/cupertino.dart'; import 'package:flutter/cupertino.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter/services.dart'; import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
...@@ -10,6 +11,56 @@ import 'package:flutter_test/flutter_test.dart'; ...@@ -10,6 +11,56 @@ import 'package:flutter_test/flutter_test.dart';
void main() { void main() {
TestWidgetsFlutterBinding.ensureInitialized(); 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 { testWidgets('buildButtonItems builds a "No Replacements Found" button when no suggestions', (WidgetTester tester) async {
await tester.pumpWidget( await tester.pumpWidget(
CupertinoApp( CupertinoApp(
...@@ -41,17 +92,22 @@ class _FakeEditableText extends EditableText { ...@@ -41,17 +92,22 @@ class _FakeEditableText extends EditableText {
} }
class _FakeEditableTextState extends EditableTextState { class _FakeEditableTextState extends EditableTextState {
_FakeEditableTextState({
this.suggestions,
});
final List<String>? suggestions;
@override @override
TextEditingValue get currentTextEditingValue => TextEditingValue.empty; TextEditingValue get currentTextEditingValue => TextEditingValue.empty;
@override @override
SuggestionSpan? findSuggestionSpanAtCursorIndex(int cursorIndex) { SuggestionSpan? findSuggestionSpanAtCursorIndex(int cursorIndex) {
return const SuggestionSpan( return SuggestionSpan(
TextRange( const TextRange(
start: 0, start: 0,
end: 0, end: 0,
), ),
<String>[], suggestions ?? <String>[],
); );
} }
} }
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
// found in the LICENSE file. // found in the LICENSE file.
import 'package:flutter/cupertino.dart' show CupertinoTextSelectionToolbar; import 'package:flutter/cupertino.dart' show CupertinoTextSelectionToolbar;
import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart'; import 'package:flutter/material.dart';
import 'package:flutter/services.dart'; import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
...@@ -87,6 +88,50 @@ void main() { ...@@ -87,6 +88,50 @@ void main() {
expect(toolbarY, equals(expectedToolbarY)); 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', () { test('buildButtonItems builds only a delete button when no suggestions', () {
final _FakeEditableTextState editableTextState = _FakeEditableTextState(); final _FakeEditableTextState editableTextState = _FakeEditableTextState();
final List<ContextMenuButtonItem>? buttonItems = final List<ContextMenuButtonItem>? buttonItems =
...@@ -98,17 +143,23 @@ void main() { ...@@ -98,17 +143,23 @@ void main() {
} }
class _FakeEditableTextState extends EditableTextState { class _FakeEditableTextState extends EditableTextState {
_FakeEditableTextState({
this.suggestions,
});
final List<String>? suggestions;
@override @override
TextEditingValue get currentTextEditingValue => TextEditingValue.empty; TextEditingValue get currentTextEditingValue => TextEditingValue.empty;
@override @override
SuggestionSpan? findSuggestionSpanAtCursorIndex(int cursorIndex) { SuggestionSpan? findSuggestionSpanAtCursorIndex(int cursorIndex) {
return const SuggestionSpan( return SuggestionSpan(
TextRange( const TextRange(
start: 0, start: 0,
end: 0, end: 0,
), ),
<String>[], suggestions ?? <String>[],
); );
} }
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment