Skip to content

Commit

Permalink
Fix inconsistencies when calculating start/end handle rects for selec…
Browse files Browse the repository at this point in the history 
…tion handles (#87236)
  • Loading branch information
@Renzo-Olivares
Renzo-Olivares committed Jul 29, 2021
1 parent cb4bc11 commit 63b6197
Show file tree
Hide file tree
Showing 2 changed files with 106 additions and 19 deletions.
39 changes: 20 additions & 19 deletions packages/flutter/lib/src/widgets/text_selection.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -602,6 +602,7 @@ class TextSelectionOverlay {
selectionControls: selectionControls,
position: position,
dragStartBehavior: dragStartBehavior,
selectionDelegate: selectionDelegate!,
),
);
}
Expand Down Expand Up @@ -695,6 +696,7 @@ class _TextSelectionHandleOverlay extends StatefulWidget {
required this.onSelectionHandleChanged,
required this.onSelectionHandleTapped,
required this.selectionControls,
required this.selectionDelegate,
this.dragStartBehavior = DragStartBehavior.start,
}) : super(key: key);

Expand All @@ -707,6 +709,7 @@ class _TextSelectionHandleOverlay extends StatefulWidget {
final VoidCallback? onSelectionHandleTapped;
final TextSelectionControls selectionControls;
final DragStartBehavior dragStartBehavior;
final TextSelectionDelegate selectionDelegate;

@override
_TextSelectionHandleOverlayState createState() => _TextSelectionHandleOverlayState();
Expand Down Expand Up @@ -833,34 +836,32 @@ class _TextSelectionHandleOverlayState
//
// For the end handle we compute the rectangles that encompass the range
// of the last full selected grapheme cluster at the end of the selection.
//
// Only calculate start/end handle rects if the text in the previous frame
// is the same as the text in the current frame. This is done because
// widget.renderObject contains the renderEditable from the previous frame.
// If the text changed between the current and previous frames then
// widget.renderObject.getRectForComposingRange might fail. In cases where
// the current frame is different from the previous we fall back to
// widget.renderObject.preferredLineHeight.
final InlineSpan span = widget.renderObject.text!;
final String text = span.toPlainText();
final String prevText = span.toPlainText();
final String currText = widget.selectionDelegate.textEditingValue.text;
final int firstSelectedGraphemeExtent;
final int lastSelectedGraphemeExtent;
final TextSelection? selection = widget.renderObject.selection;
final TextSelection selection = widget.selection;
Rect? startHandleRect;
Rect? endHandleRect;

if (selection != null && selection.isValid && !selection.isCollapsed) {
final String selectedGraphemes = selection.textInside(text);
if (prevText == currText && selection != null && selection.isValid && !selection.isCollapsed) {
final String selectedGraphemes = selection.textInside(currText);
firstSelectedGraphemeExtent = selectedGraphemes.characters.first.length;
lastSelectedGraphemeExtent = selectedGraphemes.characters.last.length;
assert(firstSelectedGraphemeExtent <= selectedGraphemes.length && lastSelectedGraphemeExtent <= selectedGraphemes.length);
} else {
// The call to selectedGraphemes.characters.first/last will throw a state
// error if the given text is empty, so fall back to first/last character
// range in this case.
//
// The call to widget.selection.textInside(text) will return a RangeError
// for a collapsed selection, fall back to this case when that happens.
firstSelectedGraphemeExtent = 0;
lastSelectedGraphemeExtent = 0;
startHandleRect = widget.renderObject.getRectForComposingRange(TextRange(start: selection.start, end: selection.start + firstSelectedGraphemeExtent));
endHandleRect = widget.renderObject.getRectForComposingRange(TextRange(start: selection.end - lastSelectedGraphemeExtent, end: selection.end));
}

final Rect? startHandleRect = widget.renderObject.getRectForComposingRange(TextRange(start: widget.selection.start, end: widget.selection.start + firstSelectedGraphemeExtent));
final Rect? endHandleRect = widget.renderObject.getRectForComposingRange(TextRange(start: widget.selection.end - lastSelectedGraphemeExtent, end: widget.selection.end));

assert(!(firstSelectedGraphemeExtent > 0 && widget.selection.isValid && !widget.selection.isCollapsed) || startHandleRect != null);
assert(!(lastSelectedGraphemeExtent > 0 && widget.selection.isValid && !widget.selection.isCollapsed) || endHandleRect != null);

final Offset handleAnchor = widget.selectionControls.getHandleAnchor(
type,
widget.renderObject.preferredLineHeight,
Expand Down
86 changes: 86 additions & 0 deletions packages/flutter/test/cupertino/text_selection_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -845,4 +845,90 @@ void main() {
skip: isBrowser, // We do not use Flutter-rendered context menu on the Web
variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.iOS }),
);

testWidgets('iOS selection handles scaling falls back to preferredLineHeight when the current frame does not match the previous', (WidgetTester tester) async {
await tester.pumpWidget(
const CupertinoApp(
home: Center(
child: SelectableText.rich(
TextSpan(
children: <InlineSpan>[
TextSpan(text: 'abc', style: TextStyle(fontSize: 40.0)),
TextSpan(text: 'def', style: TextStyle(fontSize: 50.0)),
],
),
),
),
),
);

final EditableText editableTextWidget = tester.widget(find.byType(EditableText));
final EditableTextState editableTextState = tester.state(find.byType(EditableText));
final TextEditingController controller = editableTextWidget.controller;

// Double tap to select the second word.
const int index = 4;
await tester.tapAt(textOffsetToPosition(tester, index));
await tester.pump(const Duration(milliseconds: 50));
await tester.tapAt(textOffsetToPosition(tester, index));
await tester.pumpAndSettle();
expect(editableTextState.selectionOverlay!.handlesAreVisible, isTrue);
expect(controller.selection.baseOffset, 0);
expect(controller.selection.extentOffset, 6);

// Drag the right handle 2 letters to the right. Placing the end handle on
// the third word. We use a small offset because the endpoint is on the very
// corner of the handle.
final TextSelection selection = controller.selection;
final RenderEditable renderEditable = findRenderEditable(tester);
final List<TextSelectionPoint> endpoints = globalize(
renderEditable.getEndpointsForSelection(selection),
renderEditable,
);
expect(endpoints.length, 2);

final Offset handlePos = endpoints[1].point + const Offset(1.0, 1.0);
final Offset newHandlePos = textOffsetToPosition(tester, 3);
final TestGesture gesture = await tester.startGesture(handlePos, pointer: 7);
await tester.pump();
await gesture.moveTo(newHandlePos);
await tester.pump();
await gesture.up();
await tester.pump();

expect(controller.selection.baseOffset, 0);
expect(controller.selection.extentOffset, 3);

// Find start and end handles and verify their sizes.
expect(find.byType(Overlay), findsOneWidget);
expect(find.descendant(
of: find.byType(Overlay),
matching: find.byType(CustomPaint),
), findsNWidgets(2));

final Iterable<RenderBox> handles = tester.renderObjectList(find.descendant(
of: find.byType(Overlay),
matching: find.byType(CustomPaint),
));

// The handle height is determined by the formula:
// textLineHeight + _kSelectionHandleRadius * 2 - _kSelectionHandleOverlap .
// The text line height will be the value of the fontSize.
// The constant _kSelectionHandleRadius has the value of 6.
// The constant _kSelectionHandleOverlap has the value of 1.5.
// In the case of the start handle, which is located on the word 'abc',
// 40.0 + 6 * 2 - 1.5 = 50.5 .
//
// We are now using the current frames selection and text in order to
// calculate the start and end handle heights (we fall back to preferredLineHeight
// when the current frame differs from the previous frame), where previously
// we would be using a mix of the previous and current frame. This could
// result in the start and end handle heights being calculated inaccurately
// if one of the handles falls between two varying text styles.
expect(handles.first.size.height, 50.5);
expect(handles.last.size.height, 50.5); // This is 60.5 with the previous frame.
},
skip: isBrowser, // We do not use Flutter-rendered context menu on the Web
variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.iOS }),
);
}

0 comments on commit 63b6197

Please sign in to comment.