fix: Inconsistency of SelectionArea when scrolling #128765

paurakhsharma · 2023-06-13T04:44:46Z

This PR fixes inconsistency of SelectionArea when scrolling by removing the _clearSelection on longPressUpdate gesture. This in turn makes the selection UX much better.

List which issues are fixed by this PR. You must list at least one issue.
Fixes: #120892

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

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.

flutter-dashboard · 2023-06-13T04:44:49Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

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

Renzo-Olivares · 2023-06-13T21:52:44Z

Hi @paurakhsharma, thank you for your contribution! Can you explain how this pull request fixes the issue listed, i'm not sure I understand. A test verifying the fix will also be needed to go forward with this change.

paurakhsharma · 2023-06-19T04:41:39Z

As mentioned by @gnprice in #120892 (comment) I have updated the test to make sure touchLongPressCancel doesn't trigger any event. I am not sure if this is a valid way to trigger toucnLongPressCancel. Please let me know if there is a better way to test this.

Renzo-Olivares · 2023-06-20T17:49:37Z

packages/flutter/test/widgets/selectable_region_test.dart

    });

+  testWidgets(
+    "touch long press cancel doesn't send ClearSelectionEvent",


nit: doesn't -> does not.

Renzo-Olivares · 2023-06-20T17:54:48Z

packages/flutter/test/widgets/selectable_region_test.dart

+      addTearDown(gesture.removePointer);
+
+      await tester.pump(const Duration(milliseconds: 500));
+      await gesture.cancel();


This seems like a reasonable way of testing the cancel. I think we should also test the specific situation of the cancel in a scrollable as a regression test. Something like long press to select a word in a scrollable. Then try scrolling. The selection should be retained after the scroll.

Also add a regression test comment at the top of the test. Something like
// Regression test for https://github.com/flutter/flutter/issues/128765

Renzo-Olivares · 2023-06-20T17:55:56Z

@chunhtai if you had any thoughts regarding this change.

Renzo-Olivares

Thanks for all the changes! I had a few nits but overall the new test looks good.

Renzo-Olivares · 2023-06-21T17:04:23Z

packages/flutter/test/widgets/selectable_region_test.dart

+    'touch long press cancel does not send ClearSelectionEvent',
+    (WidgetTester tester) async {
+      final UniqueKey spy = UniqueKey();
+      await tester.pumpWidget(MaterialApp(


nit formatting: move MaterialApp to its own line.

Renzo-Olivares · 2023-06-21T17:05:03Z

packages/flutter/test/widgets/selectable_region_test.dart

+          selectionControls: materialTextSelectionControls,
+          child: SelectionSpy(key: spy),
+        ),
+      ));


nit formatting: move trailing parentheses to their own line and separate with a comma.

Renzo-Olivares · 2023-06-21T17:08:48Z

packages/flutter/test/widgets/selectable_region_test.dart

+            await tester.startGesture(const Offset(200.0, 200.0));
+        addTearDown(selectGesture.removePointer);
+        await tester.pump(const Duration(milliseconds: 500));
+


Maybe here instead of moving immediately after the long press we should just do selectGesture.up() and check for the SelectWordSelectionEvent. Then clear the events list with renderSelectionSpy.events.clear(). Then try a scroll gesture to see that ClearSelectionEvent is not sent. This would simulate taking our finger off the screen and then initiating a scroll.

Thank you @Renzo-Olivares I have updated the PR accordingly.

Renzo-Olivares

LGTM with one small nit. Thanks for the contribution!

Renzo-Olivares · 2023-06-22T17:07:50Z

packages/flutter/test/widgets/selectable_region_test.dart

+              child: SelectionSpy(key: spy),
+            ),
+          ),
+        );


nit formatting: line this up with await.

Thank you, Done!

chunhtai

LGTM

…lutter#128765)

