Skip to content

Commit

Permalink
[SuperEditor][SuperTextField][iOS] Make caret word snapping less aggr…
Browse files Browse the repository at this point in the history
…essive (Resolves #2152) (#2459)
  • Loading branch information
angelosilvestre authored Jan 3, 2025
1 parent ef6b359 commit 3d5700f
Show file tree
Hide file tree
Showing 8 changed files with 207 additions and 52 deletions.
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 @@ -39,6 +39,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 @@ -239,15 +243,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 Down Expand Up @@ -280,7 +286,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

0 comments on commit 3d5700f

Please sign in to comment.