Add selection feedback for both selection area and text field #115373

chunhtai · 2022-11-15T17:39:23Z

fixes #104551
fixes #104640
fixes #104639

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.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

justinmc

I'm really excited to have this feature!

I checked out this branch and tried it out on my Pixel 6 on Android 13 and it works great, but if I'm really picky I can feel a few slight differences between this and native. I went overboard on QA'ing this, so feel free to just open issues for most of the points below. As it is now, it's a great feature and the differences are hardly noticeable.

If I drag horizontally across a few words, it feels different to me. I think what's happening is that Flutter is vibrating once for every character and native is doing once for every 2 characters or something? Or maybe I'm just feeling a slight difference in the strength or duration of each vibration...

Also, if I put my finger on the collapsed handle and move horizontally back and forth very slightly, so that the loupe moves but the selection does not change, then Flutter vibrates but native does not. Maybe this is a symptom of set selectionEndpoints or showHandles being called more than it should and it should be handled in a separate PR?

If I connect a hardware mouse to my phone, tapping to place the cursor causes the phone to vibrate on this branch, but it does not on native. (I tried everything else related to hardware keyboard/mouse and it worked correctly. Dragging selection with a mouse and changing the selection with the arrow keys etc. correctly did not vibrate, per my observations on native.) Actually, I think this is not related to the hardware mouse. Tapping to place the cursor with my finger also vibrates, but on native it does not.