Roll Flutter from 042c036 to 96a2c05 (60 revisions) flutter/flutter@042c036...96a2c05 2023-06-26 49699333+dependabot[bot]@users.noreply.github.com Bump ossf/scorecard-action from 2.1.3 to 2.2.0 (flutter/flutter#129453) 2023-06-26 [email protected] Roll Flutter Engine from 63582320d20e to 4032a9bc964e (2 revisions) (flutter/flutter#129569) 2023-06-26 [email protected] Updated TextMagnifierExampleApp to M3 (flutter/flutter#129381) 2023-06-26 [email protected] Roll Flutter Engine from debee7cece49 to 63582320d20e (3 revisions) (flutter/flutter#129563) 2023-06-26 [email protected] Fix `AnimatedList` & `AnimatedGrid` doesn't apply `MediaQuery` padding (flutter/flutter#129556) 2023-06-26 [email protected] Process only specific labels. (flutter/flutter#129475) 2023-06-26 [email protected] Roll Packages from d041934 to 6b70804 (7 revisions) (flutter/flutter#129559) 2023-06-26 [email protected] Make bug templates more consistent (flutter/flutter#129460) 2023-06-26 [email protected] Roll Flutter Engine from be46101e952d to debee7cece49 (1 revision) (flutter/flutter#129544) 2023-06-26 [email protected] Roll Flutter Engine from 88ff46e1efc1 to be46101e952d (2 revisions) (flutter/flutter#129540) 2023-06-26 [email protected] Roll Flutter Engine from 317673b3278a to 88ff46e1efc1 (1 revision) (flutter/flutter#129534) 2023-06-26 [email protected] Roll Flutter Engine from 593e1d9a9cf5 to 317673b3278a (3 revisions) (flutter/flutter#129531) 2023-06-25 [email protected] Roll Flutter Engine from 5178e8ab7764 to 593e1d9a9cf5 (1 revision) (flutter/flutter#129514) 2023-06-25 [email protected] Roll Flutter Engine from 100f6fc854b4 to 5178e8ab7764 (1 revision) (flutter/flutter#129508) 2023-06-25 [email protected] Roll Flutter Engine from a9f446e25f0c to 100f6fc854b4 (1 revision) (flutter/flutter#129498) 2023-06-25 [email protected] Roll Flutter Engine from 5b1b98305768 to a9f446e25f0c (1 revision) (flutter/flutter#129495) 2023-06-24 [email protected] Roll Flutter Engine from 72c902ce3b81 to 5b1b98305768 (2 revisions) (flutter/flutter#129493) 2023-06-24 [email protected] Roll Flutter Engine from f2976add9414 to 72c902ce3b81 (1 revision) (flutter/flutter#129489) 2023-06-24 [email protected] Roll Flutter Engine from 76a5e971dc09 to f2976add9414 (1 revision) (flutter/flutter#129487) 2023-06-24 [email protected] Roll Flutter Engine from 79fd6beea087 to 76a5e971dc09 (1 revision) (flutter/flutter#129483) 2023-06-24 [email protected] Roll Flutter Engine from 30ac9107f2f9 to 79fd6beea087 (1 revision) (flutter/flutter#129479) 2023-06-24 [email protected] Roll Flutter Engine from ee65380bf3fd to 30ac9107f2f9 (1 revision) (flutter/flutter#129476) 2023-06-24 [email protected] Roll Flutter Engine from 240a86f40781 to ee65380bf3fd (2 revisions) (flutter/flutter#129474) 2023-06-24 [email protected] Fix issues with no response bot. (flutter/flutter#129470) 2023-06-24 [email protected] Roll Flutter Engine from 5e6dcf959ea0 to 240a86f40781 (1 revision) (flutter/flutter#129473) 2023-06-24 [email protected] Roll Flutter Engine from 550bfedc8760 to 5e6dcf959ea0 (1 revision) (flutter/flutter#129468) 2023-06-24 [email protected] Roll Flutter Engine from c1abd1f17ab1 to 550bfedc8760 (3 revisions) (flutter/flutter#129466) 2023-06-24 [email protected] [devicelab] Add platform view scroll benchmarks for Impeller on Android (flutter/flutter#129455) 2023-06-23 [email protected] Roll Flutter Engine from eca910dd5e3f to c1abd1f17ab1 (3 revisions) (flutter/flutter#129456) 2023-06-23 [email protected] Roll Flutter Engine from f8a39cb4dc56 to eca910dd5e3f (3 revisions) (flutter/flutter#129451) 2023-06-23 [email protected] Roll Flutter Engine from 74ef61859bdc to f8a39cb4dc56 (2 revisions) (flutter/flutter#129445) 2023-06-23 [email protected] Add CallbackShortcuts widget of the week video (flutter/flutter#129296) 2023-06-23 [email protected] Roll Flutter Engine from 060cd9c17df3 to 74ef61859bdc (3 revisions) (flutter/flutter#129444) 2023-06-23 [email protected] Roll Flutter Engine from cd30a48ef889 to 060cd9c17df3 (3 revisions) (flutter/flutter#129440) 2023-06-23 [email protected] Roll Flutter Engine from 27f3498286ff to cd30a48ef889 (4 revisions) (flutter/flutter#129437) 2023-06-23 [email protected] Add a devicelab test to recipes cq. (flutter/flutter#129411) 2023-06-23 [email protected] fix: Inconsistency of SelectionArea when scrolling (flutter/flutter#128765) 2023-06-23 [email protected] Run misc test shard when examples/api/** change (flutter/flutter#129433) 2023-06-23 49699333+dependabot[bot]@users.noreply.github.com Bump ubuntu from `b795f8e` to `db8bf6f` in /dev/ci/docker_linux (flutter/flutter#128740) 2023-06-23 [email protected] Add r: timeout label to bugs/pr closed by no response bot. (flutter/flutter#129408) 2023-06-23 [email protected] Remove .github/move.yml. (flutter/flutter#129409) 2023-06-23 [email protected] Roll Packages from 95bc1c6 to d041934 (6 revisions) (flutter/flutter#129429) 2023-06-23 [email protected] Roll Flutter Engine from aca26b2afc27 to 27f3498286ff (16 revisions) (flutter/flutter#129421) 2023-06-23 [email protected] Add comments to .ci.yaml that were removed from recipes. (flutter/flutter#129321) 2023-06-23 [email protected] Update no response to use core action. (flutter/flutter#129405) 2023-06-23 [email protected] Revert "[web] Migrate framework to fully use package:web" (flutter/flutter#129400) ...

…lutter#128765)

fix: Inconsistency of SelectionArea when scrolling

f66f0b9

github-actions bot added the framework flutter/packages/flutter repository. See also f: labels. label Jun 13, 2023

Merge branch 'master' into fix/inconsistency-of-selectionarea

f5ed9ab

gnprice mentioned this pull request Jun 18, 2023

Inconsistency of SelectionArea when scrolling #120892

Closed

test: touch long press cancel doesn't sent any event

897f831

paurakhsharma force-pushed the fix/inconsistency-of-selectionarea branch from e596f25 to 897f831 Compare June 19, 2023 10:28

paurakhsharma added 2 commits June 19, 2023 16:15

Merge branch 'master' into fix/inconsistency-of-selectionarea

9c821f0

Merge branch 'master' into fix/inconsistency-of-selectionarea

54372ec

Renzo-Olivares requested changes Jun 20, 2023

View reviewed changes

paurakhsharma added 2 commits June 21, 2023 13:00

add regression test for scrolling while there is active selection

3699367

Merge branch 'master' into fix/inconsistency-of-selectionarea

b2ffead

Renzo-Olivares reviewed Jun 21, 2023

View reviewed changes

fix test to make sure SelectWordSelectionEvent is sent before scrolling

a04bf9d

paurakhsharma requested a review from Renzo-Olivares June 22, 2023 04:21

Merge branch 'master' into fix/inconsistency-of-selectionarea

0691fb7

Renzo-Olivares approved these changes Jun 22, 2023

View reviewed changes

paurakhsharma added 2 commits June 23, 2023 13:14

fix code formating alignment

b3053dc

Merge branch 'master' into fix/inconsistency-of-selectionarea

4860a22

chunhtai approved these changes Jun 23, 2023

View reviewed changes

Renzo-Olivares added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 23, 2023

auto-submit bot merged commit a9164a4 into flutter:master Jun 23, 2023

engine-flutter-autoroll mentioned this pull request Jun 23, 2023

Roll Flutter from 042c0366c9a9 to a9164a4d76c7 (24 revisions) flutter/packages#4287

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 23, 2023

a9164a4 fix: Inconsistency of SelectionArea when scrolling (flutter/f…

1675655

…lutter#128765)

engine-flutter-autoroll mentioned this pull request Jun 23, 2023

Roll Flutter from 042c0366c9a9 to 935652c34fd8 (26 revisions) flutter/packages#4288

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 23, 2023

a9164a4 fix: Inconsistency of SelectionArea when scrolling (flutter/f…

812fafe

…lutter#128765)

engine-flutter-autoroll mentioned this pull request Jun 23, 2023

Manual roll Flutter from 042c0366c9a9 to 80f8bf1191ac (30 revisions) flutter/packages#4290

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 23, 2023

a9164a4 fix: Inconsistency of SelectionArea when scrolling (flutter/f…

e355a0f

…lutter#128765)

engine-flutter-autoroll mentioned this pull request Jun 24, 2023

Roll Flutter from 042c0366c9a9 to fd9d13c28a0d (41 revisions) flutter/packages#4293

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 24, 2023

a9164a4 fix: Inconsistency of SelectionArea when scrolling (flutter/f…

6079f00

…lutter#128765)

engine-flutter-autoroll mentioned this pull request Jun 24, 2023

Roll Flutter from 042c0366c9a9 to fd9d13c28a0d (41 revisions) flutter/packages#4294

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 24, 2023

a9164a4 fix: Inconsistency of SelectionArea when scrolling (flutter/f…

df5f6cd

…lutter#128765)

engine-flutter-autoroll mentioned this pull request Jun 24, 2023

Roll Flutter from 042c0366c9a9 to 6855b5277924 (42 revisions) flutter/packages#4295

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 24, 2023

a9164a4 fix: Inconsistency of SelectionArea when scrolling (flutter/f…

63799b3

…lutter#128765)

engine-flutter-autoroll mentioned this pull request Jun 25, 2023

Roll Flutter from 042c0366c9a9 to 42d2c2d5926b (47 revisions) flutter/packages#4297

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 25, 2023

a9164a4 fix: Inconsistency of SelectionArea when scrolling (flutter/f…

7c7ee47

…lutter#128765)

engine-flutter-autoroll mentioned this pull request Jun 25, 2023

Roll Flutter from 042c0366c9a9 to b2e950131786 (48 revisions) flutter/packages#4298

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 25, 2023

a9164a4 fix: Inconsistency of SelectionArea when scrolling (flutter/f…

a09c270

…lutter#128765)

engine-flutter-autoroll mentioned this pull request Jun 25, 2023

Roll Flutter from 042c0366c9a9 to b2e950131786 (48 revisions) flutter/packages#4299

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 25, 2023

a9164a4 fix: Inconsistency of SelectionArea when scrolling (flutter/f…

15481f3

…lutter#128765)

engine-flutter-autoroll mentioned this pull request Jun 26, 2023

Roll Flutter from 042c0366c9a9 to a20db068dd9f (53 revisions) flutter/packages#4305

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 26, 2023

a9164a4 fix: Inconsistency of SelectionArea when scrolling (flutter/f…

6afbef0

…lutter#128765)

engine-flutter-autoroll mentioned this pull request Jun 26, 2023

Roll Flutter from 042c0366c9a9 to 4871391dda9c (54 revisions) flutter/packages#4308

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 26, 2023

a9164a4 fix: Inconsistency of SelectionArea when scrolling (flutter/f…

0da48d6

…lutter#128765)

engine-flutter-autoroll mentioned this pull request Jun 26, 2023

Roll Flutter from 042c0366c9a9 to c8e42b47f5ea (56 revisions) flutter/packages#4309

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 26, 2023

a9164a4 fix: Inconsistency of SelectionArea when scrolling (flutter/f…

3a464e1

…lutter#128765)

engine-flutter-autoroll mentioned this pull request Jun 26, 2023

Roll Flutter from 042c0366c9a9 to cef9a8c54b76 (57 revisions) flutter/packages#4310

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 26, 2023

a9164a4 fix: Inconsistency of SelectionArea when scrolling (flutter/f…

b13085e

…lutter#128765)

engine-flutter-autoroll mentioned this pull request Jun 26, 2023

Roll Flutter from 042c0366c9a9 to 96a2c0535810 (60 revisions) flutter/packages#4313

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 26, 2023

a9164a4 fix: Inconsistency of SelectionArea when scrolling (flutter/f…

a7f6ecc

…lutter#128765)

HansMuller mentioned this pull request Jun 30, 2023

Flutter Framework Team: Notable Commits #121415

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 16, 2023

a9164a4 fix: Inconsistency of SelectionArea when scrolling (flutter/f…

e9fabd8

…lutter#128765)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023

a9164a4 fix: Inconsistency of SelectionArea when scrolling (flutter/f…

0064be3

…lutter#128765)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023

a9164a4 fix: Inconsistency of SelectionArea when scrolling (flutter/f…

14e1491

…lutter#128765)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023

a9164a4 fix: Inconsistency of SelectionArea when scrolling (flutter/f…

f027570

…lutter#128765)

paurakhsharma deleted the fix/inconsistency-of-selectionarea branch September 8, 2023 10:23

fix: Inconsistency of SelectionArea when scrolling #128765

fix: Inconsistency of SelectionArea when scrolling #128765

Uh oh!

Conversation

paurakhsharma commented Jun 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

flutter-dashboard bot commented Jun 13, 2023

Uh oh!

Renzo-Olivares commented Jun 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

paurakhsharma commented Jun 19, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Renzo-Olivares commented Jun 20, 2023

Uh oh!

Renzo-Olivares 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!

Renzo-Olivares 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!

chunhtai left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

paurakhsharma commented Jun 13, 2023 •

edited

Loading

Renzo-Olivares commented Jun 13, 2023 •

edited

Loading