iOS Selection Handle Improvements #157815

Renzo-Olivares · 2024-10-29T17:32:23Z

Fixes #110306

Simulator.Screen.Recording.-.iPhone.16.Pro.-.2024-10-29.at.10.43.14.mp4

This change:

Allows selection handles on iOS to swap with each other when inverting on TextField.
Allows selection handles to visually collapse when inverting on SelectableRegion/SelectionArea, previously they showed both left and right handles when collapsed, instead of the collapsed handles.
Adds a border to the CupertinoTextMagnifier, the same color as the selection handles to match native iOS behavior.

SelectionOverlay:

Previously would build an empty end handle when the selection was collapsed. Now it builds an empty end handle when the selection is being collapsed and the start handle is being dragged, and when the selection is collapsed and no handle is being dragged.
Hides start handle when the selection is being collapsed and the end handle is being dragged.
Keeps the handles from overlapping.

TextSelectionOverlay:

Removes guards against swapping handles for iOS and macOS.
Tracks _oppositeEdge used to maintain selection as handles invert.

RenderParagraph:

Send collapsed selection handle state in SelectionGeometry, previously we wouldn't so the collapsed state would show both start and end handles.

CupertinoTextMagnifier:

Inherit border color from parent CupertinoTheme.of(context).primaryColor. Selection handles also uses primaryColor.

Pre-launch Checklist

I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
I read the [Tree Hygiene] wiki page, which explains my responsibilities.
I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
I signed the [CLA].
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is [test-exempt].
I followed the [breaking change policy] and added [Data Driven Fixes] where supported.
All existing and new tests are passing.

MaherSafadii · 2024-10-30T12:47:51Z

the iOS one has this hero like animation when tapping on one of the handles which makes the handle expand to the magnifying glass, can you pull this off?

justinmc · 2024-11-01T23:00:27Z

@MaherSafadii That's pretty cool, I didn't realize that:

It's actually not too far away with @Renzo-Olivares's changes. It's probably doable. No pressure in this PR though!

Renzo-Olivares · 2024-11-07T22:07:31Z

packages/flutter/lib/src/rendering/editable.dart

        super.paint,
        Offset.zero,
      );
