Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cherry Pick: [SuperEditor][SuperTextField][iOS] Make caret word snapping less aggressive (Resolves #2152) #2485

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ class SuperEditorIosControlsController {
_shouldShowToolbar.dispose();
}

/// {@template ios_use_selection_heuristics}
/// Whether to adjust the user's selection similar to the way iOS does.
///
/// For example: iOS doesn't let users tap directly on a text offset. Instead,
Expand All @@ -139,6 +140,7 @@ class SuperEditorIosControlsController {
/// When this property is `true`, iOS-style heuristics should be used. When
/// this value is `false`, the user's gestures should directly impact the
/// area they touched.
/// {@endtemplate}
final bool useIosSelectionHeuristics;

/// Color of the text selection drag handles on iOS.
Expand Down Expand Up @@ -245,6 +247,7 @@ class IosDocumentTouchInteractor extends StatefulWidget {
required this.selection,
this.openKeyboardWhenTappingExistingSelection = true,
required this.openSoftwareKeyboard,
required this.isImeConnected,
required this.scrollController,
required this.dragHandleAutoScroller,
required this.fillViewport,
Expand All @@ -267,6 +270,10 @@ class IosDocumentTouchInteractor extends StatefulWidget {
/// A callback that should open the software keyboard when invoked.
final VoidCallback openSoftwareKeyboard;

/// A [ValueListenable] that should notify this widget when the IME connects
/// and disconnects.
final ValueListenable<bool> isImeConnected;

/// Optional list of handlers that respond to taps on content, e.g., opening
/// a link when the user taps on text with a link attribution.
///
Expand Down Expand Up @@ -632,10 +639,10 @@ class _IosDocumentTouchInteractorState extends State<IosDocumentTouchInteractor>

final didTapOnExistingSelection = selection != null &&
selection.isCollapsed &&
selection.extent.nodeId == adjustedSelectionPosition.nodeId &&
selection.extent.nodePosition.isEquivalentTo(adjustedSelectionPosition.nodePosition);
selection.extent.nodeId == docPosition.nodeId &&
selection.extent.nodePosition.isEquivalentTo(docPosition.nodePosition);

if (didTapOnExistingSelection && _isKeyboardOpen) {
if (didTapOnExistingSelection && widget.isImeConnected.value) {
// Toggle the toolbar display when the user taps on the collapsed caret,
// or on top of an existing selection.
//
Expand All @@ -648,31 +655,33 @@ class _IosDocumentTouchInteractorState extends State<IosDocumentTouchInteractor>
_controlsController!.hideToolbar();
}

final tappedComponent = _docLayout.getComponentByNodeId(adjustedSelectionPosition.nodeId)!;
if (!tappedComponent.isVisualSelectionSupported()) {
// The user tapped a non-selectable component.
// Place the document selection at the nearest selectable node
// to the tapped component.
moveSelectionToNearestSelectableNode(
editor: widget.editor,
document: widget.document,
documentLayoutResolver: widget.getDocumentLayout,
currentSelection: widget.selection.value,
startingNode: widget.document.getNodeById(adjustedSelectionPosition.nodeId)!,
);
return;
if (didTapOnExistingSelection) {
if (widget.openKeyboardWhenTappingExistingSelection) {
// The user tapped on the existing selection. Show the software keyboard.
//
// If the user didn't tap on an existing selection, the software keyboard will
// already be visible.
widget.openSoftwareKeyboard();
}
} else {
// Place the document selection at the location where the
// user tapped.
_selectPosition(adjustedSelectionPosition);
}

if (didTapOnExistingSelection && widget.openKeyboardWhenTappingExistingSelection) {
// The user tapped on the existing selection. Show the software keyboard.
//
// If the user didn't tap on an existing selection, the software keyboard will
// already be visible.
widget.openSoftwareKeyboard();
final tappedComponent = _docLayout.getComponentByNodeId(adjustedSelectionPosition.nodeId)!;
if (!tappedComponent.isVisualSelectionSupported()) {
// The user tapped a non-selectable component.
// Place the document selection at the nearest selectable node
// to the tapped component.
moveSelectionToNearestSelectableNode(
editor: widget.editor,
document: widget.document,
documentLayoutResolver: widget.getDocumentLayout,
currentSelection: widget.selection.value,
startingNode: widget.document.getNodeById(adjustedSelectionPosition.nodeId)!,
);
return;
} else {
// Place the document selection at the location where the
// user tapped.
_selectPosition(adjustedSelectionPosition);
}
}
} else {
widget.editor.execute([
Expand All @@ -684,16 +693,6 @@ class _IosDocumentTouchInteractorState extends State<IosDocumentTouchInteractor>
widget.focusNode.requestFocus();
}

/// Returns `true` if we *think* the software keyboard is currently open, or
/// `false` otherwise.
///
/// We say "think" because Flutter doesn't report this info to us. Instead, we
/// inspect the bottom insets on the window, and we assume any insets greater than
/// zero means a keyboard is visible.
bool get _isKeyboardOpen {
return MediaQuery.viewInsetsOf(context).bottom > 0;
}

DocumentPosition _moveTapPositionToWordBoundary(DocumentPosition docPosition) {
if (!SuperEditorIosControlsScope.rootOf(context).useIosSelectionHeuristics) {
// iOS-style adjustments aren't desired. Don't adjust th given position.
Expand Down Expand Up @@ -916,10 +915,15 @@ class _IosDocumentTouchInteractorState extends State<IosDocumentTouchInteractor>
}

final extentRect = _docLayout.getRectForPosition(collapsedPosition)!;
final caretRect = Rect.fromLTWH(extentRect.left - 1, extentRect.center.dy, 1, 1).inflate(24);
final caretHitArea = Rect.fromLTRB(
extentRect.left - 24,
extentRect.top,
extentRect.right + 24,
extentRect.bottom,
);

final docOffset = _interactorOffsetToDocumentOffset(interactorOffset);
return caretRect.contains(docOffset);
return caretHitArea.contains(docOffset);
}

bool _isOverBaseHandle(Offset interactorOffset) {
Expand Down
11 changes: 10 additions & 1 deletion super_editor/lib/src/default_editor/super_editor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,8 @@ class SuperEditorState extends State<SuperEditor> {

late SoftwareKeyboardController _softwareKeyboardController;

late ValueNotifier<bool> _isImeConnected;

@override
void initState() {
super.initState();
Expand All @@ -449,6 +451,8 @@ class SuperEditorState extends State<SuperEditor> {

_softwareKeyboardController = widget.softwareKeyboardController ?? SoftwareKeyboardController();

_isImeConnected = widget.isImeConnected ?? ValueNotifier(false);

widget.editor.context.put(
Editor.layoutKey,
DocumentLayoutEditable(() => _docLayoutKey.currentState as DocumentLayout),
Expand Down Expand Up @@ -513,6 +517,10 @@ class SuperEditorState extends State<SuperEditor> {
_softwareKeyboardController = widget.softwareKeyboardController ?? SoftwareKeyboardController();
}

if (widget.isImeConnected != oldWidget.isImeConnected) {
_isImeConnected = widget.isImeConnected ?? ValueNotifier(false);
}

_recomputeIfLayoutShouldShowCaret();
}

Expand Down Expand Up @@ -797,7 +805,7 @@ class SuperEditorState extends State<SuperEditor> {
..._keyboardActions,
],
selectorHandlers: widget.selectorHandlers ?? defaultEditorSelectorHandlers,
isImeConnected: widget.isImeConnected,
isImeConnected: _isImeConnected,
child: child,
);
}
Expand Down Expand Up @@ -900,6 +908,7 @@ class SuperEditorState extends State<SuperEditor> {
selection: editContext.composer.selectionNotifier,
openKeyboardWhenTappingExistingSelection: widget.selectionPolicies.openKeyboardWhenTappingExistingSelection,
openSoftwareKeyboard: _openSoftareKeyboard,
isImeConnected: _isImeConnected,
contentTapHandlers: _contentTapHandlers,
scrollController: _scrollController,
dragHandleAutoScroller: _dragHandleAutoScroller,
Expand Down
20 changes: 13 additions & 7 deletions super_editor/lib/src/super_textfield/ios/user_interaction.dart
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ final _log = iosTextFieldLog;
///
/// Selection changes are made via the given [textController].
class IOSTextFieldTouchInteractor extends StatefulWidget {
/// {@macro ios_use_selection_heuristics}
@visibleForTesting
static bool useIosSelectionHeuristics = true;

const IOSTextFieldTouchInteractor({
Key? key,
required this.focusNode,
Expand Down Expand Up @@ -201,15 +205,17 @@ class IOSTextFieldTouchInteractorState extends State<IOSTextFieldTouchInteractor
final exactTapTextPosition = _getTextPositionNearestToOffset(details.localPosition);
final adjustedTapTextPosition =
exactTapTextPosition != null ? _moveTapPositionToWordBoundary(exactTapTextPosition) : null;
final didTapOnExistingSelection = adjustedTapTextPosition != null &&
final didTapOnExistingSelection = exactTapTextPosition != null &&
_selectionBeforeTap != null &&
(_selectionBeforeTap!.isCollapsed
? adjustedTapTextPosition.offset == _selectionBeforeTap!.extent.offset
: adjustedTapTextPosition.offset >= _selectionBeforeTap!.start &&
adjustedTapTextPosition.offset <= _selectionBeforeTap!.end);
? exactTapTextPosition.offset == _selectionBeforeTap!.extent.offset
: exactTapTextPosition.offset >= _selectionBeforeTap!.start &&
exactTapTextPosition.offset <= _selectionBeforeTap!.end);

// Select the text that's nearest to where the user tapped.
_selectPosition(adjustedTapTextPosition);
if (!didTapOnExistingSelection) {
// Select the text that's nearest to where the user tapped.
_selectPosition(adjustedTapTextPosition);
}

final didCaretStayInSamePlace = _selectionBeforeTap != null &&
_selectionBeforeTap?.hasSameBoundsAs(widget.textController.selection) == true &&
Expand All @@ -232,7 +238,7 @@ class IOSTextFieldTouchInteractorState extends State<IOSTextFieldTouchInteractor
}

TextPosition _moveTapPositionToWordBoundary(TextPosition textPosition) {
if (Testing.isInTest) {
if (!IOSTextFieldTouchInteractor.useIosSelectionHeuristics) {
// Don't adjust the tap location in tests because we want tests to be
// able to precisely position the caret at a given offset.
// TODO: Make this decision configurable, similar to SuperEditor, so that
Expand Down
5 changes: 5 additions & 0 deletions super_editor/test/flutter_test_config.dart
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
import 'dart:async';

import 'package:super_editor/src/super_textfield/ios/ios_textfield.dart';
import 'package:super_editor/src/test/test_globals.dart';
import 'package:super_text_layout/super_text_layout.dart';

Future<void> testExecutable(FutureOr<void> Function() testMain) async {
// Disable indeterminate animations
BlinkController.indeterminateAnimationsEnabled = false;

// Disable iOS selection heuristics, i.e, place the caret at the exact
// tapped position instead of placing it at word boundaries.
IOSTextFieldTouchInteractor.useIosSelectionHeuristics = false;

Testing.isInTest = true;

return testMain();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import 'package:flutter/gestures.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:flutter_test_robots/flutter_test_robots.dart';
import 'package:flutter_test_runners/flutter_test_runners.dart';
import 'package:super_editor/super_editor.dart';
import 'package:super_editor/super_editor_test.dart';
import 'package:super_text_layout/super_text_layout.dart';

import '../../test_runners.dart';
import '../../test_tools.dart';
import '../supereditor_test_tools.dart';

void main() {
Expand Down Expand Up @@ -214,6 +217,67 @@ void main() {
expect(SuperEditorInspector.isCaretVisible(), true);
});

testWidgetsOnIos("keeps current selection when tapping on caret", (tester) async {
await _pumpSingleParagraphApp(tester, useIosSelectionHeuristics: true);

// Tap at "consectetur|" to place the caret.
await tester.tapInParagraph("1", 39);

// Ensure that the selection was placed at the end of the word.
expect(
SuperEditorInspector.findDocumentSelection(),
selectionEquivalentTo(const DocumentSelection.collapsed(
position: DocumentPosition(
nodeId: "1",
nodePosition: TextNodePosition(offset: 39),
),
)),
);

// Press and drag the caret to "con|sectetur" because dragging is the only way
// we can place the caret at the middle of a word when caret snapping is enabled.
final gesture = await tester.tapDownInParagraph("1", 39);
for (int i = 0; i < 7; i += 1) {
await gesture.moveBy(const Offset(-19, 0));
await tester.pump();
}

// Resolve the gesture so that we don't have pending gesture timers.
await gesture.up();
await tester.pump(kDoubleTapTimeout);

// Ensure that the selection moved to "con|sectetur".
expect(
SuperEditorInspector.findDocumentSelection(),
selectionEquivalentTo(const DocumentSelection.collapsed(
position: DocumentPosition(
nodeId: "1",
nodePosition: TextNodePosition(offset: 32),
),
)),
);

// Ensure the toolbar is not visible.
expect(SuperEditorInspector.isMobileToolbarVisible(), isFalse);

// Tap on the caret.
await tester.tapInParagraph("1", 32);

// Ensure the selection was kept at "con|sectetur".
expect(
SuperEditorInspector.findDocumentSelection(),
selectionEquivalentTo(const DocumentSelection.collapsed(
position: DocumentPosition(
nodeId: "1",
nodePosition: TextNodePosition(offset: 32),
),
)),
);

// Ensure the toolbar is visible.
expect(SuperEditorInspector.isMobileToolbarVisible(), isTrue);
});

group("on device and web > shows ", () {
testWidgetsOnIosDeviceAndWeb("caret", (tester) async {
await _pumpSingleParagraphApp(tester);
Expand Down Expand Up @@ -309,11 +373,15 @@ void main() {
});
}

Future<void> _pumpSingleParagraphApp(WidgetTester tester) async {
Future<void> _pumpSingleParagraphApp(
WidgetTester tester, {
bool useIosSelectionHeuristics = false,
}) async {
await tester
.createDocument()
// Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor...
.withSingleParagraph()
.simulateSoftwareKeyboardInsets(true)
.useIosSelectionHeuristics(useIosSelectionHeuristics)
.pump();
}
11 changes: 8 additions & 3 deletions super_editor/test/super_editor/supereditor_caret_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,18 @@ void main() {
final lineBreakOffset = SuperEditorInspector.findOffsetOfLineBreak('1');

// Find the coordinates of the caret at the end of the first line (line break offset w/ upstream affinity).
await tester.pump(kTapTimeout * 2); // Simulate a pause to avoid a double tap.
await tester.placeCaretInParagraph('1', lineBreakOffset, affinity: TextAffinity.upstream);
final upstreamCaretOffset = SuperEditorInspector.findCaretOffsetInDocument();

// The upstream caret should be at the same y and greater x than the caret at the start of the paragraph.
expect(upstreamCaretOffset.dx, greaterThan(startOfFirstLineCaretOffset.dx + xExpectBuffer));
expect(upstreamCaretOffset.dy, startOfFirstLineCaretOffset.dy);

// Tap on another character, because tapping on the same character shows the toolbar
// instead of changing the selection.
await tester.placeCaretInParagraph('1', 3);

// Find the coordinates of the caret at the start of the second line (line break offset w/ downstream affinity).
await tester.pump(kTapTimeout * 2); // Simulate a pause to avoid a double tap.
await tester.placeCaretInParagraph('1', lineBreakOffset, affinity: TextAffinity.downstream);
final downstreamCaretOffset = SuperEditorInspector.findCaretOffsetInDocument();

Expand All @@ -77,8 +79,11 @@ void main() {
final downstreamCaretOffset = SuperEditorInspector.findCaretOffsetInDocument();
final downstreamSelection = SuperEditorInspector.findDocumentSelection();

// Tap on another character, because tapping on the same character shows the toolbar
// instead of changing the selection.
await tester.placeCaretInParagraph('1', 0);

// Place the caret at the same offset but with an upstream affinity.
await tester.pump(kTapTimeout * 2); // Simulate a pause to avoid a double tap.
await tester.placeCaretInParagraph('1', textOffset, affinity: TextAffinity.upstream);
final upstreamCaretOffset = SuperEditorInspector.findCaretOffsetInDocument();
final upstreamSelection = SuperEditorInspector.findDocumentSelection();
Expand Down
5 changes: 4 additions & 1 deletion super_editor/test/super_editor/supereditor_robot_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,11 @@ void main() {
),
);

// Tap on another character, because tapping on the same character shows the toolbar
// instead of changing the selection.
await tester.placeCaretInParagraph("1", 5);

// Place the caret at the same offset as before but with an upstream affinity.
await tester.pump(kTapTimeout * 2); // Pause to avoid double tap.
await tester.placeCaretInParagraph("1", 1, affinity: TextAffinity.upstream);
// Ensure the document has the correct selection, including affinity;
expect(
Expand Down
Loading
Loading