firka/flutter
firka/flutter
5
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix: SelectableRegion should only finalize selection after changing (#159698)

Browse Source 
There are some cases where selection behavior varies on a given platform
by the pointer device, for example dragging to select changes on mobile
platforms depending on whether a mouse or a touch is used. When a mouse
is used a user can drag to select normally, when a touch is used the
selection does not change on mobile platforms when dragging.

Before this change at the end of a touch drag users would still be
notified the selection was finalized even though nothing changed and
they had not previously received a `changing` notification. After this
change the selection is no longer finalized unless the
`SelectableRegionSelectionStatus` was previously in a `changing` state.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

---------

Co-authored-by: Renzo Olivares <roliv@google.com>
This commit is contained in:
Renzo Olivares 2024-12-09 17:25:58 -08:00 committed by GitHub
parent 129a35a5e1
commit d620ec9274
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 115 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

73
packages/flutter/lib/src/widgets/selectable_region.dart
View File

@ -491,7 +491,7 @@ class SelectableRegionState extends State<SelectableRegion> with TextSelectionDe
// the Flutter application.
clearSelection();
_selectionStatusNotifier.value = SelectableRegionSelectionStatus.changing;
_selectionStatusNotifier.value = SelectableRegionSelectionStatus.finalized;
_finalizeSelectableRegionStatus();
}
}
if (kIsWeb) {
@ -541,6 +541,14 @@ class SelectableRegionState extends State<SelectableRegion> with TextSelectionDe
}
}
void _finalizeSelectableRegionStatus() {
if (_selectionStatusNotifier.value != SelectableRegionSelectionStatus.changing) {
// Don't finalize the selection again if it is not currently changing.
return;
}
_selectionStatusNotifier.value = SelectableRegionSelectionStatus.finalized;
}
// Converts the details.consecutiveTapCount from a TapAndDrag*Details object,
// which can grow to be infinitely large, to a value between 1 and the supported
// max consecutive tap count. The value that the raw count is converted to is
@ -808,20 +816,22 @@ class SelectableRegionState extends State<SelectableRegion> with TextSelectionDe
}
void _handleMouseDragEnd(TapDragEndDetails details) {
final bool isPointerPrecise = _lastPointerDeviceKind != null && _lastPointerDeviceKind == PointerDeviceKind.mouse;
assert(_lastPointerDeviceKind != null);
final bool isPointerPrecise = _isPrecisePointerDevice(_lastPointerDeviceKind!);
// On mobile platforms like android, fuchsia, and iOS, a drag gesture will
// only show the selection overlay when the drag has finished and the pointer
// device kind is not precise, for example at the end of a double tap + drag
// to select on native iOS.
final bool shouldShowSelectionOverlayOnMobile = !isPointerPrecise;
switch (defaultTargetPlatform) {
case TargetPlatform.android:
case TargetPlatform.fuchsia:
if (!isPointerPrecise) {
// On Android, a drag gesture will only show the selection overlay when
// the drag has finished and the pointer device kind is not precise.
if (shouldShowSelectionOverlayOnMobile) {
_showHandles();
_showToolbar();
}
case TargetPlatform.iOS:
if (!isPointerPrecise) {
// On iOS, a drag gesture will only show the selection toolbar when
// the drag has finished and the pointer device kind is not precise.
if (shouldShowSelectionOverlayOnMobile) {
_showToolbar();
}
case TargetPlatform.macOS:
@ -832,7 +842,7 @@ class SelectableRegionState extends State<SelectableRegion> with TextSelectionDe
}
_finalizeSelection();
_updateSelectedContentIfNeeded();
_selectionStatusNotifier.value = SelectableRegionSelectionStatus.finalized;
_finalizeSelectableRegionStatus();
}
void _handleMouseTapUp(TapDragUpDetails details) {
@ -856,12 +866,10 @@ class SelectableRegionState extends State<SelectableRegion> with TextSelectionDe
hideToolbar();
_collapseSelectionAt(offset: details.globalPosition);
_selectionStatusNotifier.value = SelectableRegionSelectionStatus.changing;
_selectionStatusNotifier.value = SelectableRegionSelectionStatus.finalized;
case TargetPlatform.macOS:
case TargetPlatform.linux:
case TargetPlatform.windows:
// On desktop platforms the selection is set on tap down.
_selectionStatusNotifier.value = SelectableRegionSelectionStatus.finalized;
}
case 2:
final bool isPointerPrecise = _isPrecisePointerDevice(details.kind);
@ -878,7 +886,7 @@ class SelectableRegionState extends State<SelectableRegion> with TextSelectionDe
if (!isPointerPrecise) {
if (kIsWeb) {
// Double tap on iOS web only triggers when a drag begins after the double tap.
return;
break;
}
// On iOS, a double tap will only show the selection toolbar after
// the following tap up when the pointer device kind is not precise.
@ -891,24 +899,8 @@ class SelectableRegionState extends State<SelectableRegion> with TextSelectionDe
// on a double click.
break;
}
_selectionStatusNotifier.value = SelectableRegionSelectionStatus.finalized;
case 3:
switch (defaultTargetPlatform) {
case TargetPlatform.android:
case TargetPlatform.fuchsia:
case TargetPlatform.iOS:
if (_isPrecisePointerDevice(details.kind)) {
// Triple tap on static text is only supported on mobile
// platforms using a precise pointer device, so we should
// only update the SelectableRegionSelectionStatus in that case.
_selectionStatusNotifier.value = SelectableRegionSelectionStatus.finalized;
}
case TargetPlatform.macOS:
case TargetPlatform.linux:
case TargetPlatform.windows:
_selectionStatusNotifier.value = SelectableRegionSelectionStatus.finalized;
}
}
_finalizeSelectableRegionStatus();
_updateSelectedContentIfNeeded();
}
@ -946,7 +938,7 @@ class SelectableRegionState extends State<SelectableRegion> with TextSelectionDe
void _handleTouchLongPressEnd(LongPressEndDetails details) {
_finalizeSelection();
_updateSelectedContentIfNeeded();
_selectionStatusNotifier.value = SelectableRegionSelectionStatus.finalized;
_finalizeSelectableRegionStatus();
_showToolbar();
if (defaultTargetPlatform == TargetPlatform.android) {
_showHandles();
@ -1007,7 +999,7 @@ class SelectableRegionState extends State<SelectableRegion> with TextSelectionDe
}
}
_selectionStatusNotifier.value = SelectableRegionSelectionStatus.changing;
_selectionStatusNotifier.value = SelectableRegionSelectionStatus.finalized;
_finalizeSelectableRegionStatus();
// Restore _lastSecondaryTapDownPosition since it may be cleared if a user
// accesses contextMenuAnchors.
_lastSecondaryTapDownPosition = details.globalPosition;
@ -1064,7 +1056,7 @@ class SelectableRegionState extends State<SelectableRegion> with TextSelectionDe
}
_finalizeSelection();
_updateSelectedContentIfNeeded();
_selectionStatusNotifier.value = SelectableRegionSelectionStatus.finalized;
_finalizeSelectableRegionStatus();
}
void _stopSelectionEndEdgeUpdate() {
@ -1545,7 +1537,7 @@ class SelectableRegionState extends State<SelectableRegion> with TextSelectionDe
);
_updateSelectedContentIfNeeded();
_selectionStatusNotifier.value = SelectableRegionSelectionStatus.changing;
_selectionStatusNotifier.value = SelectableRegionSelectionStatus.finalized;
_finalizeSelectableRegionStatus();
}
double? _directionalHorizontalBaseline;
@ -1569,7 +1561,7 @@ class SelectableRegionState extends State<SelectableRegion> with TextSelectionDe
);
_updateSelectedContentIfNeeded();
_selectionStatusNotifier.value = SelectableRegionSelectionStatus.changing;
_selectionStatusNotifier.value = SelectableRegionSelectionStatus.finalized;
_finalizeSelectableRegionStatus();
}
// [TextSelectionDelegate] overrides.
@ -1602,7 +1594,7 @@ class SelectableRegionState extends State<SelectableRegion> with TextSelectionDe
case TargetPlatform.fuchsia:
clearSelection();
_selectionStatusNotifier.value = SelectableRegionSelectionStatus.changing;
_selectionStatusNotifier.value = SelectableRegionSelectionStatus.finalized;
_finalizeSelectableRegionStatus();
case TargetPlatform.iOS:
hideToolbar(false);
case TargetPlatform.linux:
@ -1633,7 +1625,7 @@ class SelectableRegionState extends State<SelectableRegion> with TextSelectionDe
case TargetPlatform.fuchsia:
clearSelection();
_selectionStatusNotifier.value = SelectableRegionSelectionStatus.changing;
_selectionStatusNotifier.value = SelectableRegionSelectionStatus.finalized;
_finalizeSelectableRegionStatus();
case TargetPlatform.iOS:
hideToolbar(false);
case TargetPlatform.linux:
@ -1735,7 +1727,7 @@ class SelectableRegionState extends State<SelectableRegion> with TextSelectionDe
}
_updateSelectedContentIfNeeded();
_selectionStatusNotifier.value = SelectableRegionSelectionStatus.changing;
_selectionStatusNotifier.value = SelectableRegionSelectionStatus.finalized;
_finalizeSelectableRegionStatus();
}
@Deprecated(
@ -1747,7 +1739,7 @@ class SelectableRegionState extends State<SelectableRegion> with TextSelectionDe
_copy();
clearSelection();
_selectionStatusNotifier.value = SelectableRegionSelectionStatus.changing;
_selectionStatusNotifier.value = SelectableRegionSelectionStatus.finalized;
_finalizeSelectableRegionStatus();
}
@Deprecated(
@ -3246,7 +3238,10 @@ final class _SelectableRegionSelectionStatusNotifier extends ChangeNotifier impl
/// Listeners are notified even if the value did not change.
@protected
set value(SelectableRegionSelectionStatus newStatus) {
_selectableRegionSelectionStatus = newStatus;
assert(newStatus == SelectableRegionSelectionStatus.finalized && value == SelectableRegionSelectionStatus.changing
|| newStatus == SelectableRegionSelectionStatus.changing,
'Attempting to finalize the selection when it is already finalized.');
_selectableRegionSelectionStatus = newStatus;
notifyListeners();
}
}

