[SuperEditor][SuperTextField][iOS] Make caret word snapping less aggressive (Resolves #2152) #2459

angelosilvestre · 2024-12-17T12:55:31Z

[SuperEditor][SuperTextField][iOS] Make caret word snapping less aggressive. Resolves #2152

On iOS, the caret snaps to the edges of a word. However, this shouldn't happen when the user taps at the caret position.

This PR changes SuperEditor and SuperTextField to avoid snapping the caret when tapping at the caret.

Some required adjustments:

The check for a tap over the caret was using the snapped position instead of the tap position. Modified to use the exact tap position.
The check for a tap over the caret was expanding the caret 24px in all directions. This causes an issue, because tapping on a line below (or up) can be interpreted as a tap on the caret. Modified to expand only horizontally.

We still have a problem in SuperEditor regarding showing the toolbar when tapping at the caret. We have this code:

if (didTapOnExistingSelection && _isKeyboardOpen) {
  // Toggle the toolbar display when the user taps on the collapsed caret,
  // or on top of an existing selection.
  //
  // But we only do this when the keyboard is already open. This is because
  // we don't want to show the toolbar when the user taps simply to open
  // the keyboard. That would feel unintentional, like a bug.
  _controlsController!.toggleToolbar();
} else {
  // The user tapped somewhere else in the document. Hide the toolbar.
  _controlsController!.hideToolbar();
}

However, _isKeyboardOpen uses the bottom view insets. This doesn't work if there are any Scaffolds in the tree without resizeToAvoidBottomInset set to false. Maybe we can check for an ancestor KeyboardScaffoldSafeArea and if we don't find one, we don't do the _isKeyboardOpen check.

angelosilvestre · 2024-12-17T15:07:58Z

@matthew-carroll

There are three failing tests that I'm not sure what to do:

SuperEditor robot taps to place caret at a non-linebreak offset with different affinities (on iOS)
SuperEditor text affinity upstream and downstream positions render differently at a line break (on iOS)
SuperEditor text affinity upstream and downstream positions render the same if not at a line break (on iOS)

All of these tests place the caret at a specific position, then try to place again in the same position, but with different affinity. So, the taps are being considered to be over the caret, and we are not changing the selection.

On Android, it seems we change the selection even when tapping at the caret.

Should these tests attempt to clear the selection before trying to set the new selection?

angelosilvestre · 2024-12-20T12:19:49Z

@matthew-carroll I modified also "SuperEditor text affinity upstream and downstream positions render the same if not at a line break (on iOS)" because it also uses the same document position with different affinities.

If this isn't ideal, then we probably should modify the checks to instead of looking at the document position look at the current caret rect.

super_editor/lib/src/super_textfield/ios/user_interaction.dart

matthew-carroll · 2024-12-23T23:18:49Z

super_editor/test/flutter_test_config.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.


Please mention what those heuristics are. The reader of this probably has no idea.

matthew-carroll · 2024-12-23T23:19:55Z

super_editor/test/super_editor/mobile/super_editor_ios_overlay_controls_test.dart

+        )),
+      );
+
+      // Press and drag the caret to "con|sectetur".


Please check all of these comments. For example, we say we're dragging the caret, but we show it in the same place. Down below we say we moved the caret...to the same place.

We are not showing it in the same place, we are dragging from offset 39 up to 32.

// con|sectetur // <------|

super_editor/test/super_editor/mobile/super_editor_ios_overlay_controls_test.dart

matthew-carroll · 2024-12-23T23:21:28Z

super_editor/test/super_editor/mobile/super_editor_ios_overlay_controls_test.dart

@@ -214,6 +217,63 @@ void main() {
      expect(SuperEditorInspector.isCaretVisible(), true);
    });

