Fix selection toolbar not showing on drag end #121110
Conversation
058d8d1
to
5c017ea
Compare
| @@ -533,6 +533,7 @@ class SelectableRegionState extends State<SelectableRegion> with TextSelectionDe | |||
| } else { | |||
| _selectionOverlay!.hideMagnifier(); | |||
| _selectionOverlay!.showToolbar( | |||
| context: context, | |||
There was a problem hiding this comment.
Good catch, context should be provided when contextMenuBuilder is provided. Thanks!
|
|
||
| await gesture.up(); | ||
| await tester.pump(const Duration(milliseconds: 500)); | ||
| await tester.pump(const Duration(milliseconds: 500)); |
There was a problem hiding this comment.
Can you replace these pumps with pumpAndSettle or, if possible, a pump with no Duration? Ideally we shouldn't have any arbitrary timing things like this unless it's totally necessary.
There was a problem hiding this comment.
I was able to remove most of the pumps with duration, except for one location. I believe this one is necessary though, because it is also used in other tests.
For example, here:
https://github.com/flutter/flutter/blob/686266e4184a107b5e52ddc871cb5400d8caf9df/packages/flutter/test/material/selection_area_test.dart#L61
There was a problem hiding this comment.
Sounds good, it's fine like this then 👍
There was a problem hiding this comment.
I think this particular use of pump is so the down event is held and detected as a long press.
5c017ea
to
686266e
Compare
686266e
to
ae81915
Compare
|
The test failure is something unrelated that I've seen on another PR as well. Can you push a merge commit? Or a rerun may do it. |
ae81915
to
63b67ba
Compare
|
@Renzo-Olivares @LongCatIsLooong could you take a look? |
|
Would be good cherry pick candidate IMO |
|
A new stable release will be cut soon with this fix in it, so I think that will work out and we won't need to cherry pick. Well CC @itsjustkevin in case. I guess it won't be released for awhile. |
|
Hello, which flutter version fixes this function? |
|
Looks like this was tagged in 3.9.0-18.0.pre. |
Fixes #119314
The regression was caused initially by this commit: f014c1e
Before
Before, ending the drag didn't show the toolbar, making it impossible to copy the text after adjusting the selection handles.
Screen.Recording.2023-02-20.at.23.32.51.mov
After
Now ending the drag will show the toolbar again:
Screen.Recording.2023-02-20.at.23.29.18.mov
Pre-launch Checklist
///).