Repository navigation
Fix selection handles showing when using a mouse #168751
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2164,12 +2164,23 @@ 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 [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; | ||
|
|
||
| /// The [State] of the [EditableText] for which the builder will provide a | ||
| /// [TextSelectionGestureDetector]. | ||
| @protected | ||
|
|
@@ -2277,6 +2288,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. | ||
|
|
@@ -2458,6 +2470,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 | ||
|
|
@@ -2727,6 +2740,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; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I double checked this on native Android and indeed the selection handles show when using a stylus. Good call. |
||
| } | ||
|
|
||
| /// Handler for [TextSelectionGestureDetector.onDoubleTapDown]. | ||
|
|
@@ -2864,6 +2881,7 @@ class TextSelectionGestureDetectorBuilder { | |
| final PointerDeviceKind? kind = details.kind; | ||
| _shouldShowSelectionToolbar = | ||
| kind == null || kind == PointerDeviceKind.touch || kind == PointerDeviceKind.stylus; | ||
| _shouldShowSelectionHandles = _shouldShowSelectionToolbar; | ||
|
|
||
| _dragStartSelection = renderEditable.selection; | ||
| _dragStartScrollOffset = _scrollPosition; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8642,6 +8642,108 @@ 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<EditableTextState>(find.byType(EditableText)); | ||
| expect(state.selectionOverlay, isNotNull); | ||
| expect(state.selectionOverlay!.handlesAreVisible, isFalse); | ||
| }, | ||
| variant: const TargetPlatformVariant(<TargetPlatform>{ | ||
| 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', | ||
|
Comment on lines
+8706
to
+8707
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this test separate from the previous test because iOS uses the system context menu?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No its mostly separate because of cupertino vs material toolbar. I don't think we can test this specific scenario using the system rendered context menu on iOS since from my understanding they wouldn't show up in a widget test because |
||
| (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(); | ||
| expect(controller.selection, const TextSelection.collapsed(offset: 5)); | ||
| expectCupertinoToolbarForCollapsedSelection(); | ||
|
|
||
| // 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<EditableTextState>(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 | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree it's worth it to expose this 👍 .