Unverified Commit 98aaf00a authored by Justin McCandless's avatar Justin McCandless Committed by GitHub

Fix the position of the Android-style spell check toolbar (#124897)

The spell check menu now appears directly below the misspelled word on Android.
parent 3ab8cd26
...@@ -10,7 +10,6 @@ import 'adaptive_text_selection_toolbar.dart'; ...@@ -10,7 +10,6 @@ import 'adaptive_text_selection_toolbar.dart';
import 'colors.dart'; import 'colors.dart';
import 'material.dart'; import 'material.dart';
import 'spell_check_suggestions_toolbar_layout_delegate.dart'; import 'spell_check_suggestions_toolbar_layout_delegate.dart';
import 'text_selection_toolbar.dart';
import 'text_selection_toolbar_text_button.dart'; import 'text_selection_toolbar_text_button.dart';
// The default height of the SpellCheckSuggestionsToolbar, which // The default height of the SpellCheckSuggestionsToolbar, which
...@@ -74,10 +73,6 @@ class SpellCheckSuggestionsToolbar extends StatelessWidget { ...@@ -74,10 +73,6 @@ class SpellCheckSuggestionsToolbar extends StatelessWidget {
/// suggestions toolbar. /// suggestions toolbar.
final List<ContextMenuButtonItem> buttonItems; final List<ContextMenuButtonItem> buttonItems;
/// Padding between the toolbar and the anchor. Eyeballed on Pixel 4 emulator
/// running Android API 31.
static const double kToolbarContentDistanceBelow = TextSelectionToolbar.kHandleSize - 3.0;
/// 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(
...@@ -153,6 +148,8 @@ class SpellCheckSuggestionsToolbar extends StatelessWidget { ...@@ -153,6 +148,8 @@ class SpellCheckSuggestionsToolbar extends StatelessWidget {
/// Determines the Offset that the toolbar will be anchored to. /// Determines the Offset that the toolbar will be anchored to.
static Offset getToolbarAnchor(TextSelectionToolbarAnchors anchors) { static Offset getToolbarAnchor(TextSelectionToolbarAnchors anchors) {
// Since this will be positioned below the anchor point, use the secondary
// anchor by default.
return anchors.secondaryAnchor == null ? anchors.primaryAnchor : anchors.secondaryAnchor!; return anchors.secondaryAnchor == null ? anchors.primaryAnchor : anchors.secondaryAnchor!;
} }
...@@ -190,24 +187,26 @@ class SpellCheckSuggestionsToolbar extends StatelessWidget { ...@@ -190,24 +187,26 @@ class SpellCheckSuggestionsToolbar extends StatelessWidget {
final double spellCheckSuggestionsToolbarHeight = final double spellCheckSuggestionsToolbarHeight =
_kDefaultToolbarHeight - (48.0 * (4 - buttonItems.length)); _kDefaultToolbarHeight - (48.0 * (4 - buttonItems.length));
// Incorporate the padding distance between the content and toolbar. // Incorporate the padding distance between the content and toolbar.
final Offset anchorPadded =
anchor + const Offset(0.0, kToolbarContentDistanceBelow);
final MediaQueryData mediaQueryData = MediaQuery.of(context); final MediaQueryData mediaQueryData = MediaQuery.of(context);
final double softKeyboardViewInsetsBottom = mediaQueryData.viewInsets.bottom; final double softKeyboardViewInsetsBottom = mediaQueryData.viewInsets.bottom;
final double paddingAbove = mediaQueryData.padding.top + CupertinoTextSelectionToolbar.kToolbarScreenPadding; final double paddingAbove = mediaQueryData.padding.top
+ CupertinoTextSelectionToolbar.kToolbarScreenPadding;
// Makes up for the Padding. // Makes up for the Padding.
final Offset localAdjustment = Offset(CupertinoTextSelectionToolbar.kToolbarScreenPadding, paddingAbove); final Offset localAdjustment = Offset(
CupertinoTextSelectionToolbar.kToolbarScreenPadding,
paddingAbove,
);
return Padding( return Padding(
padding: EdgeInsets.fromLTRB( padding: EdgeInsets.fromLTRB(
CupertinoTextSelectionToolbar.kToolbarScreenPadding, CupertinoTextSelectionToolbar.kToolbarScreenPadding,
kToolbarContentDistanceBelow, paddingAbove,
CupertinoTextSelectionToolbar.kToolbarScreenPadding, CupertinoTextSelectionToolbar.kToolbarScreenPadding,
CupertinoTextSelectionToolbar.kToolbarScreenPadding + softKeyboardViewInsetsBottom, CupertinoTextSelectionToolbar.kToolbarScreenPadding + softKeyboardViewInsetsBottom,
), ),
child: CustomSingleChildLayout( child: CustomSingleChildLayout(
delegate: SpellCheckSuggestionsToolbarLayoutDelegate( delegate: SpellCheckSuggestionsToolbarLayoutDelegate(
anchor: anchorPadded - localAdjustment, anchor: anchor - localAdjustment,
), ),
child: AnimatedSize( child: AnimatedSize(
// This duration was eyeballed on a Pixel 2 emulator running Android // This duration was eyeballed on a Pixel 2 emulator running Android
......
...@@ -3994,7 +3994,8 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien ...@@ -3994,7 +3994,8 @@ class EditableTextState extends State<EditableText> with AutomaticKeepAliveClien
|| platformNotSupported || platformNotSupported
|| widget.readOnly || widget.readOnly
|| _selectionOverlay == null || _selectionOverlay == null
|| !_spellCheckResultsReceived) { || !_spellCheckResultsReceived
|| findSuggestionSpanAtCursorIndex(textEditingValue.selection.extentOffset) == null) {
// Only attempt to show the spell check suggestions toolbar if there // Only attempt to show the spell check suggestions toolbar if there
// is a toolbar specified and spell check suggestions available to show. // is a toolbar specified and spell check suggestions available to show.
return false; return false;
......
...@@ -2210,12 +2210,12 @@ class TextSelectionGestureDetectorBuilder { ...@@ -2210,12 +2210,12 @@ class TextSelectionGestureDetectorBuilder {
// On desktop platforms the selection is set on tap down. // On desktop platforms the selection is set on tap down.
case TargetPlatform.android: case TargetPlatform.android:
editableText.hideToolbar(); editableText.hideToolbar();
editableText.showSpellCheckSuggestionsToolbar();
if (isShiftPressedValid) { if (isShiftPressedValid) {
_extendSelection(details.globalPosition, SelectionChangedCause.tap); _extendSelection(details.globalPosition, SelectionChangedCause.tap);
return; return;
} }
renderEditable.selectPosition(cause: SelectionChangedCause.tap); renderEditable.selectPosition(cause: SelectionChangedCause.tap);
editableText.showSpellCheckSuggestionsToolbar();
case TargetPlatform.fuchsia: case TargetPlatform.fuchsia:
editableText.hideToolbar(); editableText.hideToolbar();
if (isShiftPressedValid) { if (isShiftPressedValid) {
...@@ -2276,8 +2276,7 @@ class TextSelectionGestureDetectorBuilder { ...@@ -2276,8 +2276,7 @@ class TextSelectionGestureDetectorBuilder {
} else { } else {
editableText.toggleToolbar(false); editableText.toggleToolbar(false);
} }
} } else if (((_positionWasOnSelectionExclusive(textPosition) && !previousSelection.isCollapsed) || (_positionWasOnSelectionInclusive(textPosition) && previousSelection.isCollapsed && isAffinityTheSame)) && renderEditable.hasFocus) {
else if (((_positionWasOnSelectionExclusive(textPosition) && !previousSelection.isCollapsed) || (_positionWasOnSelectionInclusive(textPosition) && previousSelection.isCollapsed && isAffinityTheSame)) && renderEditable.hasFocus) {
editableText.toggleToolbar(false); editableText.toggleToolbar(false);
} else { } else {
renderEditable.selectWordEdge(cause: SelectionChangedCause.tap); renderEditable.selectWordEdge(cause: SelectionChangedCause.tap);
......
...@@ -2,7 +2,6 @@ ...@@ -2,7 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'package:flutter/cupertino.dart' show CupertinoTextSelectionToolbar;
import 'package:flutter/foundation.dart'; 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';
...@@ -50,9 +49,6 @@ void main() { ...@@ -50,9 +49,6 @@ void main() {
testWidgets('positions toolbar below anchor when it fits above bottom view padding', (WidgetTester tester) async { testWidgets('positions toolbar below anchor when it fits above bottom view padding', (WidgetTester tester) async {
// We expect the toolbar to be positioned right below the anchor with padding accounted for. // We expect the toolbar to be positioned right below the anchor with padding accounted for.
const double expectedToolbarY =
_kAnchor + (2 * SpellCheckSuggestionsToolbar.kToolbarContentDistanceBelow) - CupertinoTextSelectionToolbar.kToolbarScreenPadding;
await tester.pumpWidget( await tester.pumpWidget(
MaterialApp( MaterialApp(
home: Scaffold( home: Scaffold(
...@@ -65,13 +61,12 @@ void main() { ...@@ -65,13 +61,12 @@ void main() {
); );
final double toolbarY = tester.getTopLeft(findSpellCheckSuggestionsToolbar()).dy; final double toolbarY = tester.getTopLeft(findSpellCheckSuggestionsToolbar()).dy;
expect(toolbarY, equals(expectedToolbarY)); expect(toolbarY, equals(_kAnchor));
}); });
testWidgets('re-positions toolbar higher below anchor when it does not fit above bottom view padding', (WidgetTester tester) async { testWidgets('re-positions toolbar higher below anchor when it does not fit above bottom view padding', (WidgetTester tester) async {
// We expect the toolbar to be positioned _kTestToolbarOverlap pixels above the anchor with padding accounted for. // We expect the toolbar to be positioned _kTestToolbarOverlap pixels above the anchor.
const double expectedToolbarY = const double expectedToolbarY = _kAnchor - _kTestToolbarOverlap;
_kAnchor + (2 * SpellCheckSuggestionsToolbar.kToolbarContentDistanceBelow) - CupertinoTextSelectionToolbar.kToolbarScreenPadding - _kTestToolbarOverlap;
await tester.pumpWidget( await tester.pumpWidget(
MaterialApp( MaterialApp(
......
...@@ -15542,6 +15542,67 @@ testWidgets('Floating cursor ending with selection', (WidgetTester tester) async ...@@ -15542,6 +15542,67 @@ testWidgets('Floating cursor ending with selection', (WidgetTester tester) async
variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.iOS, TargetPlatform.android }), variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.iOS, TargetPlatform.android }),
skip: kIsWeb, // [intended] skip: kIsWeb, // [intended]
); );
testWidgets('tapping on a misspelled word hides the handles', (WidgetTester tester) async {
tester.binding.platformDispatcher.nativeSpellCheckServiceDefinedTestValue =
true;
controller.value = const TextEditingValue(
// All misspellings of "test". One the same length, one shorter, and one
// longer.
text: 'test test testt',
selection: TextSelection(affinity: TextAffinity.upstream, baseOffset: 0, extentOffset: 4),
);
await tester.pumpWidget(
MaterialApp(
home: EditableText(
backgroundCursorColor: Colors.grey,
controller: controller,
focusNode: focusNode,
style: textStyle,
cursorColor: cursorColor,
selectionControls: materialTextSelectionControls,
showSelectionHandles: true,
spellCheckConfiguration:
const SpellCheckConfiguration(
misspelledTextStyle: TextField.materialMisspelledTextStyle,
spellCheckSuggestionsToolbarBuilder: TextField.defaultSpellCheckSuggestionsToolbarBuilder,
),
),
),
);
final EditableTextState state =
tester.state<EditableTextState>(find.byType(EditableText));
state.spellCheckResults = SpellCheckResults(
controller.value.text,
const <SuggestionSpan>[
SuggestionSpan(TextRange(start: 10, end: 15), <String>['test']),
]);
await tester.tapAt(textOffsetToPosition(tester, 0));
await tester.pumpAndSettle();
expect(state.showSpellCheckSuggestionsToolbar(), isFalse);
await tester.pumpAndSettle();
expect(find.text('test'), findsNothing);
expect(state.selectionOverlay!.handlesAreVisible, isTrue);
await tester.tapAt(textOffsetToPosition(tester, 12));
await tester.pumpAndSettle();
expect(state.showSpellCheckSuggestionsToolbar(), isTrue);
await tester.pumpAndSettle();
expect(find.text('test'), findsOneWidget);
expect(state.selectionOverlay!.handlesAreVisible, isFalse);
await tester.tapAt(textOffsetToPosition(tester, 5));
await tester.pumpAndSettle();
expect(state.showSpellCheckSuggestionsToolbar(), isFalse);
await tester.pumpAndSettle();
expect(find.text('test'), findsNothing);
expect(state.selectionOverlay!.handlesAreVisible, isTrue);
},
variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.android }),
skip: kIsWeb, // [intended]
);
}); });
group('magnifier', () { group('magnifier', () {
......
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