In two of the issues (#104640, #104639), it's mentioned that Android has a system setting to toggle this vibration. It seems like we also need an engine PR to tie into this information? Should we create a new separate issue for that?

justinmc · 2022-11-15T23:02:34Z

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

@@ -1127,6 +1155,9 @@ class SelectionOverlay {
    if (!listEquals(_selectionEndpoints, value)) {
      _markNeedsBuild();
    }
+    if (_isDraggingEndHandle || _isDraggingStartHandle) {
+      HapticFeedback.selectionClick();


Is there any platform check that should happen here? Or if not I guess that 1. iOS and Android both vibrate in the same case here and 2. calling this on platforms that don't vibrate, like desktop, is just a noop.

Update: I tried this on native iOS and there is no vibration for changing the text selection at all, as far as I can tell. So this should only happen on Android?

yeah that sounds good, for iOS it looks like it only vibrate when you first long press to place a selection.

justinmc · 2022-11-15T23:30:29Z

packages/flutter/test/widgets/scrollable_selection_test.dart

@@ -493,6 +494,113 @@ void main() {
    await gesture.up();
  });

+  testWidgets('select to scroll by dragging start selection handle stop scroll when release', (WidgetTester tester) async {


"stop" => "stops", "release" => "released"

And same for the next test's title as well.

chunhtai · 2022-11-15T17:39:57Z

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

@@ -537,6 +537,7 @@ class SelectableRegionState extends State<SelectableRegion> with TextSelectionDe
       },
     );
   }
+  _stopSelectionStartEdgeUpdate();


a bug fix, also added tests to verify the fix

chunhtai · 2022-11-15T17:40:57Z

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

@@ -1127,6 +1155,9 @@ class SelectionOverlay {
    if (!listEquals(_selectionEndpoints, value)) {
      _markNeedsBuild();
    }
+    if (_isDraggingEndHandle || _isDraggingStartHandle) {


If user is dragging handle, and the selection handle has actually moved.

chunhtai · 2022-11-16T17:21:28Z

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

@@ -1127,6 +1155,9 @@ class SelectionOverlay {
    if (!listEquals(_selectionEndpoints, value)) {
      _markNeedsBuild();
    }
+    if (_isDraggingEndHandle || _isDraggingStartHandle) {
+      HapticFeedback.selectionClick();


yeah that sounds good, for iOS it looks like it only vibrate when you first long press to place a selection.

chunhtai · 2022-11-16T21:28:25Z

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

@@ -52,6 +52,19 @@ class TextSelectionPoint {
  /// Direction of the text at this edge of the selection.
  final TextDirection? direction;

+  @override
+  bool operator ==(Object other) {


This is a bug I found that the selectionEndPoint setter does the list comparison. which will always returns false because it does identity compare.

Great catch! It looks like you're right.

chunhtai · 2022-11-16T21:30:14Z

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

+    // This method may be called due to windows metrics changes. In that case,
+    // non of the properties in _selectionOverlay will change, but a rebuild is
+    // still needed.
+    _selectionOverlay.markNeedsBuild();


Another bug i found after I fix the TextSelectionPoint == operator. This method will be called when windows insets change, previous it will always mark needs build because the selectionEndPoint will always think it received a different endpoints.

chunhtai · 2022-11-16T21:32:47Z

after the latest commit.

iOS will only vibrate when long press to place the selection or cursor. Only Android will vibrate when drag to extend selection, but it won't vibrate if user is dragging collapsed selection handle.

Mouse selection will no longer trigger vibration

justinmc

LGTM 👍 But we should open issues for any of the remaining things I mentioned that you don't end up fixing here.

Vibration while dragging a handle doesn't quite feel native. Maybe native is vibrating once every other letter, but this branch vibrates every letter?

screen-20221116-153818.mp4

Very slightly dragging the collapsed handle horizontally, so that the loupe moves a little bit but the selection doesn't change, causes vibration. Native does not. (In the video, I sometimes drag too much and the selection changes, but you can see the loupe move slightly when the selection doesn't change as well.)

screen-20221116-154036.mp4

Clicking to place the cursor with a mouse caused vibration.
Tapping to place the cursor causes vibration, but native does not.

screen-20221116-154137.mp4

Respect Android's system setting that toggles this vibration.
iOS vibration.

justinmc · 2022-11-16T23:22:14Z

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

@@ -52,6 +52,19 @@ class TextSelectionPoint {
  /// Direction of the text at this edge of the selection.
  final TextDirection? direction;

+  @override
+  bool operator ==(Object other) {


Great catch! It looks like you're right.

chunhtai · 2022-11-17T16:27:31Z

Vibration while dragging a handle doesn't quite feel native. Maybe native is vibrating once every other letter, but this branch vibrates every letter?

I can't reproduce with my phone. Although the behavior for native textfields are sometime different to. Some of the native textfield will only vibrate once for long press, but not when dragging the handle. Some of native textfield will vibrate both for long press and dragging handles. For those that vibrate when dragging handles, I see it vibrate for every character, which match this pr's behavior.

Very slightly dragging the collapsed handle horizontally, so that the loupe moves a little bit but the selection doesn't change, causes vibration. Native does not. (In the video, I sometimes drag too much and the selection changes, but you can see the loupe move slightly when the selection doesn't change as well.)

This should be fix now, this is because of the selectionendpoint operator == override was missing.

Tapping to place the cursor causes vibration, but native does not.

I think this was also fixed in the latest commit.

Respect Android's system setting that toggles this vibration.

Yes we should open an issue for this

chunhtai · 2022-11-17T17:24:57Z

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

+    // text metrics and selection doesn't change even if the text has changed.
+    // This rebuild is needed for the toolbar to update based on the latest text
+    // value.
+    _selectionOverlay.markNeedsBuild();


Another place which relies own TextSelectionPoint missing operator ==

chunhtai · 2022-11-17T17:48:00Z

filed an issue #115574

…flutter/flutter#115373)

* 70b7445 Reland Added Badge.count constructor (flutter/flutter#115566) * 31f8631 fa7e1965e [Impeller] Fix glyph atlas uploads and renders (flutter/engine#37691) (flutter/flutter#115556) * a1ea383 Label should always be aligned with text in filled input decoration (flutter/flutter#115540) * c2b2950 Add selection feedback for both selection area and text field (flutter/flutter#115373) * 0344407 Rev package:pub_semver to the latest version (flutter/flutter#115570) * ac06523 Add Material 3 support for `Slider` - Part 2 (flutter/flutter#114624) * b181d07 a2fa4e9 cirrus to luci (flutter/plugins#6711) (flutter/flutter#115573) * e1efd0d b241e69fd [ui] reland add docs to FragmentShader (flutter/engine#37699) (flutter/flutter#115578) * efb0694 Remove unused flutter_attach_test_fuchsia (flutter/flutter#115515) * a5a368c 487ee66f6 [macOS] Merge FlutterRenderer and implementation (flutter/engine#37696) (flutter/flutter#115581) * 4ff7fc6 Fixes a bug where dragging a collapsed handle in TextField does not vibrate (flutter/flutter#115586) * 20be280 da9534ea6 [macOS] Consolidate external texture classes (flutter/engine#37703) (flutter/flutter#115585) * 8a7102e Roll Flutter Engine from da9534ea6534 to d955a72c5604 (3 revisions) (flutter/flutter#115589) * e1903a2 Roll Flutter Engine from d955a72c5604 to 1e1a4ab3c993 (4 revisions) (flutter/flutter#115592) * 78390a0 Roll Flutter Engine from 1e1a4ab3c993 to b65c24ce621a (2 revisions) (flutter/flutter#115598) * 75a0a72 [devicelab] measure entire release folder size, zipped (flutter/flutter#115597) * 59a01b6 Roll Flutter Engine from b65c24ce621a to 49b52db603cc (3 revisions) (flutter/flutter#115606) * ec03f1c Revert "[devicelab] measure entire release folder size, zipped (#115597)" (flutter/flutter#115609) * 710e708 Improve showSnackBar documentation (flutter/flutter#114612) * 915c3de Roll Flutter Engine from 49b52db603cc to 80b25a302b4c (2 revisions) (flutter/flutter#115608) * 450f162 Roll Flutter Engine from 80b25a302b4c to e812122e4060 (2 revisions) (flutter/flutter#115614) * 0b33b85 [devicelab] measure entire release folder size, zipped (flutter/flutter#115612) * 9379c32 Revert "[devicelab] measure entire release folder size, zipped (#115612)" (flutter/flutter#115617) * b746557 f27666d2f [macOS] Merge FlutterBackingStore implementations (flutter/engine#37730) (flutter/flutter#115616) * 5487a7d Roll Flutter Engine from f27666d2f4da to 39f546585b0b (2 revisions) (flutter/flutter#115618) * f261c2f update comments (flutter/flutter#115603) * 9c9f781 04aea3c47 iOS PlatformView only sets a maskView when necessary (flutter/engine#37434) (flutter/flutter#115621) * 6926960 4ca2c1d78 Roll Skia from 55f654bf5cff to 9d56e506b4df (13 revisions) (flutter/engine#37739) (flutter/flutter#115625) * de4c0b1 Use `double.isNaN` instead of `... == double.nan` (which is always false) (flutter/flutter#115424) * a655f85 a62736769 Roll Skia from 9d56e506b4df to d693b4b9fe5e (5 revisions) (flutter/engine#37741) (flutter/flutter#115640) * 18c8727 f092cd826 Roll Fuchsia Mac SDK from SVtX810D2U_ZgBcpx... to tklUfTsSOVKk49tYq... (flutter/engine#37742) (flutter/flutter#115643)

…r#115373) * Add selection feedback for both selection area and text field * Addressing comment * Fixes more test

chunhtai requested a review from justinmc November 15, 2022 17:42

justinmc reviewed Nov 15, 2022

View reviewed changes

chunhtai commented Nov 16, 2022

View reviewed changes

chunhtai requested a review from justinmc November 16, 2022 21:31

justinmc approved these changes Nov 16, 2022

View reviewed changes

chunhtai added 3 commits November 17, 2022 09:23

Add selection feedback for both selection area and text field

2cc2829

Addressing comment

2ed8daa

Fixes more test

8c68d41

chunhtai force-pushed the issues/104551 branch from 5fd20d1 to 8c68d41 Compare November 17, 2022 17:23

chunhtai commented Nov 17, 2022

View reviewed changes

chunhtai mentioned this pull request Nov 17, 2022

[Android] selection haptics feedback should respect android settings #115574

Open

chunhtai added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 17, 2022

auto-submit bot merged commit c2b2950 into flutter:master Nov 17, 2022

engine-flutter-autoroll mentioned this pull request Nov 17, 2022

Roll Flutter from 61e927d22fe6 to 20be280cc19f (114 revisions) flutter/plugins#6714

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Nov 17, 2022

c2b2950 Add selection feedback for both selection area and text field (…

f9810a2

…flutter/flutter#115373)

This was referenced Nov 18, 2022

Roll Flutter from 537f89ca0ef1 to 18c87274cdc9 (31 revisions) flutter/packages#2822

Merged

Roll Flutter from 61e927d22fe6 to 18c87274cdc9 (133 revisions) flutter/plugins#6716

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 18, 2022

c2b2950 Add selection feedback for both selection area and text field (…

27166cf

…flutter/flutter#115373)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Nov 18, 2022

c2b2950 Add selection feedback for both selection area and text field (…

5caa59a

…flutter/flutter#115373)

engine-flutter-autoroll mentioned this pull request Nov 18, 2022

Roll Flutter from 61e927d22fe6 to 87cb150c2062 (134 revisions) flutter/plugins#6717

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Nov 18, 2022

c2b2950 Add selection feedback for both selection area and text field (…

d9b5f0a

…flutter/flutter#115373)

engine-flutter-autoroll mentioned this pull request Nov 18, 2022

Roll Flutter from 61e927d22fe6 to 10c039b2f871 (135 revisions) flutter/plugins#6718

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Nov 18, 2022

c2b2950 Add selection feedback for both selection area and text field (…

eb7104f

…flutter/flutter#115373)

engine-flutter-autoroll mentioned this pull request Nov 18, 2022

Roll Flutter from 61e927d22fe6 to 7fd3bb2aedf7 (136 revisions) flutter/plugins#6719

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Nov 18, 2022

c2b2950 Add selection feedback for both selection area and text field (…

a65d029

…flutter/flutter#115373)

engine-flutter-autoroll mentioned this pull request Nov 18, 2022

Roll Flutter from 61e927d22fe6 to 3a3a0db13e48 (137 revisions) flutter/plugins#6720

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Nov 18, 2022

c2b2950 Add selection feedback for both selection area and text field (…

69dc50d

…flutter/flutter#115373)

engine-flutter-autoroll mentioned this pull request Nov 18, 2022

Roll Flutter from 61e927d22fe6 to ca78327402bf (138 revisions) flutter/plugins#6721

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Nov 18, 2022

c2b2950 Add selection feedback for both selection area and text field (…

4b37ca9

…flutter/flutter#115373)

engine-flutter-autoroll mentioned this pull request Nov 18, 2022

Roll Flutter from 61e927d22fe6 to d2e6dfefa5ca (143 revisions) flutter/plugins#6722

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Nov 18, 2022

c2b2950 Add selection feedback for both selection area and text field (…

f2d6888

…flutter/flutter#115373)

huycozy mentioned this pull request Nov 29, 2022

[SelectionArea] Light haptic feedback should be invoked when selecting a text on mobile devices #116126

Closed

shogohida pushed a commit to shogohida/flutter that referenced this pull request Dec 7, 2022

Add selection feedback for both selection area and text field (flutte…

401233e

…r#115373) * Add selection feedback for both selection area and text field * Addressing comment * Fixes more test

Add selection feedback for both selection area and text field #115373

Add selection feedback for both selection area and text field #115373

Uh oh!

Conversation

chunhtai commented Nov 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

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!

Choose a reason for hiding this comment

Uh oh!

chunhtai commented Nov 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

justinmc left a comment • 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!

chunhtai commented Nov 17, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chunhtai commented Nov 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

chunhtai commented Nov 15, 2022 •

edited

Loading

chunhtai commented Nov 16, 2022 •

edited

Loading

justinmc left a comment •

edited

Loading

chunhtai commented Nov 17, 2022 •

edited

Loading