Unverified Commit c80366a1 authored by Mouad Debbar's avatar Mouad Debbar Committed by GitHub

Avoid flickering while dragging to select text (#29563)

parent 469a859c
// Copyright 2018 The Chromium Authors. All rights reserved. // Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'package:flutter/gestures.dart';
import 'package:flutter/rendering.dart'; import 'package:flutter/rendering.dart';
import 'package:flutter/services.dart'; import 'package:flutter/services.dart';
import 'package:flutter/widgets.dart'; import 'package:flutter/widgets.dart';
...@@ -180,12 +181,14 @@ class CupertinoTextField extends StatefulWidget { ...@@ -180,12 +181,14 @@ class CupertinoTextField extends StatefulWidget {
this.cursorColor, this.cursorColor,
this.keyboardAppearance, this.keyboardAppearance,
this.scrollPadding = const EdgeInsets.all(20.0), this.scrollPadding = const EdgeInsets.all(20.0),
this.dragStartBehavior = DragStartBehavior.start,
}) : assert(textAlign != null), }) : assert(textAlign != null),
assert(autofocus != null), assert(autofocus != null),
assert(obscureText != null), assert(obscureText != null),
assert(autocorrect != null), assert(autocorrect != null),
assert(maxLengthEnforced != null), assert(maxLengthEnforced != null),
assert(scrollPadding != null), assert(scrollPadding != null),
assert(dragStartBehavior != null),
assert(maxLines == null || maxLines > 0), assert(maxLines == null || maxLines > 0),
assert(minLines == null || minLines > 0), assert(minLines == null || minLines > 0),
assert( assert(
...@@ -404,6 +407,9 @@ class CupertinoTextField extends StatefulWidget { ...@@ -404,6 +407,9 @@ class CupertinoTextField extends StatefulWidget {
/// {@macro flutter.widgets.editableText.scrollPadding} /// {@macro flutter.widgets.editableText.scrollPadding}
final EdgeInsets scrollPadding; final EdgeInsets scrollPadding;
/// {@macro flutter.widgets.scrollable.dragStartBehavior}
final DragStartBehavior dragStartBehavior;
@override @override
_CupertinoTextFieldState createState() => _CupertinoTextFieldState(); _CupertinoTextFieldState createState() => _CupertinoTextFieldState();
...@@ -703,6 +709,7 @@ class _CupertinoTextFieldState extends State<CupertinoTextField> with AutomaticK ...@@ -703,6 +709,7 @@ class _CupertinoTextFieldState extends State<CupertinoTextField> with AutomaticK
backgroundCursorColor: CupertinoColors.inactiveGray, backgroundCursorColor: CupertinoColors.inactiveGray,
scrollPadding: widget.scrollPadding, scrollPadding: widget.scrollPadding,
keyboardAppearance: keyboardAppearance, keyboardAppearance: keyboardAppearance,
dragStartBehavior: widget.dragStartBehavior,
), ),
), ),
); );
......
...@@ -1384,15 +1384,15 @@ class RenderEditable extends RenderBox { ...@@ -1384,15 +1384,15 @@ class RenderEditable extends RenderBox {
extentOffset = math.max(fromPosition.offset, toPosition.offset); extentOffset = math.max(fromPosition.offset, toPosition.offset);
} }
onSelectionChanged( final TextSelection newSelection = TextSelection(
TextSelection(
baseOffset: baseOffset, baseOffset: baseOffset,
extentOffset: extentOffset, extentOffset: extentOffset,
affinity: fromPosition.affinity, affinity: fromPosition.affinity,
),
this,
cause,
); );
// Call [onSelectionChanged] only when the selection actually changed.
if (newSelection != _selection) {
onSelectionChanged(newSelection, this, cause);
}
} }
} }
......
...@@ -8,6 +8,7 @@ import 'package:flutter/cupertino.dart'; ...@@ -8,6 +8,7 @@ import 'package:flutter/cupertino.dart';
import 'package:flutter/rendering.dart'; import 'package:flutter/rendering.dart';
import 'package:flutter/services.dart'; import 'package:flutter/services.dart';
import 'package:flutter/foundation.dart'; import 'package:flutter/foundation.dart';
import 'package:flutter/gestures.dart' show DragStartBehavior;
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
class MockClipboard { class MockClipboard {
...@@ -30,6 +31,45 @@ void main() { ...@@ -30,6 +31,45 @@ void main() {
final MockClipboard mockClipboard = MockClipboard(); final MockClipboard mockClipboard = MockClipboard();
SystemChannels.platform.setMockMethodCallHandler(mockClipboard.handleMethodCall); SystemChannels.platform.setMockMethodCallHandler(mockClipboard.handleMethodCall);
// Returns the first RenderEditable.
RenderEditable findRenderEditable(WidgetTester tester) {
final RenderObject root = tester.renderObject(find.byType(EditableText));
expect(root, isNotNull);
RenderEditable renderEditable;
void recursiveFinder(RenderObject child) {
if (child is RenderEditable) {
renderEditable = child;
return;
}
child.visitChildren(recursiveFinder);
}
root.visitChildren(recursiveFinder);
expect(renderEditable, isNotNull);
return renderEditable;
}
List<TextSelectionPoint> globalize(Iterable<TextSelectionPoint> points, RenderBox box) {
return points.map<TextSelectionPoint>((TextSelectionPoint point) {
return TextSelectionPoint(
box.localToGlobal(point.point),
point.direction,
);
}).toList();
}
Offset textOffsetToPosition(WidgetTester tester, int offset) {
final RenderEditable renderEditable = findRenderEditable(tester);
final List<TextSelectionPoint> endpoints = globalize(
renderEditable.getEndpointsForSelection(
TextSelection.collapsed(offset: offset),
),
renderEditable,
);
expect(endpoints.length, 1);
return endpoints[0].point + const Offset(0.0, -2.0);
}
testWidgets( testWidgets(
'takes available space horizontally and takes intrinsic space vertically no-strut', 'takes available space horizontally and takes intrinsic space vertically no-strut',
(WidgetTester tester) async { (WidgetTester tester) async {
...@@ -1688,6 +1728,69 @@ void main() { ...@@ -1688,6 +1728,69 @@ void main() {
expect(find.byType(CupertinoButton), findsNothing); expect(find.byType(CupertinoButton), findsNothing);
}); });
testWidgets('Cannot drag one handle past the other', (WidgetTester tester) async {
final TextEditingController controller = TextEditingController(
text: 'abc def ghi',
);
await tester.pumpWidget(
CupertinoApp(
home: Center(
child: CupertinoTextField(
dragStartBehavior: DragStartBehavior.down,
controller: controller,
style: const TextStyle(
fontFamily: 'Ahem',
fontSize: 10.0,
),
),
),
),
);
// Double tap on 'e' to select 'def'.
final Offset ePos = textOffsetToPosition(tester, 5);
await tester.tapAt(ePos, pointer: 7);
await tester.pump(const Duration(milliseconds: 50));
expect(controller.selection.isCollapsed, isTrue);
expect(controller.selection.baseOffset, 4);
await tester.tapAt(ePos, pointer: 7);
await tester.pump(const Duration(milliseconds: 50));
expect(controller.selection.baseOffset, 4);
expect(controller.selection.extentOffset, 7);
final RenderEditable renderEditable = findRenderEditable(tester);
final List<TextSelectionPoint> endpoints = globalize(
renderEditable.getEndpointsForSelection(controller.selection),
renderEditable,
);
expect(endpoints.length, 2);
// Drag the right handle until there's only 1 char selected.
// We use a small offset because the endpoint is on the very corner
// of the handle.
final Offset handlePos = endpoints[1].point;
Offset newHandlePos = textOffsetToPosition(tester, 5); // Position of 'e'.
final TestGesture gesture = await tester.startGesture(handlePos, pointer: 7);
await tester.pump();
await gesture.moveTo(newHandlePos);
await tester.pump();
expect(controller.selection.baseOffset, 4);
expect(controller.selection.extentOffset, 5);
newHandlePos = textOffsetToPosition(tester, 2); // Position of 'c'.
await gesture.moveTo(newHandlePos);
await tester.pump();
await gesture.up();
await tester.pump();
expect(controller.selection.baseOffset, 4);
// The selection doesn't move beyond the left handle. There's always at
// least 1 char selected.
expect(controller.selection.extentOffset, 5);
});
testWidgets( testWidgets(
'text field respects theme', 'text field respects theme',
(WidgetTester tester) async { (WidgetTester tester) async {
......
...@@ -645,6 +645,58 @@ void main() { ...@@ -645,6 +645,58 @@ void main() {
expect(controller.selection.extentOffset, testValue.indexOf('g')); expect(controller.selection.extentOffset, testValue.indexOf('g'));
}); });
testWidgets('Continuous dragging does not cause flickering', (WidgetTester tester) async {
int selectionChangedCount = 0;
const String testValue = 'abc def ghi';
final TextEditingController controller = TextEditingController(text: testValue);
controller.addListener(() {
selectionChangedCount++;
});
await tester.pumpWidget(
MaterialApp(
home: Material(
child: TextField(
dragStartBehavior: DragStartBehavior.down,
controller: controller,
style: const TextStyle(fontFamily: 'Ahem', fontSize: 10.0),
),
),
),
);
final Offset cPos = textOffsetToPosition(tester, 2); // Index of 'c'.
final Offset gPos = textOffsetToPosition(tester, 8); // Index of 'g'.
final Offset hPos = textOffsetToPosition(tester, 9); // Index of 'h'.
// Drag from 'c' to 'g'.
final TestGesture gesture = await tester.startGesture(cPos, kind: PointerDeviceKind.mouse);
await tester.pump();
await gesture.moveTo(gPos);
await tester.pumpAndSettle();
expect(selectionChangedCount, isNonZero);
selectionChangedCount = 0;
expect(controller.selection.baseOffset, 2);
expect(controller.selection.extentOffset, 8);
// Tiny movement shouldn't cause text selection to change.
await gesture.moveTo(gPos + const Offset(4.0, 0.0));
await tester.pumpAndSettle();
expect(selectionChangedCount, 0);
// Now a text selection change will occur after a significant movement.
await gesture.moveTo(hPos);
await tester.pump();
await gesture.up();
await tester.pumpAndSettle();
expect(selectionChangedCount, 1);
expect(controller.selection.baseOffset, 2);
expect(controller.selection.extentOffset, 9);
});
testWidgets('Dragging in opposite direction also works', (WidgetTester tester) async { testWidgets('Dragging in opposite direction also works', (WidgetTester tester) async {
final TextEditingController controller = TextEditingController(); final TextEditingController controller = TextEditingController();
...@@ -712,14 +764,12 @@ void main() { ...@@ -712,14 +764,12 @@ void main() {
final TextEditingController controller = TextEditingController(); final TextEditingController controller = TextEditingController();
await tester.pumpWidget( await tester.pumpWidget(
MaterialApp( overlay(
home: Material(
child: TextField( child: TextField(
dragStartBehavior: DragStartBehavior.down, dragStartBehavior: DragStartBehavior.down,
controller: controller, controller: controller,
), ),
), ),
),
); );
const String testValue = 'abc def ghi'; const String testValue = 'abc def ghi';
...@@ -735,6 +785,8 @@ void main() { ...@@ -735,6 +785,8 @@ void main() {
await tester.pump(const Duration(milliseconds: 200)); // skip past the frame where the opacity is zero await tester.pump(const Duration(milliseconds: 200)); // skip past the frame where the opacity is zero
final TextSelection selection = controller.selection; final TextSelection selection = controller.selection;
expect(selection.baseOffset, 4);
expect(selection.extentOffset, 7);
final RenderEditable renderEditable = findRenderEditable(tester); final RenderEditable renderEditable = findRenderEditable(tester);
final List<TextSelectionPoint> endpoints = globalize( final List<TextSelectionPoint> endpoints = globalize(
...@@ -747,7 +799,7 @@ void main() { ...@@ -747,7 +799,7 @@ void main() {
// We use a small offset because the endpoint is on the very corner // We use a small offset because the endpoint is on the very corner
// of the handle. // of the handle.
Offset handlePos = endpoints[1].point + const Offset(1.0, 1.0); Offset handlePos = endpoints[1].point + const Offset(1.0, 1.0);
Offset newHandlePos = textOffsetToPosition(tester, selection.extentOffset+2); Offset newHandlePos = textOffsetToPosition(tester, 9); // Position of 'h'.
gesture = await tester.startGesture(handlePos, pointer: 7); gesture = await tester.startGesture(handlePos, pointer: 7);
await tester.pump(); await tester.pump();
await gesture.moveTo(newHandlePos); await gesture.moveTo(newHandlePos);
...@@ -755,12 +807,12 @@ void main() { ...@@ -755,12 +807,12 @@ void main() {
await gesture.up(); await gesture.up();
await tester.pump(); await tester.pump();
expect(controller.selection.baseOffset, selection.baseOffset); expect(controller.selection.baseOffset, 4);
expect(controller.selection.extentOffset, selection.extentOffset); expect(controller.selection.extentOffset, 9);
// Drag the left handle 2 letters to the left. // Drag the left handle 2 letters to the left.
handlePos = endpoints[0].point + const Offset(-1.0, 1.0); handlePos = endpoints[0].point + const Offset(-1.0, 1.0);
newHandlePos = textOffsetToPosition(tester, selection.baseOffset-2); newHandlePos = textOffsetToPosition(tester, 2); // Position of 'c'.
gesture = await tester.startGesture(handlePos, pointer: 7); gesture = await tester.startGesture(handlePos, pointer: 7);
await tester.pump(); await tester.pump();
await gesture.moveTo(newHandlePos); await gesture.moveTo(newHandlePos);
...@@ -768,8 +820,68 @@ void main() { ...@@ -768,8 +820,68 @@ void main() {
await gesture.up(); await gesture.up();
await tester.pump(); await tester.pump();
expect(controller.selection.baseOffset, selection.baseOffset); expect(controller.selection.baseOffset, 2);
expect(controller.selection.extentOffset, selection.extentOffset); expect(controller.selection.extentOffset, 9);
});
testWidgets('Cannot drag one handle past the other', (WidgetTester tester) async {
final TextEditingController controller = TextEditingController();
await tester.pumpWidget(
overlay(
child: TextField(
dragStartBehavior: DragStartBehavior.down,
controller: controller,
),
),
);
const String testValue = 'abc def ghi';
await tester.enterText(find.byType(TextField), testValue);
await skipPastScrollingAnimation(tester);
// Long press the 'e' to select 'def'.
final Offset ePos = textOffsetToPosition(tester, 5); // Position of 'e'.
TestGesture gesture = await tester.startGesture(ePos, pointer: 7);
await tester.pump(const Duration(seconds: 2));
await gesture.up();
await tester.pump();
await tester.pump(const Duration(milliseconds: 200)); // skip past the frame where the opacity is zero
final TextSelection selection = controller.selection;
expect(selection.baseOffset, 4);
expect(selection.extentOffset, 7);
final RenderEditable renderEditable = findRenderEditable(tester);
final List<TextSelectionPoint> endpoints = globalize(
renderEditable.getEndpointsForSelection(selection),
renderEditable,
);
expect(endpoints.length, 2);
// Drag the right handle until there's only 1 char selected.
// We use a small offset because the endpoint is on the very corner
// of the handle.
final Offset handlePos = endpoints[1].point + const Offset(1.0, 1.0);
Offset newHandlePos = textOffsetToPosition(tester, 5); // Position of 'e'.
gesture = await tester.startGesture(handlePos, pointer: 7);
await tester.pump();
await gesture.moveTo(newHandlePos);
await tester.pump();
expect(controller.selection.baseOffset, 4);
expect(controller.selection.extentOffset, 5);
newHandlePos = textOffsetToPosition(tester, 2); // Position of 'c'.
await gesture.moveTo(newHandlePos);
await tester.pump();
await gesture.up();
await tester.pump();
expect(controller.selection.baseOffset, 4);
// The selection doesn't move beyond the left handle. There's always at
// least 1 char selected.
expect(controller.selection.extentOffset, 5);
}); });
testWidgets('Can use selection toolbar', (WidgetTester tester) async { testWidgets('Can use selection toolbar', (WidgetTester tester) async {
......
...@@ -367,4 +367,59 @@ void main() { ...@@ -367,4 +367,59 @@ void main() {
expect(currentSelection.baseOffset, 1); expect(currentSelection.baseOffset, 1);
expect(currentSelection.extentOffset, 3); expect(currentSelection.extentOffset, 3);
}); });
test('selection does not flicker as user is dragging', () {
int selectionChangedCount = 0;
TextSelection updatedSelection;
final TextSelectionDelegate delegate = FakeEditableTextState();
const TextSpan text = TextSpan(
text: 'abc def ghi',
style: TextStyle(
height: 1.0, fontSize: 10.0, fontFamily: 'Ahem',
),
);
final RenderEditable editable1 = RenderEditable(
textSelectionDelegate: delegate,
textDirection: TextDirection.ltr,
offset: ViewportOffset.zero(),
selection: const TextSelection(baseOffset: 3, extentOffset: 4),
onSelectionChanged: (TextSelection selection, RenderEditable renderObject, SelectionChangedCause cause) {
selectionChangedCount++;
updatedSelection = selection;
},
text: text,
);
layout(editable1);
// Shouldn't cause a selection change.
editable1.selectPositionAt(from: const Offset(30, 2), to: const Offset(42, 2), cause: SelectionChangedCause.drag);
pumpFrame();
expect(updatedSelection, isNull);
expect(selectionChangedCount, 0);
final RenderEditable editable2 = RenderEditable(
textSelectionDelegate: delegate,
textDirection: TextDirection.ltr,
offset: ViewportOffset.zero(),
selection: const TextSelection(baseOffset: 3, extentOffset: 4),
onSelectionChanged: (TextSelection selection, RenderEditable renderObject, SelectionChangedCause cause) {
selectionChangedCount++;
updatedSelection = selection;
},
text: text,
);
layout(editable2);
// Now this should cause a selection change.
editable2.selectPositionAt(from: const Offset(30, 2), to: const Offset(48, 2), cause: SelectionChangedCause.drag);
pumpFrame();
expect(updatedSelection.baseOffset, 3);
expect(updatedSelection.extentOffset, 5);
expect(selectionChangedCount, 1);
});
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment