Fix text selection edge scrolling when inside a horizontal scrollable by Renzo-Olivares · Pull Request #140250 · flutter/flutter

Renzo-Olivares · 2023-12-15T21:54:32Z

Consider AxisDirection when calculating scroll offset used in determining TextSelection during a drag/long press drag. Previously it seems that we were assuming the direction was always vertical

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

Lines 2842 to 2844 in 30cc831

    
           final Offset scrollableOffset = Offset( 
        
             0.0, 
        
             _scrollPosition - _dragStartScrollOffset,

.

SelectableText now considers RenderEditable offset changes and Scrollable offset changes when calculating the TextSelection during a long press drag.

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.

Renzo-Olivares · 2024-01-10T02:32:19Z

packages/flutter/lib/src/material/selectable_text.dart

A way we can avoid having these overrides in selectable_text.dart so we can use the logic already in text_selection.dart is to use editableText.readOnly to differentiate between SelectableText and EditableText at the TextSelectionGestureDetector level. Not sure if we want to do this since maybe a user of TextField/EditableText might set readOnly to true and question why their gestures are behaving differently.

What's the behavior difference exactly?

It's not too much code to be concerning, but either way I can't wait to unify these so we don't have to think about problems like this!

The main difference is that on iOS a long press on static text select word-by-word, and on editable text it moves the collapsed cursor to the position.

On all other platforms (Android) a long press selects word-by-word.

…l scrollable

justinmc

LGTM 👍

Still debating your question in https://github.com/flutter/flutter/pull/140250/files#r1446817469 but I think it's fine either way.

justinmc · 2024-01-24T22:52:01Z

packages/flutter/lib/src/material/selectable_text.dart

What's the behavior difference exactly?

It's not too much code to be concerning, but either way I can't wait to unify these so we don't have to think about problems like this!

justinmc · 2024-01-24T22:55:12Z

packages/flutter/lib/src/material/selectable_text.dart

+        ? Offset(renderEditable.offset.pixels - _dragStartViewportOffset, 0.0)
+        : Offset(0.0, renderEditable.offset.pixels - _dragStartViewportOffset);
+    final double effectiveScrollPosition = _scrollPosition - _dragStartScrollOffset;
+    final bool scrollingOnVerticalAxis = _scrollDirection == AxisDirection.up || _scrollDirection == AxisDirection.down;


Nit: would this be nicer as a switch or am I just overly excited to use switch everywhere?

I think a switch would work but might be a little verbose for this specific case. I think the logical comparison is clear and concise enough for this case.

final bool scrollingOnVerticalAxis = switch (_scrollDirection) { AxisDirection.up => true, AxisDirection.down => true, AxisDirection.right => false, AxisDirection.left => false, };

justinmc · 2024-01-24T23:01:33Z

packages/flutter/test/widgets/selectable_text_test.dart

+
+    final TestGesture gesture =
+        await tester.startGesture(selectableTextStart + const Offset(200.0, 0.0));
+    await tester.pump(const Duration(milliseconds: 500));


Nit: Maybe use kLongPressTimeout.

…rollable (flutter/flutter#140250)

Roll Flutter from 19b06f4 to a8efa77 (38 revisions) flutter/flutter@19b06f4...a8efa77 2024-01-25 [email protected] Roll Flutter Engine from e2014f007f61 to 7c4ed15cb271 (1 revision) (flutter/flutter#142221) 2024-01-25 [email protected] Roll Flutter Engine from 5fa2e2920274 to e2014f007f61 (1 revision) (flutter/flutter#142213) 2024-01-25 [email protected] provide command to `FakeCommand::onRun` (flutter/flutter#142206) 2024-01-25 [email protected] Roll Flutter Engine from c346fd3d9ca1 to 5fa2e2920274 (1 revision) (flutter/flutter#142212) 2024-01-25 [email protected] Roll Flutter Engine from d4d8f668159b to c346fd3d9ca1 (1 revision) (flutter/flutter#142209) 2024-01-25 [email protected] Roll Flutter Engine from 499ed00bbda2 to d4d8f668159b (2 revisions) (flutter/flutter#142205) 2024-01-25 [email protected] Roll Flutter Engine from 4880592ca5ba to 499ed00bbda2 (6 revisions) (flutter/flutter#142202) 2024-01-25 [email protected] [ci] Adds test for web hot restart with const App. (flutter/flutter#141824) 2024-01-25 [email protected] Migrate android_view to linux_android_emu platform. (flutter/flutter#142184) 2024-01-25 [email protected] Refactor `external_ui` without making any name changes (I think) (flutter/flutter#142192) 2024-01-25 [email protected] Fix text selection edge scrolling when inside a horizontal scrollable (flutter/flutter#140250) 2024-01-25 [email protected] Roll Flutter Engine from d7bf5ec1dcdd to 4880592ca5ba (2 revisions) (flutter/flutter#142186) 2024-01-24 [email protected] Roll Flutter Engine from 6a7f963dc751 to d7bf5ec1dcdd (2 revisions) (flutter/flutter#142185) 2024-01-24 [email protected] Reland "Remove hack from PageView." (flutter/flutter#142172) 2024-01-24 [email protected] Upgrade leak_tracker. (flutter/flutter#142162) 2024-01-24 [email protected] Migrate android views to devicelab. (flutter/flutter#142081) 2024-01-24 [email protected] Roll Flutter Engine from ed498f111d53 to 6a7f963dc751 (4 revisions) (flutter/flutter#142176) 2024-01-24 [email protected] Support wireless debugging for iOS 12 or earlier (flutter/flutter#141439) 2024-01-24 [email protected] Roll Flutter Engine from 4bc368ee5f74 to ed498f111d53 (5 revisions) (flutter/flutter#142167) 2024-01-24 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Refactor `external_ui` â�� `external_textures`" (flutter/flutter#142173) 2024-01-24 [email protected] Refactor `external_ui` â�� `external_textures` (flutter/flutter#142062) 2024-01-24 [email protected] Roll Flutter Engine from 0b9fce355df9 to 4bc368ee5f74 (3 revisions) (flutter/flutter#142157) 2024-01-24 [email protected] Update navigationBar label's maxScaleFactor to meet GAR requirement (flutter/flutter#141998) 2024-01-24 [email protected] Roll Flutter Engine from b65556fa543e to 0b9fce355df9 (1 revision) (flutter/flutter#142147) 2024-01-24 [email protected] Marks Mac_arm64 build_tests_2_4 to be unflaky (flutter/flutter#142115) 2024-01-24 [email protected] Marks Mac_x64 build_tests_2_4 to be unflaky (flutter/flutter#142111) 2024-01-24 [email protected] Marks Mac_x64 build_tests_1_4 to be unflaky (flutter/flutter#142110) 2024-01-24 [email protected] Marks Mac_x64 build_tests_3_4 to be unflaky (flutter/flutter#142112) 2024-01-24 [email protected] Revise tooltip theme docs, including more cross-references (flutter/flutter#137316) 2024-01-24 [email protected] Run some tests explicitly in both arm and x64. (flutter/flutter#141910) 2024-01-24 [email protected] Roll Flutter Engine from ba3a70ce7722 to b65556fa543e (3 revisions) (flutter/flutter#142140) 2024-01-24 [email protected] Fixes #138773, port autocomplete to OverlayPortal (flutter/flutter#140285) 2024-01-24 [email protected] Revert "[web] - Fix broken `TextField` in semantics mode when it's a sibling of `Navigator`" (flutter/flutter#142129) 2024-01-24 [email protected] Marks Mac_arm64 build_tests_1_4 to be unflaky (flutter/flutter#142114) 2024-01-24 [email protected] Roll Packages from 841fe90 to 8fbdf65 (2 revisions) (flutter/flutter#142139) 2024-01-24 [email protected] Marks Mac_arm64 build_tests_3_4 to be unflaky (flutter/flutter#142116) 2024-01-24 [email protected] Marks Mac_x64 build_tests_4_4 to be unflaky (flutter/flutter#142113) 2024-01-24 [email protected] Marks Mac_arm64 build_tests_4_4 to be unflaky (flutter/flutter#142117) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose ...

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: material design flutter/packages/flutter/material repository. labels Dec 15, 2023

Renzo-Olivares force-pushed the selection-fix branch from 39bc576 to 7c9d1b7 Compare December 15, 2023 22:25

Renzo-Olivares force-pushed the selection-fix branch 2 times, most recently from 127e1ba to 81e176f Compare January 5, 2024 22:23

Renzo-Olivares closed this Jan 9, 2024

Renzo-Olivares reopened this Jan 10, 2024

Renzo-Olivares force-pushed the selection-fix branch from 81e176f to e8b6bec Compare January 10, 2024 02:27

Renzo-Olivares commented Jan 10, 2024

View reviewed changes

Renzo-Olivares added 7 commits January 24, 2024 13:07

Fix text selection edge scrolling when EditableText is in a horizonta…

2cfb532

…l scrollable

Add test

7e15aa5

update test

715d32e

Add test

bbb1967

updates

22a5e95

update tests

ac5ac77

updates

e7f6fd8

Renzo-Olivares force-pushed the selection-fix branch from e8b6bec to e7f6fd8 Compare January 24, 2024 21:07

Renzo-Olivares added 4 commits January 24, 2024 13:57

dont run test on Android

f3b18a0

add TODO

ccdd1c6

add desktop test

4a3e84c

updates

d1d754c

Renzo-Olivares requested a review from justinmc January 24, 2024 22:44

justinmc approved these changes Jan 24, 2024

View reviewed changes

Address comments

4b4afe1

Renzo-Olivares added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 25, 2024

auto-submit bot merged commit eba38c4 into flutter:master Jan 25, 2024

engine-flutter-autoroll mentioned this pull request Jan 25, 2024

Roll Flutter from 19b06f4ee9a0 to a8efa771d6a3 (38 revisions) flutter/packages#5969

Merged

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

eba38c4 Fix text selection edge scrolling when inside a horizontal sc…

f97f275

…rollable (flutter/flutter#140250)

HansMuller mentioned this pull request Jan 26, 2024

Flutter Framework Team: Notable Commits #121415

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix text selection edge scrolling when inside a horizontal scrollable#140250

Fix text selection edge scrolling when inside a horizontal scrollable#140250
auto-submit[bot] merged 12 commits intoflutter:masterfrom
Renzo-Olivares:selection-fix

Renzo-Olivares commented Dec 15, 2023 •

edited

Loading

Uh oh!

Renzo-Olivares Jan 10, 2024

Uh oh!

justinmc Jan 24, 2024

Uh oh!

Renzo-Olivares Jan 24, 2024

Uh oh!

justinmc left a comment

Uh oh!

justinmc Jan 24, 2024

Uh oh!

justinmc Jan 24, 2024

Uh oh!

Renzo-Olivares Jan 24, 2024

Uh oh!

justinmc Jan 24, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	final Offset scrollableOffset = Offset(
	0.0,
	_scrollPosition - _dragStartScrollOffset,

Conversation

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

Uh oh!

Pre-launch Checklist

Uh oh!

Renzo-Olivares Jan 10, 2024

Choose a reason for hiding this comment

Uh oh!

justinmc Jan 24, 2024

Choose a reason for hiding this comment

Uh oh!

Renzo-Olivares Jan 24, 2024

Choose a reason for hiding this comment

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

justinmc Jan 24, 2024

Choose a reason for hiding this comment

Uh oh!

justinmc Jan 24, 2024

Choose a reason for hiding this comment

Uh oh!

Renzo-Olivares Jan 24, 2024

Choose a reason for hiding this comment

Uh oh!

justinmc Jan 24, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Renzo-Olivares commented Dec 15, 2023 •

edited

Loading