Skip to content

Commit

Permalink
Implement SelectionArea single click/tap gestures (#132682)
Browse files Browse the repository at this point in the history
This change collapses the selection at the clicked/tapped location on single click down for desktop platforms, and on single click/tap up for mobile platforms to match native.

This is a change from how `SelectionArea` previously worked. Before this change a single click down would clear the selection. From observing a native browser it looks like when tapping on static text the selection is not cleared but collapsed. A user can still attain the selection from static text using the `window.getSelection` API.

https://jsfiddle.net/juepasn3/11/ You can try this demo out here to observe this behavior yourself. When clicking on static text the selection will change.

This change also allows `Paragraph.selections` to return selections that are collapsed. This for testing purposes to confirm where the selection has been collapsed.

Partially fixes: #129583
  • Loading branch information
Renzo-Olivares authored Sep 28, 2023
1 parent 80fb7bd commit 21ad712
Show file tree
Hide file tree
Showing 7 changed files with 375 additions and 99 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ void main() {

// Right clicking the Text in the SelectionArea shows the custom context
// menu.
final TestGesture primaryMouseButtonGesture = await tester.createGesture(
kind: PointerDeviceKind.mouse,
);
final TestGesture gesture = await tester.startGesture(
tester.getCenter(find.text(example.text)),
kind: PointerDeviceKind.mouse,
Expand All @@ -37,7 +40,9 @@ void main() {
expect(find.text('Print'), findsOneWidget);

// Tap to dismiss.
await tester.tapAt(tester.getCenter(find.byType(Scaffold)));
await primaryMouseButtonGesture.down(tester.getCenter(find.byType(Scaffold)));
await tester.pump();
await primaryMouseButtonGesture.up();
await tester.pumpAndSettle();

expect(find.byType(AdaptiveTextSelectionToolbar), findsNothing);
Expand Down
11 changes: 5 additions & 6 deletions packages/flutter/lib/src/rendering/paragraph.dart
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ mixin RenderInlineChildrenContainerDefaults on RenderBox, ContainerRenderObjectM
ui.PlaceholderAlignment.belowBaseline ||
ui.PlaceholderAlignment.bottom ||
ui.PlaceholderAlignment.middle ||
ui.PlaceholderAlignment.top => null,
ui.PlaceholderAlignment.top => null,
ui.PlaceholderAlignment.baseline => child.getDistanceToBaseline(span.baseline!),
},
);
Expand Down Expand Up @@ -351,8 +351,7 @@ class RenderParagraph extends RenderBox with ContainerRenderObjectMixin<RenderBo
final List<TextSelection> results = <TextSelection>[];
for (final _SelectableFragment fragment in _lastSelectableFragments!) {
if (fragment._textSelectionStart != null &&
fragment._textSelectionEnd != null &&
fragment._textSelectionStart!.offset != fragment._textSelectionEnd!.offset) {
fragment._textSelectionEnd != null) {
results.add(
TextSelection(
baseOffset: fragment._textSelectionStart!.offset,
Expand Down Expand Up @@ -1309,9 +1308,9 @@ class RenderParagraph extends RenderBox with ContainerRenderObjectMixin<RenderBo

/// A continuous, selectable piece of paragraph.
///
/// Since the selections in [PlaceHolderSpan] are handled independently in its
/// Since the selections in [PlaceholderSpan] are handled independently in its
/// subtree, a selection in [RenderParagraph] can't continue across a
/// [PlaceHolderSpan]. The [RenderParagraph] splits itself on [PlaceHolderSpan]
/// [PlaceholderSpan]. The [RenderParagraph] splits itself on [PlaceholderSpan]
/// to create multiple `_SelectableFragment`s so that they can be selected
/// separately.
class _SelectableFragment with Selectable, ChangeNotifier implements TextLayoutMetrics {
Expand Down Expand Up @@ -1720,7 +1719,7 @@ class _SelectableFragment with Selectable, ChangeNotifier implements TextLayoutM
_selectableContainsOriginWord = true;

final TextPosition position = paragraph.getPositionForOffset(paragraph.globalToLocal(globalPosition));
if (_positionIsWithinCurrentSelection(position)) {
if (_positionIsWithinCurrentSelection(position) && _textSelectionStart != _textSelectionEnd) {
return SelectionResult.end;
}
final _WordBoundaryRecord wordBoundary = _getWordBoundaryAtPosition(position);
Expand Down
4 changes: 2 additions & 2 deletions packages/flutter/lib/src/widgets/scrollable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -670,7 +670,7 @@ class ScrollableState extends State<Scrollable> with TickerProviderStateMixin, R
if (oldWidget.controller == null) {
// The old controller was null, meaning the fallback cannot be null.
// Dispose of the fallback.
assert(_fallbackScrollController != null);
assert(_fallbackScrollController != null);
assert(widget.controller != null);
_fallbackScrollController!.detach(position);
_fallbackScrollController!.dispose();
Expand Down Expand Up @@ -1954,7 +1954,7 @@ class TwoDimensionalScrollableState extends State<TwoDimensionalScrollable> {
if (oldWidget.horizontalDetails.controller == null) {
// The old controller was null, meaning the fallback cannot be null.
// Dispose of the fallback.
assert(_horizontalFallbackController != null);
assert(_horizontalFallbackController != null);
assert(widget.horizontalDetails.controller != null);
_horizontalFallbackController!.dispose();
_horizontalFallbackController = null;
Expand Down
139 changes: 120 additions & 19 deletions packages/flutter/lib/src/widgets/selectable_region.dart
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,8 @@ class SelectableRegionState extends State<SelectableRegion> with TextSelectionDe
_showToolbar(location: details.globalPosition);
}
} else {
_clearSelection();
hideToolbar();
_collapseSelectionAt(offset: details.globalPosition);
}
};
instance.onSecondaryTapDown = _handleRightClickDown;
Expand Down Expand Up @@ -472,6 +473,7 @@ class SelectableRegionState extends State<SelectableRegion> with TextSelectionDe
(TapAndPanGestureRecognizer instance) {
instance
..onTapDown = _startNewMouseSelectionGesture
..onTapUp = _handleMouseTapUp
..onDragStart = _handleMouseDragStart
..onDragUpdate = _handleMouseDragUpdate
..onDragEnd = _handleMouseDragEnd
Expand All @@ -498,7 +500,17 @@ class SelectableRegionState extends State<SelectableRegion> with TextSelectionDe
case 1:
widget.focusNode.requestFocus();
hideToolbar();
_clearSelection();
switch (defaultTargetPlatform) {
case TargetPlatform.android:
case TargetPlatform.fuchsia:
case TargetPlatform.iOS:
// On mobile platforms the selection is set on tap up.
break;
case TargetPlatform.macOS:
case TargetPlatform.linux:
case TargetPlatform.windows:
_collapseSelectionAt(offset: details.globalPosition);
}
case 2:
_selectWordAt(offset: details.globalPosition);
}
Expand Down Expand Up @@ -528,6 +540,24 @@ class SelectableRegionState extends State<SelectableRegion> with TextSelectionDe
_updateSelectedContentIfNeeded();
}

void _handleMouseTapUp(TapDragUpDetails details) {
switch (_getEffectiveConsecutiveTapCount(details.consecutiveTapCount)) {
case 1:
switch (defaultTargetPlatform) {
case TargetPlatform.android:
case TargetPlatform.fuchsia:
case TargetPlatform.iOS:
_collapseSelectionAt(offset: details.globalPosition);
case TargetPlatform.macOS:
case TargetPlatform.linux:
case TargetPlatform.windows:
// On desktop platforms the selection is set on tap down.
break;
}
}
_updateSelectedContentIfNeeded();
}

void _updateSelectedContentIfNeeded() {
if (_lastSelectedContent?.plainText != _selectable?.getSelectedContent()?.plainText) {
_lastSelectedContent = _selectable?.getSelectedContent();
Expand Down Expand Up @@ -586,8 +616,7 @@ class SelectableRegionState extends State<SelectableRegion> with TextSelectionDe
// keep the current selection, if not then collapse it.
final bool lastSecondaryTapDownPositionWasOnActiveSelection = _positionIsOnActiveSelection(globalPosition: details.globalPosition);
if (!lastSecondaryTapDownPositionWasOnActiveSelection) {
_selectStartTo(offset: lastSecondaryTapDownPosition!);
_selectEndTo(offset: lastSecondaryTapDownPosition!);
_collapseSelectionAt(offset: lastSecondaryTapDownPosition!);
}
_showHandles();
_showToolbar(location: lastSecondaryTapDownPosition);
Expand All @@ -612,8 +641,7 @@ class SelectableRegionState extends State<SelectableRegion> with TextSelectionDe
// keep the current selection, if not then collapse it.
final bool lastSecondaryTapDownPositionWasOnActiveSelection = _positionIsOnActiveSelection(globalPosition: details.globalPosition);
if (!lastSecondaryTapDownPositionWasOnActiveSelection) {
_selectStartTo(offset: lastSecondaryTapDownPosition!);
_selectEndTo(offset: lastSecondaryTapDownPosition!);
_collapseSelectionAt(offset: lastSecondaryTapDownPosition!);
}
_showHandles();
_showToolbar(location: lastSecondaryTapDownPosition);
Expand Down Expand Up @@ -925,8 +953,9 @@ class SelectableRegionState extends State<SelectableRegion> with TextSelectionDe
/// See also:
/// * [_selectStartTo], which sets or updates selection start edge.
/// * [_finalizeSelection], which stops the `continuous` updates.
/// * [_clearSelection], which clear the ongoing selection.
/// * [_clearSelection], which clears the ongoing selection.
/// * [_selectWordAt], which selects a whole word at the location.
/// * [_collapseSelectionAt], which collapses the selection at the location.
/// * [selectAll], which selects the entire content.
void _selectEndTo({required Offset offset, bool continuous = false, TextGranularity? textGranularity}) {
if (!continuous) {
Expand Down Expand Up @@ -964,8 +993,9 @@ class SelectableRegionState extends State<SelectableRegion> with TextSelectionDe
/// See also:
/// * [_selectEndTo], which sets or updates selection end edge.
/// * [_finalizeSelection], which stops the `continuous` updates.
/// * [_clearSelection], which clear the ongoing selection.
/// * [_clearSelection], which clears the ongoing selection.
/// * [_selectWordAt], which selects a whole word at the location.
/// * [_collapseSelectionAt], which collapses the selection at the location.
/// * [selectAll], which selects the entire content.
void _selectStartTo({required Offset offset, bool continuous = false, TextGranularity? textGranularity}) {
if (!continuous) {
Expand All @@ -978,6 +1008,20 @@ class SelectableRegionState extends State<SelectableRegion> with TextSelectionDe
}
}

/// Collapses the selection at the given `offset` location.
///
/// See also:
/// * [_selectStartTo], which sets or updates selection start edge.
/// * [_selectEndTo], which sets or updates selection end edge.
/// * [_finalizeSelection], which stops the `continuous` updates.
/// * [_clearSelection], which clears the ongoing selection.
/// * [_selectWordAt], which selects a whole word at the location.
/// * [selectAll], which selects the entire content.
void _collapseSelectionAt({required Offset offset}) {
_selectStartTo(offset: offset);
_selectEndTo(offset: offset);
}

/// Selects a whole word at the `offset` location.
///
/// If the whole word is already in the current selection, selection won't
Expand All @@ -991,7 +1035,8 @@ class SelectableRegionState extends State<SelectableRegion> with TextSelectionDe
/// * [_selectStartTo], which sets or updates selection start edge.
/// * [_selectEndTo], which sets or updates selection end edge.
/// * [_finalizeSelection], which stops the `continuous` updates.
/// * [_clearSelection], which clear the ongoing selection.
/// * [_clearSelection], which clears the ongoing selection.
/// * [_collapseSelectionAt], which collapses the selection at the location.
/// * [selectAll], which selects the entire content.
void _selectWordAt({required Offset offset}) {
// There may be other selection ongoing.
Expand Down Expand Up @@ -1881,7 +1926,7 @@ abstract class MultiSelectableSelectionContainerDelegate extends SelectionContai

SelectionPoint? startPoint;
if (startGeometry.startSelectionPoint != null) {
final Matrix4 startTransform = getTransformFrom(selectables[startIndexWalker]);
final Matrix4 startTransform = getTransformFrom(selectables[startIndexWalker]);
final Offset start = MatrixUtils.transformPoint(startTransform, startGeometry.startSelectionPoint!.localPosition);
// It can be NaN if it is detached or off-screen.
if (start.isFinite) {
Expand All @@ -1902,7 +1947,7 @@ abstract class MultiSelectableSelectionContainerDelegate extends SelectionContai
}
SelectionPoint? endPoint;
if (endGeometry.endSelectionPoint != null) {
final Matrix4 endTransform = getTransformFrom(selectables[endIndexWalker]);
final Matrix4 endTransform = getTransformFrom(selectables[endIndexWalker]);
final Offset end = MatrixUtils.transformPoint(endTransform, endGeometry.endSelectionPoint!.localPosition);
// It can be NaN if it is detached or off-screen.
if (end.isFinite) {
Expand Down Expand Up @@ -1986,8 +2031,8 @@ abstract class MultiSelectableSelectionContainerDelegate extends SelectionContai
final Rect? drawableArea = hasSize ? Rect
.fromLTWH(0, 0, containerSize.width, containerSize.height)
.inflate(_kSelectionHandleDrawableAreaPadding) : null;
final bool hideStartHandle = value.startSelectionPoint == null || drawableArea == null || !drawableArea.contains(value.startSelectionPoint!.localPosition);
final bool hideEndHandle = value.endSelectionPoint == null || drawableArea == null|| !drawableArea.contains(value.endSelectionPoint!.localPosition);
final bool hideStartHandle = value.startSelectionPoint == null || drawableArea == null || !drawableArea.contains(value.startSelectionPoint!.localPosition);
final bool hideEndHandle = value.endSelectionPoint == null || drawableArea == null|| !drawableArea.contains(value.endSelectionPoint!.localPosition);
effectiveStartHandle = hideStartHandle ? null : _startHandleLayer;
effectiveEndHandle = hideEndHandle ? null : _endHandleLayer;
}
Expand Down Expand Up @@ -2047,6 +2092,34 @@ abstract class MultiSelectableSelectionContainerDelegate extends SelectionContai
);
}

// Clears the selection on all selectables not in the range of
// currentSelectionStartIndex..currentSelectionEndIndex.
//
// If one of the edges does not exist, then this method will clear the selection
// in all selectables except the existing edge.
//
// If neither of the edges exist this method immediately returns.
void _flushInactiveSelections() {
if (currentSelectionStartIndex == -1 && currentSelectionEndIndex == -1) {
return;
}
if (currentSelectionStartIndex == -1 || currentSelectionEndIndex == -1) {
final int skipIndex = currentSelectionStartIndex == -1 ? currentSelectionEndIndex : currentSelectionStartIndex;
selectables
.where((Selectable target) => target != selectables[skipIndex])
.forEach((Selectable target) => dispatchSelectionEventToChild(target, const ClearSelectionEvent()));
return;
}
final int skipStart = min(currentSelectionStartIndex, currentSelectionEndIndex);
final int skipEnd = max(currentSelectionStartIndex, currentSelectionEndIndex);
for (int index = 0; index < selectables.length; index += 1) {
if (index >= skipStart && index <= skipEnd) {
continue;
}
dispatchSelectionEventToChild(selectables[index], const ClearSelectionEvent());
}
}

/// Selects all contents of all selectables.
@protected
SelectionResult handleSelectAll(SelectAllSelectionEvent event) {
Expand Down Expand Up @@ -2290,7 +2363,7 @@ abstract class MultiSelectableSelectionContainerDelegate extends SelectionContai
bool hasFoundEdgeIndex = false;
SelectionResult? result;
for (int index = 0; index < selectables.length && !hasFoundEdgeIndex; index += 1) {
final Selectable child = selectables[index];
final Selectable child = selectables[index];
final SelectionResult childResult = dispatchSelectionEventToChild(child, event);
switch (childResult) {
case SelectionResult.next:
Expand Down Expand Up @@ -2323,6 +2396,7 @@ abstract class MultiSelectableSelectionContainerDelegate extends SelectionContai
} else {
currentSelectionStartIndex = newIndex;
}
_flushInactiveSelections();
// The result can only be null if the loop went through the entire list
// without any of the selection returned end or previous. In this case, the
// caller of this method needs to find the next selectable in their list.
Expand All @@ -2345,13 +2419,39 @@ abstract class MultiSelectableSelectionContainerDelegate extends SelectionContai
return true;
}());
SelectionResult? finalResult;
int newIndex = isEnd ? currentSelectionEndIndex : currentSelectionStartIndex;
// Determines if the edge being adjusted is within the current viewport.
// - If so, we begin the search for the new selection edge position at the
// currentSelectionEndIndex/currentSelectionStartIndex.
// - If not, we attempt to locate the new selection edge starting from
// the opposite end.
// - If neither edge is in the current viewport, the search for the new
// selection edge position begins at 0.
//
// This can happen when there is a scrollable child and the edge being adjusted
// has been scrolled out of view.
final bool isCurrentEdgeWithinViewport = isEnd ? _selectionGeometry.endSelectionPoint != null : _selectionGeometry.startSelectionPoint != null;
final bool isOppositeEdgeWithinViewport = isEnd ? _selectionGeometry.startSelectionPoint != null : _selectionGeometry.endSelectionPoint != null;
int newIndex = switch ((isEnd, isCurrentEdgeWithinViewport, isOppositeEdgeWithinViewport)) {
(true, true, true) => currentSelectionEndIndex,
(true, true, false) => currentSelectionEndIndex,
(true, false, true) => currentSelectionStartIndex,
(true, false, false) => 0,
(false, true, true) => currentSelectionStartIndex,
(false, true, false) => currentSelectionStartIndex,
(false, false, true) => currentSelectionEndIndex,
(false, false, false) => 0,
};
bool? forward;
late SelectionResult currentSelectableResult;
// This loop sends the selection event to the
// currentSelectionEndIndex/currentSelectionStartIndex to determine the
// direction of the search. If the result is `SelectionResult.next`, this
// loop look backward. Otherwise, it looks forward.
// This loop sends the selection event to one of the following to determine
// the direction of the search.
// - currentSelectionEndIndex/currentSelectionStartIndex if the current edge
// is in the current viewport.
// - The opposite edge index if the current edge is not in the current viewport.
// - Index 0 if neither edge is in the current viewport.
//
// If the result is `SelectionResult.next`, this loop look backward.
// Otherwise, it looks forward.
//
// The terminate condition are:
// 1. the selectable returns end, pending, none.
Expand Down Expand Up @@ -2391,6 +2491,7 @@ abstract class MultiSelectableSelectionContainerDelegate extends SelectionContai
} else {
currentSelectionStartIndex = newIndex;
}
_flushInactiveSelections();
return finalResult!;
}
}
Expand Down
10 changes: 8 additions & 2 deletions packages/flutter/test/material/selection_area_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,14 @@ void main() {

// Backwards selection.
await gesture.down(textOffsetToPosition(paragraph, 3));
await tester.pumpAndSettle();
expect(content, isNull);
await tester.pump();
await gesture.up();
await tester.pumpAndSettle(kDoubleTapTimeout);
expect(content, isNotNull);
expect(content!.plainText, '');

await gesture.down(textOffsetToPosition(paragraph, 3));
await tester.pump();
await gesture.moveTo(textOffsetToPosition(paragraph, 0));
await gesture.up();
await tester.pump();
Expand Down
3 changes: 2 additions & 1 deletion packages/flutter/test/rendering/paragraph_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -978,7 +978,8 @@ void main() {
granularity: TextGranularity.word,
),
);
expect(paragraph.selections.length, 0); // how []are you
expect(paragraph.selections.length, 1); // how []are you
expect(paragraph.selections[0], const TextSelection.collapsed(offset: 4));

// Equivalent to sending shift + alt + arrow-left.
registrar.selectables[0].dispatchSelectionEvent(
Expand Down
Loading

0 comments on commit 21ad712

Please sign in to comment.