+    } else if (selection!.isCollapsed) {


@justinmc I'm wondering if this change is worth while, i'm not seeing any issues without it. Previously when the selection was collapsed SelectionOverlay would build an empty end handle and RenderEditable would only push a layer for the start handle. Now SelectionOverlay may build and empty end handle or start handle depending on certain conditions.

When the selection is collapsed with no handles being dragged, then build an empty end handle, this case would be alright without this code since the end handle is not built.

When the selection is collapsed with the start handle being dragged, then build an empty end handle, this case would also be fine without this code since the end handle is not built.

When the selection is collapsed with the end handle being dragged, then build an empty start handle, this case might not be fine without this code since we are dragging the end handle and its layer won't be there.

This is also the cause of the failing editable_test.dart test.

My first reaction is to not paint this since it's never going to be visible (it always overlaps the start handle perfectly). There won't be any accessibility implications right?

It looks like the failing test just tests the number of layers painted so it shouldn't affect the decision.

00:52 +1212 ~8 -1: /b/s/w/ir/x/w/flutter/packages/flutter/test/rendering/editable_test.dart: RenderEditable.paint respects offset argument [E] Expected: an object with length of <1> Actual: [ LeaderLayer:LeaderLayer#e5a4a DETACHED(handles: 2, offset: Offset(100.0, 220.0), link: LayerLink#9f416(<linked>)), LeaderLayer:LeaderLayer#8743c DETACHED(handles: 1, offset: Offset(100.0, 220.0), link: LayerLink#d0324(<linked>)) ] Which: has length of <2> _paintHandleLayers will paint a LeaderLayer package:matcher expect package:flutter_test/src/widget_tester.dart 480:18 expect test/rendering/editable_test.dart 200:5 main.<fn>

I'm leaning towards keeping this in for the case of:

When the selection is collapsed with the end handle being dragged, then without the code only the start handle layer is pushed. This seems wrong since the end handle is being dragged and its layer will not be there.

RenderParagraph does something similar (pushes both layers when collapsed). https://github.com/flutter/flutter/blob/ae736e2cc325998df328595e039208d545128265/packages/flutter/lib/src/rendering/paragraph.dart#L3086C1-L3105C6

Sounds good.

Renzo-Olivares · 2024-11-07T22:13:58Z

@MaherSafadii, that is a cool behavior. I probably won't get to it in this PR, but will definitely add it to my radar.

justinmc

LGTM with nits 👍 . Can you create an issue for the handle animation (#157815 (comment))?

justinmc · 2024-11-07T23:56:21Z

packages/flutter/lib/src/rendering/editable.dart

        super.paint,
        Offset.zero,
      );
+    } else if (selection!.isCollapsed) {


My first reaction is to not paint this since it's never going to be visible (it always overlaps the start handle perfectly). There won't be any accessibility implications right?

It looks like the failing test just tests the number of layers painted so it shouldn't affect the decision.

00:52 +1212 ~8 -1: /b/s/w/ir/x/w/flutter/packages/flutter/test/rendering/editable_test.dart: RenderEditable.paint respects offset argument [E] Expected: an object with length of <1> Actual: [ LeaderLayer:LeaderLayer#e5a4a DETACHED(handles: 2, offset: Offset(100.0, 220.0), link: LayerLink#9f416(<linked>)), LeaderLayer:LeaderLayer#8743c DETACHED(handles: 1, offset: Offset(100.0, 220.0), link: LayerLink#d0324(<linked>)) ] Which: has length of <2> _paintHandleLayers will paint a LeaderLayer package:matcher expect package:flutter_test/src/widget_tester.dart 480:18 expect test/rendering/editable_test.dart 200:5 main.<fn>

justinmc · 2024-11-08T00:03:23Z

packages/flutter/lib/src/widgets/text_selection.dart

  late double _endHandleDragTarget;

+  // The edge to hold in place when the selection is inverted.
+  int? _oppositeEdge;


Nit: Maybe pivotPoint or something? Not sure if that's any more understandable, up to you...

Or rather than being an integer, should it just indicate whether the start or end handle is being dragged? I feel like the indicator contains information that's duplicated in _selection.

Thinking about this some more I think you're right. I think instead of having _oppositeEdge and _dragStartSelectionIsCollapsed, I can just store _dragStartSelection and use it directly. I think we know what handle is being dragged depending on what method we are in _handleSelectionEndHandleDragUpdate and _handleSelectionStartHandleDragUpdate.

Thinking out loud about some of the behaviors:

End handle:
When the selection is normalized and we drag the end handle, for the duration of the drag we can hold the baseOffset in place at the base.
When the selection is inverted and we drag the end handle, for the duration of the drag we can hold the extentOffset in place at the base.

Start handle:
When the selection is normalized and we drag the start handle, for the duration of the drag we can hold the extentOffset in place at the base.
When the selection is inverted and we drag the start handle, for the duration of the drag we can hold the baseOffset in place at the base.

Concrete example:
selection format: (baseOffset, extentOffset)
initial selection (end handle at position 10): (5,10)
drag end handle forward (to position 15): (5, 15)
drag end handle backwards & invert (to position 2) - release drag: (5, 2)
new end handle drag forward (to position 8) (end handle is on position 5 and iOS always sets the new offset as the extentOffset) : (2, 8) since the start handle is on position 2, so that position should be held in place while dragging the end handle.

justinmc · 2024-11-08T00:07:59Z

packages/flutter/test/cupertino/text_field_test.dart

    // The selection doesn't move beyond the left handle. There's always at
    // least 1 char selected.
    expect(controller.selection.extentOffset, 5);
+  });


Should this run only on non-Apple platforms?

justinmc · 2024-11-08T00:08:14Z

packages/flutter/test/cupertino/text_field_test.dart

+    // The selection inverts moving beyond the left handle.
+    expect(controller.selection.baseOffset, 4);
+    expect(controller.selection.extentOffset, 2);
  }, variant: const TargetPlatformVariant(<TargetPlatform>{ TargetPlatform.iOS }));


