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.

bin/internal/goldens.version

@@ -1 +1 @@
-5e0cf0b9b347526d299718a4755e2d65cffacaba
+25ab3c95b1ac02a41973c05f96e664370857ce55

packages/flutter/lib/src/rendering/editable.dart

@@ -350,6 +350,18 @@ class RenderEditable extends RenderBox {
   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 @@ class RenderEditable extends RenderBox {
     // 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 @@ class RenderEditable extends RenderBox {
         else if (rightArrow)
           newOffset = _baseOffset > _extentOffset ? _baseOffset : _extentOffset;
       }
-      onSelectionChanged(
+      _handlePotentialSelectionChange(
         TextSelection.fromPosition(
           TextPosition(
             offset: newOffset
           )
         ),
-        this,
         SelectionChangedCause.keyboard,
       );
     }
@@ -564,12 +573,11 @@ class RenderEditable extends RenderBox {
       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 @@ class RenderEditable extends RenderBox {
   }
 
   void _handleSetSelection(TextSelection selection) {
-    onSelectionChanged(selection, this, SelectionChangedCause.keyboard);
+    _handlePotentialSelectionChange(selection, SelectionChangedCause.keyboard);
   }
 
   void _handleMoveCursorForwardByCharacter(bool extentSelection) {
@@ -975,8 +983,8 @@ class RenderEditable extends RenderBox {
     if (extentOffset == null)
       return;
     final int baseOffset = !extentSelection ? extentOffset : _selection.baseOffset;
-    onSelectionChanged(
+    _handlePotentialSelectionChange(
-      TextSelection(baseOffset: baseOffset, extentOffset: extentOffset), this, SelectionChangedCause.keyboard,
+      TextSelection(baseOffset: baseOffset, extentOffset: extentOffset), SelectionChangedCause.keyboard,
     );
   }
 
@@ -985,8 +993,8 @@ class RenderEditable extends RenderBox {
     if (extentOffset == null)
       return;
     final int baseOffset = !extentSelection ? extentOffset : _selection.baseOffset;
-    onSelectionChanged(
+    _handlePotentialSelectionChange(
-      TextSelection(baseOffset: baseOffset, extentOffset: extentOffset), this, SelectionChangedCause.keyboard,
+      TextSelection(baseOffset: baseOffset, extentOffset: extentOffset), SelectionChangedCause.keyboard,
     );
   }
 
@@ -998,12 +1006,11 @@ class RenderEditable extends RenderBox {
     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 @@ class RenderEditable extends RenderBox {
     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 @@ class RenderEditable extends RenderBox {
       );
       // Call [onSelectionChanged] only when the selection actually changed.
       if (newSelection != _selection) {
-        onSelectionChanged(newSelection, this, cause);
+        _handlePotentialSelectionChange(newSelection, cause);
       }
     }
   }
@@ -1428,12 +1434,13 @@ class RenderEditable extends RenderBox {
       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 @@ class RenderEditable extends RenderBox {
       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,
         );
       }

packages/flutter/test/cupertino/text_field_test.dart

@@ -367,13 +367,14 @@ void main() {
 
     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 @@ void main() {
 
     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);
 

packages/flutter/test/material/text_field_test.dart

@@ -735,6 +735,45 @@ void main() {
     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();