+    testWidgetsOnIos("keeps current selection when tapping on caret", (tester) async {


Between the name of this test, and the dragging happening in the test, it's not clear to me what we're really trying to demonstrate, or why....

I'm dragging the caret because that's the only way to simulate an user interaction to place the caret at a specific position that's not a word boundary.

Without dragging we need to manually issue a selection change request.

matthew-carroll · 2024-12-23T23:22:25Z

super_editor/test/super_editor/mobile/super_editor_ios_overlay_controls_test.dart

  await tester
      .createDocument()
      // Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor...
      .withSingleParagraph()
      .simulateSoftwareKeyboardInsets(true)
+      .useIosSelectionHeuristics(useIosSelectionHeuristics)


If this is configured on our editor configurator, then why do we also have it defined statically?

It defaults to false in the configurator. Here it allows tests to enable the heuristics.

matthew-carroll · 2024-12-23T23:23:48Z

super_editor/test/super_textfield/ios/super_textfield_ios_selection_test.dart

+        const TextSelection.collapsed(offset: -1),
+      );
+
+      // Tap at "ips|um" to place the caret at the end of the word.


Please mention how it ends up at the end of the word. When reading this comment in isolated, it looks like the first half of the comment contradicts the second half of the comment.

matthew-carroll · 2024-12-23T23:24:49Z

super_editor/test/super_textfield/ios/super_textfield_ios_selection_test.dart

@@ -53,6 +54,56 @@ void main() {
      // Ensure that the text field toolbar disappeared.
      expect(find.byType(IOSTextEditingFloatingToolbar), findsNothing);
    });
+
+    testWidgetsOnIos("keeps current selection when tapping on caret", (tester) async {


Similar to the other test - is this really just about tapping on a caret? Because dragging doesn't seem necessary for that specific check. Seems like we're missing some additional important details about the goal.

Dragging is the only user interaction that can place the caret at the middle of a word when the selection heuristics are enabled. Should I mention this in the comment?

super_editor/lib/src/default_editor/document_gestures_touch_ios.dart

…essive (Resolves #2152)

angelosilvestre · 2024-12-24T00:32:00Z

@matthew-carroll I pushed a change that I forgot to push before.

matthew-carroll

LGTM

…essive (Resolves #2152) (#2459)

angelosilvestre requested a review from matthew-carroll December 17, 2024 15:08

matthew-carroll reviewed Dec 23, 2024

View reviewed changes

angelosilvestre added 4 commits December 23, 2024 20:56

[SuperEditor][SuperTextField][iOS] Make caret word snapping less aggr…

73eeddf

…essive (Resolves #2152)

Test updates

f034c51

Test updates

8c30cbb

PR updates

da1b1eb

angelosilvestre force-pushed the 2152_caret-snapping branch from c0a2452 to da1b1eb Compare December 23, 2024 23:57

Fix merge conflict

5d26928

angelosilvestre requested a review from matthew-carroll December 24, 2024 00:32

angelosilvestre added 2 commits January 2, 2025 23:13

PR updates

f7039eb

Test update

9f48af8

matthew-carroll approved these changes Jan 3, 2025

View reviewed changes

matthew-carroll merged commit 3d5700f into main Jan 3, 2025
16 checks passed

matthew-carroll deleted the 2152_caret-snapping branch January 3, 2025 02:34

github-actions bot pushed a commit that referenced this pull request Jan 3, 2025

[SuperEditor][SuperTextField][iOS] Make caret word snapping less aggr…

143f300

…essive (Resolves #2152) (#2459)

github-actions bot mentioned this pull request Jan 3, 2025

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

Merged

matthew-carroll pushed a commit that referenced this pull request Jan 3, 2025

[SuperEditor][SuperTextField][iOS] Make caret word snapping less aggr…

b67dbfd

…essive (Resolves #2152) (#2459)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SuperEditor][SuperTextField][iOS] Make caret word snapping less aggressive (Resolves #2152) #2459

[SuperEditor][SuperTextField][iOS] Make caret word snapping less aggressive (Resolves #2152) #2459

angelosilvestre commented Dec 17, 2024

angelosilvestre commented Dec 17, 2024

angelosilvestre commented Dec 20, 2024

matthew-carroll Dec 23, 2024

angelosilvestre Dec 23, 2024

matthew-carroll Dec 23, 2024

angelosilvestre Dec 23, 2024

matthew-carroll Dec 23, 2024

angelosilvestre Dec 23, 2024

matthew-carroll Dec 23, 2024

angelosilvestre Dec 23, 2024

matthew-carroll Dec 23, 2024

angelosilvestre Dec 23, 2024

matthew-carroll Dec 23, 2024

angelosilvestre Dec 23, 2024

angelosilvestre commented Dec 24, 2024

matthew-carroll left a comment

[SuperEditor][SuperTextField][iOS] Make caret word snapping less aggressive (Resolves #2152) #2459

[SuperEditor][SuperTextField][iOS] Make caret word snapping less aggressive (Resolves #2152) #2459

Conversation

angelosilvestre commented Dec 17, 2024

angelosilvestre commented Dec 17, 2024

angelosilvestre commented Dec 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

angelosilvestre commented Dec 24, 2024

matthew-carroll left a comment

Choose a reason for hiding this comment