From d015b60313d68a41375b05115f971fe787941c4c Mon Sep 17 00:00:00 2001 From: Renzo Olivares Date: Tue, 13 May 2025 11:10:21 -0700 Subject: [PATCH 1/5] Fix selection handles showing when using a mouse --- packages/flutter/lib/src/cupertino/text_field.dart | 5 +++-- packages/flutter/lib/src/material/text_field.dart | 5 +++-- .../flutter/lib/src/widgets/text_selection.dart | 14 ++++++++++++++ 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/packages/flutter/lib/src/cupertino/text_field.dart b/packages/flutter/lib/src/cupertino/text_field.dart index a8ea790a4503a..4c177660fc3bb 100644 --- a/packages/flutter/lib/src/cupertino/text_field.dart +++ b/packages/flutter/lib/src/cupertino/text_field.dart @@ -1185,8 +1185,9 @@ class _CupertinoTextFieldState extends State bool _shouldShowSelectionHandles(SelectionChangedCause? cause) { // When the text field is activated by something that doesn't trigger the - // selection overlay, we shouldn't show the handles either. - if (!_selectionGestureDetectorBuilder.shouldShowSelectionToolbar) { + // selection toolbar, we shouldn't show the handles either. + if (!_selectionGestureDetectorBuilder.shouldShowSelectionToolbar || + !_selectionGestureDetectorBuilder.shouldShowSelectionHandles) { return false; } diff --git a/packages/flutter/lib/src/material/text_field.dart b/packages/flutter/lib/src/material/text_field.dart index fdb6b7375ff1b..3057f700cd495 100644 --- a/packages/flutter/lib/src/material/text_field.dart +++ b/packages/flutter/lib/src/material/text_field.dart @@ -1341,8 +1341,9 @@ class _TextFieldState extends State bool _shouldShowSelectionHandles(SelectionChangedCause? cause) { // When the text field is activated by something that doesn't trigger the - // selection overlay, we shouldn't show the handles either. - if (!_selectionGestureDetectorBuilder.shouldShowSelectionToolbar) { + // selection toolbar, we shouldn't show the handles either. + if (!_selectionGestureDetectorBuilder.shouldShowSelectionToolbar || + !_selectionGestureDetectorBuilder.shouldShowSelectionHandles) { return false; } diff --git a/packages/flutter/lib/src/widgets/text_selection.dart b/packages/flutter/lib/src/widgets/text_selection.dart index 1703359c72d30..b3f78c4c28c4d 100644 --- a/packages/flutter/lib/src/widgets/text_selection.dart +++ b/packages/flutter/lib/src/widgets/text_selection.dart @@ -2170,6 +2170,14 @@ class TextSelectionGestureDetectorBuilder { bool get shouldShowSelectionToolbar => _shouldShowSelectionToolbar; bool _shouldShowSelectionToolbar = true; + /// Whether to show the selection handles. + /// + /// It is based on the signal source when a [onTapDown] is called. This getter + /// will return true if current [onTapDown] event is triggered by a touch or + /// a stylus. + bool get shouldShowSelectionHandles => _shouldShowSelectionHandles; + bool _shouldShowSelectionHandles = true; + /// The [State] of the [EditableText] for which the builder will provide a /// [TextSelectionGestureDetector]. @protected @@ -2277,6 +2285,7 @@ class TextSelectionGestureDetectorBuilder { // https://github.com/flutter/flutter/issues/106586 _shouldShowSelectionToolbar = kind == null || kind == PointerDeviceKind.touch || kind == PointerDeviceKind.stylus; + _shouldShowSelectionHandles = _shouldShowSelectionToolbar; // It is impossible to extend the selection when the shift key is pressed, if the // renderEditable.selection is invalid. @@ -2727,6 +2736,10 @@ class TextSelectionGestureDetectorBuilder { // See https://github.com/flutter/flutter/issues/115130. renderEditable.handleSecondaryTapDown(TapDownDetails(globalPosition: details.globalPosition)); _shouldShowSelectionToolbar = true; + _shouldShowSelectionHandles = + details.kind == null || + details.kind == PointerDeviceKind.touch || + details.kind == PointerDeviceKind.stylus; } /// Handler for [TextSelectionGestureDetector.onDoubleTapDown]. @@ -2864,6 +2877,7 @@ class TextSelectionGestureDetectorBuilder { final PointerDeviceKind? kind = details.kind; _shouldShowSelectionToolbar = kind == null || kind == PointerDeviceKind.touch || kind == PointerDeviceKind.stylus; + _shouldShowSelectionHandles = _shouldShowSelectionToolbar; _dragStartSelection = renderEditable.selection; _dragStartScrollOffset = _scrollPosition; From ce510262544ade10e9ce288c569f0d213d598c1d Mon Sep 17 00:00:00 2001 From: Renzo Olivares Date: Tue, 13 May 2025 11:52:02 -0700 Subject: [PATCH 2/5] add test --- .../test/material/text_field_test.dart | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/packages/flutter/test/material/text_field_test.dart b/packages/flutter/test/material/text_field_test.dart index 3a5d8c08e7719..a6eae65d41250 100644 --- a/packages/flutter/test/material/text_field_test.dart +++ b/packages/flutter/test/material/text_field_test.dart @@ -15917,6 +15917,66 @@ void main() { skip: isContextMenuProvidedByPlatform, ); + testWidgets( + 'Selection handles should not show when using a mouse on non-Apple platforms', + (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/pull/168252. + final TextEditingController controller = _textEditingController(text: 'blah1 blah2'); + await tester.pumpWidget( + MaterialApp(home: Material(child: TextField(controller: controller))), + ); + + // Initially, the menu is not shown and there is no selection. + expectNoMaterialToolbar(); + expect(controller.selection, const TextSelection(baseOffset: -1, extentOffset: -1)); + + final Offset secondBlah = textOffsetToPosition(tester, 8); + + // Right click the second word using a mouse. + final TestGesture gesture = await tester.startGesture( + secondBlah, + kind: PointerDeviceKind.mouse, + buttons: kSecondaryMouseButton, + ); + await tester.pump(); + await gesture.up(); + await tester.pumpAndSettle(); + + switch (defaultTargetPlatform) { + case TargetPlatform.iOS: + case TargetPlatform.macOS: + return; + case TargetPlatform.android: + case TargetPlatform.fuchsia: + case TargetPlatform.linux: + case TargetPlatform.windows: + expect(controller.selection, const TextSelection.collapsed(offset: 8)); + expect(find.text('Cut'), findsNothing); + expect(find.text('Copy'), findsNothing); + expect(find.text('Paste'), findsOneWidget); + expect(find.text('Select all'), findsOneWidget); + } + + // Press select all. + await tester.tap(find.text('Select all'), kind: PointerDeviceKind.mouse); + await tester.pumpAndSettle(); + expect(controller.selection, const TextSelection(baseOffset: 0, extentOffset: 11)); + + // Selection handles are hidden. + final EditableTextState state = tester.state(find.byType(EditableText)); + expect(state.selectionOverlay, isNotNull); + expect(state.selectionOverlay!.handlesAreVisible, isFalse); + }, + variant: const TargetPlatformVariant({ + TargetPlatform.android, + TargetPlatform.fuchsia, + TargetPlatform.linux, + TargetPlatform.windows, + }), + // [intended] only applies to platforms where we supply the context menu. + skip: isContextMenuProvidedByPlatform, + ); + testWidgets('Cannot request focus when canRequestFocus is false', (WidgetTester tester) async { final FocusNode focusNode = _focusNode(); From 920a3813f8837f2d86faefdec82df6541a447f83 Mon Sep 17 00:00:00 2001 From: Renzo Olivares Date: Wed, 14 May 2025 11:47:14 -0700 Subject: [PATCH 3/5] Add more tests --- .../lib/src/widgets/text_selection.dart | 1 + .../test/cupertino/text_field_test.dart | 112 ++++++++++++++++++ .../test/material/text_field_test.dart | 50 ++++++++ 3 files changed, 163 insertions(+) diff --git a/packages/flutter/lib/src/widgets/text_selection.dart b/packages/flutter/lib/src/widgets/text_selection.dart index b3f78c4c28c4d..52d7c50e7716a 100644 --- a/packages/flutter/lib/src/widgets/text_selection.dart +++ b/packages/flutter/lib/src/widgets/text_selection.dart @@ -2467,6 +2467,7 @@ class TextSelectionGestureDetectorBuilder { // Precise devices should place the cursor at a precise position if the // word at the text position is not misspelled. renderEditable.selectPosition(cause: SelectionChangedCause.tap); + editableText.hideToolbar(); case PointerDeviceKind.touch: case PointerDeviceKind.unknown: // If the word that was tapped is misspelled, select the word and show the spell check suggestions diff --git a/packages/flutter/test/cupertino/text_field_test.dart b/packages/flutter/test/cupertino/text_field_test.dart index 9f59bf3f7d75f..02a60d72b4e07 100644 --- a/packages/flutter/test/cupertino/text_field_test.dart +++ b/packages/flutter/test/cupertino/text_field_test.dart @@ -8642,6 +8642,118 @@ void main() { skip: isContextMenuProvidedByPlatform, ); + testWidgets( + 'Selection handles should not show when using a mouse on non-Apple platforms', + (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/pull/168252. + final TextEditingController controller = TextEditingController(text: 'blah1 blah2'); + addTearDown(controller.dispose); + await tester.pumpWidget( + CupertinoApp(home: Center(child: CupertinoTextField(controller: controller))), + ); + + // Initially, the menu is not shown and there is no selection. + expectNoCupertinoToolbar(); + expect(controller.selection, const TextSelection(baseOffset: -1, extentOffset: -1)); + + final Offset secondBlah = textOffsetToPosition(tester, 8); + + // Right click the second word using a mouse. + final TestGesture gesture = await tester.startGesture( + secondBlah, + kind: PointerDeviceKind.mouse, + buttons: kSecondaryMouseButton, + ); + await tester.pump(); + await gesture.up(); + await tester.pumpAndSettle(); + + switch (defaultTargetPlatform) { + case TargetPlatform.iOS: + case TargetPlatform.macOS: + return; + case TargetPlatform.android: + case TargetPlatform.fuchsia: + case TargetPlatform.linux: + case TargetPlatform.windows: + expect(controller.selection, const TextSelection.collapsed(offset: 8)); + expect(find.text('Cut'), findsNothing); + expect(find.text('Copy'), findsNothing); + expect(find.text('Paste'), findsOneWidget); + expect(find.text('Select All'), findsOneWidget); + } + + // Press select all. + await tester.tap(find.text('Select All'), kind: PointerDeviceKind.mouse); + await tester.pumpAndSettle(); + expect(controller.selection, const TextSelection(baseOffset: 0, extentOffset: 11)); + + // Selection handles are hidden. + final EditableTextState state = tester.state(find.byType(EditableText)); + expect(state.selectionOverlay, isNotNull); + expect(state.selectionOverlay!.handlesAreVisible, isFalse); + }, + variant: const TargetPlatformVariant({ + TargetPlatform.android, + TargetPlatform.fuchsia, + TargetPlatform.linux, + TargetPlatform.windows, + }), + // [intended] only applies to platforms where we supply the context menu. + skip: isContextMenuProvidedByPlatform, + ); + + testWidgets( + 'Selection handles should not show when using a mouse on Apple platforms using Flutter context menu', + (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/pull/168252. + final TextEditingController controller = TextEditingController(text: 'blah1 blah2'); + addTearDown(controller.dispose); + await tester.pumpWidget( + CupertinoApp(home: Center(child: CupertinoTextField(controller: controller))), + ); + + // Initially, the menu is not shown and there is no selection. + expectNoCupertinoToolbar(); + expect(controller.selection, const TextSelection(baseOffset: -1, extentOffset: -1)); + + final Offset firstBlah = textOffsetToPosition(tester, 5); + + // Click at the end of blah1. + await tester.tapAt(firstBlah, kind: PointerDeviceKind.mouse); + await tester.pumpAndSettle(); + + // Right click the same position to reveal the context menu. + await tester.tapAt(firstBlah, kind: PointerDeviceKind.mouse, buttons: kSecondaryMouseButton); + await tester.pumpAndSettle(); + + switch (defaultTargetPlatform) { + case TargetPlatform.iOS: + case TargetPlatform.macOS: + expect(controller.selection, const TextSelection.collapsed(offset: 5)); + expectCupertinoToolbarForCollapsedSelection(); + case TargetPlatform.android: + case TargetPlatform.fuchsia: + case TargetPlatform.linux: + case TargetPlatform.windows: + return; + } + + // Press select all. + await tester.tap(find.text('Select All'), kind: PointerDeviceKind.mouse); + await tester.pumpAndSettle(); + expect(controller.selection, const TextSelection(baseOffset: 0, extentOffset: 11)); + + // Selection handles are hidden. + final EditableTextState state = tester.state(find.byType(EditableText)); + expect(state.selectionOverlay, isNotNull); + expect(state.selectionOverlay!.handlesAreVisible, isFalse); + }, + variant: TargetPlatformVariant.only(TargetPlatform.iOS), + // [intended] only applies to platforms where we supply the context menu. + skip: isContextMenuProvidedByPlatform, + ); + group('Right click focus', () { testWidgets('Can right click to focus multiple times', (WidgetTester tester) async { // Regression test for https://github.com/flutter/flutter/pull/103228 diff --git a/packages/flutter/test/material/text_field_test.dart b/packages/flutter/test/material/text_field_test.dart index a6eae65d41250..1c8730b29df67 100644 --- a/packages/flutter/test/material/text_field_test.dart +++ b/packages/flutter/test/material/text_field_test.dart @@ -15977,6 +15977,56 @@ void main() { skip: isContextMenuProvidedByPlatform, ); + testWidgets( + 'Selection handles should not show when using a mouse on Apple platforms using Flutter context menu', + (WidgetTester tester) async { + // Regression test for https://github.com/flutter/flutter/pull/168252. + final TextEditingController controller = _textEditingController(text: 'blah1 blah2'); + await tester.pumpWidget( + MaterialApp(home: Material(child: TextField(controller: controller))), + ); + + // Initially, the menu is not shown and there is no selection. + expectNoCupertinoToolbar(); + expect(controller.selection, const TextSelection(baseOffset: -1, extentOffset: -1)); + + final Offset firstBlah = textOffsetToPosition(tester, 5); + + // Click at the end of blah1. + await tester.tapAt(firstBlah, kind: PointerDeviceKind.mouse); + await tester.pumpAndSettle(); + + // Right click the same position to reveal the context menu. + await tester.tapAt(firstBlah, kind: PointerDeviceKind.mouse, buttons: kSecondaryMouseButton); + await tester.pumpAndSettle(); + + switch (defaultTargetPlatform) { + case TargetPlatform.iOS: + case TargetPlatform.macOS: + expect(controller.selection, const TextSelection.collapsed(offset: 5)); + expectCupertinoToolbarForCollapsedSelection(); + case TargetPlatform.android: + case TargetPlatform.fuchsia: + case TargetPlatform.linux: + case TargetPlatform.windows: + return; + } + + // Press select all. + await tester.tap(find.text('Select All'), kind: PointerDeviceKind.mouse); + await tester.pumpAndSettle(); + expect(controller.selection, const TextSelection(baseOffset: 0, extentOffset: 11)); + + // Selection handles are hidden. + final EditableTextState state = tester.state(find.byType(EditableText)); + expect(state.selectionOverlay, isNotNull); + expect(state.selectionOverlay!.handlesAreVisible, isFalse); + }, + variant: TargetPlatformVariant.only(TargetPlatform.iOS), + // [intended] only applies to platforms where we supply the context menu. + skip: isContextMenuProvidedByPlatform, + ); + testWidgets('Cannot request focus when canRequestFocus is false', (WidgetTester tester) async { final FocusNode focusNode = _focusNode(); From 6ea10ba94eae783e499aaa3c7a67e62ea6df717c Mon Sep 17 00:00:00 2001 From: Renzo Olivares Date: Wed, 14 May 2025 12:30:20 -0700 Subject: [PATCH 4/5] update docs --- .../flutter/lib/src/widgets/text_selection.dart | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/flutter/lib/src/widgets/text_selection.dart b/packages/flutter/lib/src/widgets/text_selection.dart index 52d7c50e7716a..0234d14a5351f 100644 --- a/packages/flutter/lib/src/widgets/text_selection.dart +++ b/packages/flutter/lib/src/widgets/text_selection.dart @@ -2164,17 +2164,20 @@ class TextSelectionGestureDetectorBuilder { /// Whether to show the selection toolbar. /// - /// It is based on the signal source when a [onTapDown] is called. This getter - /// will return true if current [onTapDown] event is triggered by a touch or - /// a stylus. + /// It is based on the signal source when [onTapDown], [onSecondaryTapDown], + /// [onDragSelectionStart], or [onForcePressStart] is called. This getter + /// will return true if the current [onTapDown], or [onDragSelectionStart] event + /// is triggered by a touch or a stylus. It will always return true for the + /// current [onSecondaryTapDown] or [onForcePressStart] event. bool get shouldShowSelectionToolbar => _shouldShowSelectionToolbar; bool _shouldShowSelectionToolbar = true; /// Whether to show the selection handles. /// - /// It is based on the signal source when a [onTapDown] is called. This getter - /// will return true if current [onTapDown] event is triggered by a touch or - /// a stylus. + /// It is based on the signal source when [onTapDown], [onSecondaryTapDown], + /// [onDragSelectionStart], is called. This getter will return true if the + /// current [onTapDown], [onSecondaryTapDown], or [onDragSelectionStart] event + /// is triggered by a touch or a stylus. bool get shouldShowSelectionHandles => _shouldShowSelectionHandles; bool _shouldShowSelectionHandles = true; From cdfb4b24af03cc8493143a0cc892e441ad585665 Mon Sep 17 00:00:00 2001 From: Renzo Olivares Date: Wed, 14 May 2025 13:04:12 -0700 Subject: [PATCH 5/5] address reviewer comments --- .../flutter/test/cupertino/text_field_test.dart | 14 ++------------ .../flutter/test/material/text_field_test.dart | 14 ++------------ 2 files changed, 4 insertions(+), 24 deletions(-) diff --git a/packages/flutter/test/cupertino/text_field_test.dart b/packages/flutter/test/cupertino/text_field_test.dart index 02a60d72b4e07..23c082aa741f6 100644 --- a/packages/flutter/test/cupertino/text_field_test.dart +++ b/packages/flutter/test/cupertino/text_field_test.dart @@ -8726,18 +8726,8 @@ void main() { // Right click the same position to reveal the context menu. await tester.tapAt(firstBlah, kind: PointerDeviceKind.mouse, buttons: kSecondaryMouseButton); await tester.pumpAndSettle(); - - switch (defaultTargetPlatform) { - case TargetPlatform.iOS: - case TargetPlatform.macOS: - expect(controller.selection, const TextSelection.collapsed(offset: 5)); - expectCupertinoToolbarForCollapsedSelection(); - case TargetPlatform.android: - case TargetPlatform.fuchsia: - case TargetPlatform.linux: - case TargetPlatform.windows: - return; - } + expect(controller.selection, const TextSelection.collapsed(offset: 5)); + expectCupertinoToolbarForCollapsedSelection(); // Press select all. await tester.tap(find.text('Select All'), kind: PointerDeviceKind.mouse); diff --git a/packages/flutter/test/material/text_field_test.dart b/packages/flutter/test/material/text_field_test.dart index 1c8730b29df67..8c360947c595b 100644 --- a/packages/flutter/test/material/text_field_test.dart +++ b/packages/flutter/test/material/text_field_test.dart @@ -15999,18 +15999,8 @@ void main() { // Right click the same position to reveal the context menu. await tester.tapAt(firstBlah, kind: PointerDeviceKind.mouse, buttons: kSecondaryMouseButton); await tester.pumpAndSettle(); - - switch (defaultTargetPlatform) { - case TargetPlatform.iOS: - case TargetPlatform.macOS: - expect(controller.selection, const TextSelection.collapsed(offset: 5)); - expectCupertinoToolbarForCollapsedSelection(); - case TargetPlatform.android: - case TargetPlatform.fuchsia: - case TargetPlatform.linux: - case TargetPlatform.windows: - return; - } + expect(controller.selection, const TextSelection.collapsed(offset: 5)); + expectCupertinoToolbarForCollapsedSelection(); // Press select all. await tester.tap(find.text('Select All'), kind: PointerDeviceKind.mouse);