Hide text selection toolbar when dragging handles on mobile #104274
Conversation
flutter-dashboard
bot
added
a: text input
markusaksli-nc
force-pushed
the
handle-hide-toolbar
branch
3 times, most recently
from
185cb35
to
50a7ecf
Compare
flutter-dashboard
bot
added
the
f: material design
markusaksli-nc
force-pushed
the
handle-hide-toolbar
branch
from
50a7ecf
to
5934192
Compare
There was a problem hiding this comment.
Thanks for fixing this @markusaksli-nc! Some comments below but overall I like the approach.
Also, probably should be a separate PR, but I think Android also hides the handle that is being dragged but Flutter does not?
| @@ -29,6 +29,10 @@ export 'package:flutter/services.dart' show TextSelectionDelegate; | |||
| /// called. | |||
| const Duration _kDragSelectionUpdateThrottle = Duration(milliseconds: 50); | |||
|
|
|||
| /// A duration that determines the delay before showing the text selection | |||
| /// toolbar again after dragging either selection handle on Android. | |||
| const Duration _kAndroidPostDragShowDelay = Duration(milliseconds: 300); | |||
There was a problem hiding this comment.
Can you add a comment like this about where you got this value? "Eyeballed on a physical Android <model e.g. Pixel 6 etc.> device."
That way we know if/when/how we need to update it if someone notices it doesn't match.
There was a problem hiding this comment.
I tried counting frames on a Pixel 5 emulator recording but I'm not sure if the 20fps rate lines up with the exact timing or not (42ms of error?) so it's pretty much eyeballing still.
| @@ -1244,7 +1244,7 @@ void main() { | |||
| expect(endDragUpdateDetails!.globalPosition, newLocation); | |||
|
|
|||
| await gesture2.up(); | |||
| await tester.pump(const Duration(milliseconds: 20)); | |||
| await tester.pump(const Duration(milliseconds: 300)); | |||
There was a problem hiding this comment.
What happens if you don't make this change?
There was a problem hiding this comment.
The 300ms android timer doesn't get dismissed and test fails due to remaining timer. I thought canceling it in dispose should fix this but doesn't work here even though it fixed other test failures. Any ideas?
There was a problem hiding this comment.
Ah maybe you have to manually call selectionOverlay.dispose() in this test? Try that.
markusaksli-nc
force-pushed
the
handle-hide-toolbar
branch
from
5934192
to
dbdd9a5
Compare
|
@justinmc Updated based on the feedback. About hiding the handle it depends on the magnifier position when using Pixel 5 with Android 12. The magnifier hides the dragged handle and the other handle if it overlaps it in Files search bar, Google Chrome search bar, etc where the magnifier is displayed inline with the text. When the magnifier is above the text (where the toolbar usually is) neither handle is hidden in Messages or in the basic EditText, the dragged handle is still hidden in Google Sign-in though, seemingly inconsistent usage. But noticed some other fidelity issues compared to Android EditText. DetailsPixel 5 Android 12 - Android EditText
API31-native.mp4SM-S908B Android 12, One UI 4.1 - Android EditText This has a bunch of wild and wacky additions:
S908B-native.mp4Pixel 5 and SM-S908B (exactly the same) - Flutter TextField
API31-flutter.mp4 |
markusaksli-nc
force-pushed
the
handle-hide-toolbar
branch
3 times, most recently
from
c1e5f94
to
0dd27fd
Compare
| @@ -1244,7 +1244,7 @@ void main() { | |||
| expect(endDragUpdateDetails!.globalPosition, newLocation); | |||
|
|
|||
| await gesture2.up(); | |||
| await tester.pump(const Duration(milliseconds: 20)); | |||
| await tester.pump(const Duration(milliseconds: 300)); | |||
There was a problem hiding this comment.
Ah maybe you have to manually call selectionOverlay.dispose() in this test? Try that.
markusaksli-nc
force-pushed
the
handle-hide-toolbar
branch
from
0dd27fd
to
491fcca
Compare
…lutter#104274)" (flutter#105247) This reverts commit 4ec4c24.
Hides the text selection toolbar on Android and iOS when dragging the text selection handles. Shows it again after the dragging ends. There seems to be a 300ms delay on Android after the toolbar is shown again (couldn't find a spec for this).
The precise timing for this behavior is likely complicated by #17617 still missing from both platforms.
Fixes #62193
Pre-launch Checklist
///).