Material Long Press Text Handle Flash (#32911)
Fix a bug where holding down on text selection caused the handles to flash. The fix was to only update selection when it actually changed.
diff --git a/bin/internal/goldens.version b/bin/internal/goldens.version
index ef84419..00adb77 100644
--- a/bin/internal/goldens.version
+++ b/bin/internal/goldens.version
@@ -1 +1 @@
-5e0cf0b9b347526d299718a4755e2d65cffacaba
+25ab3c95b1ac02a41973c05f96e664370857ce55
diff --git a/packages/flutter/lib/src/rendering/editable.dart b/packages/flutter/lib/src/rendering/editable.dart
index a10f721..bed0fb5 100644
--- a/packages/flutter/lib/src/rendering/editable.dart
+++ b/packages/flutter/lib/src/rendering/editable.dart
@@ -350,6 +350,18 @@
static const int _kShiftMask = 1; // https://developer.android.com/reference/android/view/KeyEvent.html#META_SHIFT_ON
static const int _kControlMask = 1 << 12; // https://developer.android.com/reference/android/view/KeyEvent.html#META_CTRL_ON
+ // Call through to onSelectionChanged only if the given nextSelection is
+ // different from the existing selection.
+ void _handlePotentialSelectionChange(
+ TextSelection nextSelection,
+ SelectionChangedCause cause,
+ ) {
+ if (nextSelection == selection) {
+ return;
+ }
+ onSelectionChanged(nextSelection, this, cause);
+ }
+
// TODO(goderbauer): doesn't handle extended grapheme clusters with more than one Unicode scalar value (https://github.com/flutter/flutter/issues/13404).
void _handleKeyEvent(RawKeyEvent keyEvent) {
// Only handle key events on Android.
@@ -484,21 +496,19 @@
// base offset is always less than the extent offset.
if (shift) {
if (_baseOffset < newOffset) {
- onSelectionChanged(
+ _handlePotentialSelectionChange(
TextSelection(
baseOffset: _baseOffset,
extentOffset: newOffset,
),
- this,
SelectionChangedCause.keyboard,
);
} else {
- onSelectionChanged(
+ _handlePotentialSelectionChange(
TextSelection(
baseOffset: newOffset,
extentOffset: _baseOffset,
),
- this,
SelectionChangedCause.keyboard,
);
}
@@ -511,13 +521,12 @@
else if (rightArrow)
newOffset = _baseOffset > _extentOffset ? _baseOffset : _extentOffset;
}
- onSelectionChanged(
+ _handlePotentialSelectionChange(
TextSelection.fromPosition(
TextPosition(
offset: newOffset
)
),
- this,
SelectionChangedCause.keyboard,
);
}
@@ -564,12 +573,11 @@
case _kAKeyCode:
_baseOffset = 0;
_extentOffset = textSelectionDelegate.textEditingValue.text.length;
- onSelectionChanged(
+ _handlePotentialSelectionChange(
TextSelection(
baseOffset: 0,
extentOffset: textSelectionDelegate.textEditingValue.text.length,
),
- this,
SelectionChangedCause.keyboard,
);
break;
@@ -967,7 +975,7 @@
}
void _handleSetSelection(TextSelection selection) {
- onSelectionChanged(selection, this, SelectionChangedCause.keyboard);
+ _handlePotentialSelectionChange(selection, SelectionChangedCause.keyboard);
}
void _handleMoveCursorForwardByCharacter(bool extentSelection) {
@@ -975,8 +983,8 @@
if (extentOffset == null)
return;
final int baseOffset = !extentSelection ? extentOffset : _selection.baseOffset;
- onSelectionChanged(
- TextSelection(baseOffset: baseOffset, extentOffset: extentOffset), this, SelectionChangedCause.keyboard,
+ _handlePotentialSelectionChange(
+ TextSelection(baseOffset: baseOffset, extentOffset: extentOffset), SelectionChangedCause.keyboard,
);
}
@@ -985,8 +993,8 @@
if (extentOffset == null)
return;
final int baseOffset = !extentSelection ? extentOffset : _selection.baseOffset;
- onSelectionChanged(
- TextSelection(baseOffset: baseOffset, extentOffset: extentOffset), this, SelectionChangedCause.keyboard,
+ _handlePotentialSelectionChange(
+ TextSelection(baseOffset: baseOffset, extentOffset: extentOffset), SelectionChangedCause.keyboard,
);
}
@@ -998,12 +1006,11 @@
if (nextWord == null)
return;
final int baseOffset = extentSelection ? _selection.baseOffset : nextWord.start;
- onSelectionChanged(
+ _handlePotentialSelectionChange(
TextSelection(
baseOffset: baseOffset,
extentOffset: nextWord.start,
),
- this,
SelectionChangedCause.keyboard,
);
}
@@ -1016,12 +1023,11 @@
if (previousWord == null)
return;
final int baseOffset = extentSelection ? _selection.baseOffset : previousWord.start;
- onSelectionChanged(
+ _handlePotentialSelectionChange(
TextSelection(
baseOffset: baseOffset,
extentOffset: previousWord.start,
),
- this,
SelectionChangedCause.keyboard,
);
}
@@ -1400,7 +1406,7 @@
);
// Call [onSelectionChanged] only when the selection actually changed.
if (newSelection != _selection) {
- onSelectionChanged(newSelection, this, cause);
+ _handlePotentialSelectionChange(newSelection, cause);
}
}
}
@@ -1428,12 +1434,13 @@
final TextSelection lastWord = to == null ?
firstWord : _selectWordAtOffset(_textPainter.getPositionForOffset(globalToLocal(to - _paintOffset)));
- onSelectionChanged(
+ _handlePotentialSelectionChange(
TextSelection(
baseOffset: firstWord.base.offset,
extentOffset: lastWord.extent.offset,
affinity: firstWord.affinity,
- ), this, cause,
+ ),
+ cause,
);
}
}
@@ -1449,15 +1456,13 @@
final TextPosition position = _textPainter.getPositionForOffset(globalToLocal(_lastTapDownPosition - _paintOffset));
final TextRange word = _textPainter.getWordBoundary(position);
if (position.offset - word.start <= 1) {
- onSelectionChanged(
+ _handlePotentialSelectionChange(
TextSelection.collapsed(offset: word.start, affinity: TextAffinity.downstream),
- this,
cause,
);
} else {
- onSelectionChanged(
+ _handlePotentialSelectionChange(
TextSelection.collapsed(offset: word.end, affinity: TextAffinity.upstream),
- this,
cause,
);
}
diff --git a/packages/flutter/test/cupertino/text_field_test.dart b/packages/flutter/test/cupertino/text_field_test.dart
index 6c8c988..7f3c3a5 100644
--- a/packages/flutter/test/cupertino/text_field_test.dart
+++ b/packages/flutter/test/cupertino/text_field_test.dart
@@ -367,13 +367,14 @@
const String testValue = 'A short phrase';
await tester.enterText(find.byType(CupertinoTextField), testValue);
+ await tester.pump();
await tester.tapAt(textOffsetToPosition(tester, testValue.length));
- await tester.pump();
+ await tester.pumpAndSettle();
await expectLater(
find.byKey(const ValueKey<int>(1)),
- matchesGoldenFile('text_field_cursor_test.0.1.png'),
+ matchesGoldenFile('text_field_cursor_test.0.2.png'),
);
}, skip: !Platform.isLinux);
@@ -395,14 +396,15 @@
const String testValue = 'A short phrase';
await tester.enterText(find.byType(CupertinoTextField), testValue);
+ await tester.pump();
await tester.tapAt(textOffsetToPosition(tester, testValue.length));
- await tester.pump();
+ await tester.pumpAndSettle();
debugDefaultTargetPlatformOverride = null;
await expectLater(
find.byKey(const ValueKey<int>(1)),
- matchesGoldenFile('text_field_cursor_test.1.1.png'),
+ matchesGoldenFile('text_field_cursor_test.1.2.png'),
);
}, skip: !Platform.isLinux);
diff --git a/packages/flutter/test/material/text_field_test.dart b/packages/flutter/test/material/text_field_test.dart
index 40f6ebc..4f7021f 100644
--- a/packages/flutter/test/material/text_field_test.dart
+++ b/packages/flutter/test/material/text_field_test.dart
@@ -735,6 +735,45 @@
expect(controller.selection.extentOffset, testValue.indexOf('f')+1);
});
+ testWidgets('Slight movements in longpress don\'t hide/show handles', (WidgetTester tester) async {
+ final TextEditingController controller = TextEditingController();
+
+ await tester.pumpWidget(
+ overlay(
+ child: TextField(
+ controller: controller,
+ ),
+ )
+ );
+
+ const String testValue = 'abc def ghi';
+ await tester.enterText(find.byType(TextField), testValue);
+ expect(controller.value.text, testValue);
+ await skipPastScrollingAnimation(tester);
+
+ expect(controller.selection.isCollapsed, true);
+
+ // Long press the 'e' to select 'def', but don't release the gesture.
+ final Offset ePos = textOffsetToPosition(tester, testValue.indexOf('e'));
+ final TestGesture gesture = await tester.startGesture(ePos, pointer: 7);
+ await tester.pump(const Duration(seconds: 2));
+ await tester.pumpAndSettle();
+
+ // Handles are shown
+ final Finder fadeFinder = find.byType(FadeTransition);
+ expect(fadeFinder, findsNWidgets(2)); // 2 handles, 1 toolbar
+ FadeTransition handle = tester.widget(fadeFinder.at(0));
+ expect(handle.opacity.value, equals(1.0));
+
+ // Move the gesture very slightly
+ await gesture.moveBy(const Offset(1.0, 1.0));
+ await tester.pump(TextSelectionOverlay.fadeDuration * 0.5);
+ handle = tester.widget(fadeFinder.at(0));
+
+ // The handle should still be fully opaque.
+ expect(handle.opacity.value, equals(1.0));
+ });
+
testWidgets('Mouse long press is just like a tap', (WidgetTester tester) async {
final TextEditingController controller = TextEditingController();