And macOS too?

I set it to run on iOS only since:

There are no touch devices running macOS afaik, and selection handles can only be dragged with a touch pointer device.

flutter/packages/flutter/lib/src/cupertino/text_field.dart

Lines 1121 to 1127 in 74669e4

case TargetPlatform.macOS:

case TargetPlatform.linux:

case TargetPlatform.windows:

if (cause == SelectionChangedCause.drag) {

_editableText.hideToolbar();

}

}

This case triggers when we drag selection handles on macOS and immediately causes the handles to hide when dragging them, causing the test to fail after the handle is dragged initially since the handle disappears.

I think we need to revisit how handles behave on desktop devices, there is a related issue open #108668. By default desktop platforms do not render selection handles right now, their TextSelectionControls explicitly block this. Right now you can provide a TextSelectionControls to TextField that allows the selection handles to render on desktop platforms, but using a mouse to manipulate them does not work since they only except touch pointer events. The case pointed out in #157815 (comment) also prevents selection handles from working properly on desktop platforms since they disappear immediately.

Ok got it. Similarly I think we messed up mouse input on mobile devices for awhile, but it should work just like it does natively on those devices. So macOS is no big deal, but we should solve #108668 for Windows and Linux.

Piinks · 2024-11-13T23:10:01Z

(Triage flyby comment) @Renzo-Olivares it looks like some checks are failing. :)

flutter-dashboard · 2024-12-03T23:11:39Z

This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Renzo-Olivares · 2024-12-05T00:37:54Z

The tests are fixed, but thinking about this more I wonder if this is the correct behavior. After this PR on iOS devices when providing any selection handles, whether Cupertino or Material, swapping handles is enabled. On other platforms selection handles does not allow for handle swapping at all.

Just taking into account native behavior, I'm leaning towards only CupertinoTextSelectionHandleControls having the ability to swap handles. Native Android material selection handles do not allow for order swapping. Maybe we can add to TextSelectionControls the ability to control whether the handles can swap. This way a user can provide their own TextSelectionControls that swap if they want them to regardless of the platform, and we can use this flag to control the default behavior of the cupertino and material handles. For example a user can provide a MaterialTextSelectionControls, with canSwap set to true, to allow the material handles to swap. Curious to hear any thoughts on this.

edit: This may be more involved since SelectableRegion/SelectionArea also needs a way to prevent the handles from swapping by using the canSwap.

flutter-dashboard · 2024-12-05T01:18:49Z

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #157815 at sha 70bb3ab

justinmc

We talked offline and I advocated keeping the PR as-is, so that the behavior is dependent on the platform and not on the visual handle style, but I'm open for a way to modify the behavior like swapHandles in the future.

Renewing my LGTM 👍

flutter-dashboard · 2024-12-06T20:38:31Z

Golden file changes are available for triage from new commit, Click here to view.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #157815 at sha f1fc476

justinmc

LGTM 👍

Thanks for doing this, it seems like it will greatly improve the look and feel here.

justinmc · 2024-12-06T23:24:30Z

packages/flutter/lib/src/widgets/text_selection.dart

    }

    final TextSelection newSelection;
+    final bool dragStartSelectionNormalized = _dragStartSelection!.extentOffset >= _dragStartSelection!.baseOffset;


Could you use _dragStartSelection.isNormalized? Here and below.

isNormalized seemed to behave a bit strangely to me, maybe TextSelection should have an override for it, but the default implementation checks end >= start, which will always be true with regards to a TextSelection, since TextSelection passes normalized values to TextRange

flutter/packages/flutter/lib/src/services/text_editing.dart

Lines 20 to 23 in 5621a5b

}) : super(

start: baseOffset < extentOffset ? baseOffset : extentOffset,

end: baseOffset < extentOffset ? extentOffset : baseOffset,

);

. What do you think?

Ah ok, keeping it as-is is fine then. I think I've run into the same thing before. Maybe add a comment to avoid someone else coming in and moving this to _dragStartSelection.isNormalized if you think it might help.