81
packages/flutter/test/widgets/selectable_region_test.dart
View File

@ -1431,6 +1431,87 @@ void main() {
await gesture.up();
}, variant: TargetPlatformVariant.mobile());
testWidgets('mouse drag finalizes the selection', (WidgetTester tester) async {
SelectableRegionSelectionStatus? selectionStatus;
final GlobalKey textKey = GlobalKey();
await tester.pumpWidget(
MaterialApp(
home: SelectableRegion(
selectionControls: materialTextSelectionControls,
child: Center(
child: Text(
key: textKey,
'How are you',
),
),
),
),
);
await tester.pumpAndSettle();
expect(textKey.currentContext, isNotNull);
final ValueListenable<SelectableRegionSelectionStatus>? selectionStatusNotifier = SelectableRegionSelectionStatusScope.maybeOf(textKey.currentContext!);
void onSelectionStatusChange() {
selectionStatus = selectionStatusNotifier?.value;
}
selectionStatusNotifier?.addListener(onSelectionStatusChange);
addTearDown(() {
selectionStatusNotifier?.removeListener(onSelectionStatusChange);
});
final RenderParagraph paragraph = tester.renderObject<RenderParagraph>(find.descendant(of: find.text('How are you'), matching: find.byType(RichText)));
final TestGesture gesture = await tester.startGesture(textOffsetToPosition(paragraph, 2), kind: PointerDeviceKind.mouse);
addTearDown(gesture.removePointer);
await tester.pump();
await gesture.moveTo(textOffsetToPosition(paragraph, 4));
await tester.pump();
expect(selectionStatus, SelectableRegionSelectionStatus.changing);
await gesture.up();
await tester.pump();
expect(paragraph.selections.length, 1);
expect(selectionStatus, SelectableRegionSelectionStatus.finalized);
}, variant: TargetPlatformVariant.all());
testWidgets('touch drag does not finalize selection on mobile platforms', (WidgetTester tester) async {
SelectableRegionSelectionStatus? selectionStatus;
final GlobalKey textKey = GlobalKey();
await tester.pumpWidget(
MaterialApp(
home: SelectableRegion(
selectionControls: materialTextSelectionControls,
child: Center(
child: Text(
key: textKey,
'How are you',
),
),
),
),
);
await tester.pumpAndSettle();
expect(textKey.currentContext, isNotNull);
final ValueListenable<SelectableRegionSelectionStatus>? selectionStatusNotifier = SelectableRegionSelectionStatusScope.maybeOf(textKey.currentContext!);
void onSelectionStatusChange() {
selectionStatus = selectionStatusNotifier?.value;
}
selectionStatusNotifier?.addListener(onSelectionStatusChange);
addTearDown(() {
selectionStatusNotifier?.removeListener(onSelectionStatusChange);
});
final RenderParagraph paragraph = tester.renderObject<RenderParagraph>(find.descendant(of: find.text('How are you'), matching: find.byType(RichText)));
final TestGesture gesture = await tester.startGesture(textOffsetToPosition(paragraph, 2));
addTearDown(gesture.removePointer);
await tester.pump();
await gesture.moveTo(textOffsetToPosition(paragraph, 4));
await tester.pump();
await gesture.up();
await tester.pump();
expect(paragraph.selections.length, 0);
expect(selectionStatus, isNull);
}, variant: TargetPlatformVariant.mobile());
testWidgets('mouse can select word-by-word on double click drag', (WidgetTester tester) async {
await tester.pumpWidget(
MaterialApp(