justinmc · 2024-12-06T23:28:42Z

packages/flutter/lib/src/widgets/text_selection.dart

-        );
-        if (position.offset <= _selection.start) {
-          return; // Don't allow order swapping.
+        if (dragStartSelectionNormalized) {


Nit: You could also remove this if/else and just do a ternary for the baseOffset. Either way is fine though.

justinmc · 2024-12-06T23:33:23Z

packages/flutter/lib/src/rendering/editable.dart

        super.paint,
        Offset.zero,
      );
+    } else if (selection!.isCollapsed) {


Sounds good.

justinmc · 2024-12-06T23:34:27Z

packages/flutter/lib/src/rendering/paragraph.dart

+                    ? TextSelectionHandleType.right
+                    : TextSelectionHandleType.left,


Nit: Would it be easier to read if you indented these two lines? Same for below.

Or you could always do a switch, but I'm not sure if that would help with readability or not here.

I think I like the switch better 48f01fc (#157815) .

…n place

github-actions bot added a: text input Entering text in a text field or keyboard related problems framework flutter/packages/flutter repository. See also f: labels. f: cupertino flutter/packages/flutter/cupertino repository labels Oct 29, 2024

Renzo-Olivares force-pushed the ios-handle-swap branch 2 times, most recently from 9c8c8b9 to 8fd31ea Compare November 7, 2024 21:05

Renzo-Olivares marked this pull request as ready for review November 7, 2024 21:08

Renzo-Olivares requested a review from justinmc November 7, 2024 21:49

Renzo-Olivares commented Nov 7, 2024

View reviewed changes

justinmc approved these changes Nov 8, 2024

View reviewed changes

Renzo-Olivares force-pushed the ios-handle-swap branch from 7dda689 to ef7d70e Compare November 12, 2024 09:14

Renzo-Olivares force-pushed the ios-handle-swap branch from ef7d70e to 2d7f8d8 Compare December 4, 2024 21:55

flutter-dashboard bot added the will affect goldens Changes to golden files label Dec 5, 2024

justinmc approved these changes Dec 5, 2024

View reviewed changes

github-actions bot added the f: material design flutter/packages/flutter/material repository. label Dec 6, 2024

Renzo-Olivares force-pushed the ios-handle-swap branch from bd0fef7 to f1fc476 Compare December 6, 2024 19:59

github-actions bot removed the f: material design flutter/packages/flutter/material repository. label Dec 6, 2024

justinmc approved these changes Dec 6, 2024

View reviewed changes

Renzo-Olivares added 5 commits December 9, 2024 12:10

ios handle swap

4c87598

check position.offset on handle drag down to determine edge to hold i…

820db15

…n place

clean up

a3b69fc

clean up

adfd15f

update magnifier

483dffe

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Dec 9, 2024

engine-flutter-autoroll mentioned this pull request Dec 10, 2024

Manual roll Flutter from ab5b20c16d56 to 129a35a5e186 (49 revisions) flutter/packages#8257

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 10, 2024

0737dbf iOS Selection Handle Improvements (flutter/flutter#157815)

8ce5d40

engine-flutter-autoroll mentioned this pull request Dec 10, 2024

Roll Flutter from ab5b20c16d56 to 2eee054d41f6 (53 revisions) flutter/packages#8258

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 10, 2024

0737dbf iOS Selection Handle Improvements (flutter/flutter#157815)

bb91fb9

engine-flutter-autoroll mentioned this pull request Dec 10, 2024

Roll Flutter from ab5b20c16d56 to 4b1430841359 (54 revisions) flutter/packages#8259

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 10, 2024

0737dbf iOS Selection Handle Improvements (flutter/flutter#157815)

9c7e90f

engine-flutter-autoroll mentioned this pull request Dec 10, 2024

Roll Flutter from ab5b20c16d56 to a5feed55157a (55 revisions) flutter/packages#8260

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 10, 2024

0737dbf iOS Selection Handle Improvements (flutter/flutter#157815)

f7ca6a9

engine-flutter-autoroll mentioned this pull request Dec 10, 2024

Roll Flutter from ab5b20c16d56 to 953208752eb5 (56 revisions) flutter/packages#8261

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 10, 2024

0737dbf iOS Selection Handle Improvements (flutter/flutter#157815)

ab7c7f4

engine-flutter-autoroll mentioned this pull request Dec 10, 2024

Manual roll Flutter from ab5b20c16d56 to 953208752eb5 (56 revisions) flutter/packages#8262

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 10, 2024

0737dbf iOS Selection Handle Improvements (flutter/flutter#157815)

ca8dea5

engine-flutter-autoroll mentioned this pull request Dec 11, 2024

Roll Flutter from 456366232af9 to 4397f43d37ab (59 revisions) flutter/packages#8271

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 11, 2024

0737dbf iOS Selection Handle Improvements (flutter/flutter#157815)

b3ec8a8

engine-flutter-autoroll mentioned this pull request Dec 11, 2024

Roll Flutter from 456366232af9 to 4397f43d37ab (59 revisions) flutter/packages#8272

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 11, 2024

0737dbf iOS Selection Handle Improvements (flutter/flutter#157815)

c4ff778

engine-flutter-autoroll mentioned this pull request Dec 11, 2024

Manual roll Flutter from 456366232af9 to 34c0f5221ef3 (71 revisions) flutter/packages#8276

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 11, 2024

0737dbf iOS Selection Handle Improvements (flutter/flutter#157815)

1d913e1

reidbaker mentioned this pull request Dec 13, 2024

Cutout rotation integration test #160255

Closed

11 tasks

Piinks mentioned this pull request Dec 13, 2024

Flutter Framework Team: Notable Commits #121415

Closed

justinmc mentioned this pull request Jan 27, 2025

Emoji size is off on iOS #28894

Open

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2025

0737dbf iOS Selection Handle Improvements (flutter/flutter#157815)

51f0275

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025

0737dbf iOS Selection Handle Improvements (flutter/flutter#157815)

fb636de

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025

0737dbf iOS Selection Handle Improvements (flutter/flutter#157815)

a935193

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025

0737dbf iOS Selection Handle Improvements (flutter/flutter#157815)

9950511

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2025

0737dbf iOS Selection Handle Improvements (flutter/flutter#157815)

12d1561

Renzo-Olivares mentioned this pull request Mar 18, 2025

(iOS 17) Text selection magnifier is out of date #133001

Closed

2 tasks

amantoux mentioned this pull request Apr 26, 2025

iOS selection improvements fleather-editor/fleather#486

Merged

	case TargetPlatform.macOS:
	case TargetPlatform.linux:
	case TargetPlatform.windows:
	if (cause == SelectionChangedCause.drag) {
	_editableText.hideToolbar();
	}
	}

	}) : super(
	start: baseOffset < extentOffset ? baseOffset : extentOffset,
	end: baseOffset < extentOffset ? extentOffset : baseOffset,
	);

		? TextSelectionHandleType.right
		: TextSelectionHandleType.left,

iOS Selection Handle Improvements #157815

iOS Selection Handle Improvements #157815

Uh oh!

Conversation

Renzo-Olivares commented Oct 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

MaherSafadii commented Oct 30, 2024

Uh oh!

justinmc commented Nov 1, 2024

Uh oh!

Renzo-Olivares Nov 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Renzo-Olivares commented Nov 7, 2024

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Piinks commented Nov 13, 2024

Uh oh!

flutter-dashboard bot commented Dec 3, 2024

Uh oh!

Renzo-Olivares commented Dec 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flutter-dashboard bot commented Dec 5, 2024

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

flutter-dashboard bot commented Dec 6, 2024

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Renzo-Olivares Dec 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Renzo-Olivares commented Oct 29, 2024 •

edited

Loading

Renzo-Olivares Nov 7, 2024 •

edited

Loading

Renzo-Olivares commented Dec 5, 2024 •

edited

Loading

Renzo-Olivares Dec 6, 2024 •

edited

Loading