Skip to content

Conversation

@justinmc
Copy link
Contributor

@justinmc justinmc commented Apr 30, 2019

Closes #31286 #23432

Text selection handles are being moved outside of their enclosing GestureDetector, making them not interactive. I have seen this for the left Material handle at the start of a text field and for the left Cupertino handle at any position.

@justinmc justinmc self-assigned this Apr 30, 2019
@goderbauer goderbauer added f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Apr 30, 2019
@justinmc justinmc force-pushed the immovable-selection-handles branch from 03107ef to 0bf58c0 Compare April 30, 2019 23:18
@justinmc justinmc force-pushed the immovable-selection-handles branch from 7002a32 to aa6c887 Compare May 2, 2019 22:21
@justinmc
Copy link
Contributor Author

justinmc commented May 2, 2019

I've gotten the GestureDetectors to be the correct size and position for all of the different text selection handles that we have.

However, despite the GestureDetectors being the right size and in the right place, there are parts of them that are sometimes still not interactive. I think that's because of the CompositedTransformFollower that we use to reset the coordinate space.

From the docs, "Hit testing on descendants of this widget will only work if the target position is within the box that this widget's parent considers to be hitable." I can't quite get a perfect visual on exactly where the widget's boundary is and if it lines up perfectly with the non-interactive regions I'm seeing, though.

Update: Hit detection works in the places we need it now as of dcf30cf.

@justinmc justinmc force-pushed the immovable-selection-handles branch from aa6c887 to da9aa08 Compare May 2, 2019 22:33
@justinmc justinmc force-pushed the immovable-selection-handles branch from da9aa08 to 056f5e1 Compare May 3, 2019 16:53
@justinmc justinmc force-pushed the immovable-selection-handles branch from 363f071 to 49b8d57 Compare May 7, 2019 21:59
@justinmc justinmc force-pushed the immovable-selection-handles branch from 49b8d57 to c66739c Compare May 10, 2019 23:25
@justinmc
Copy link
Contributor Author

The last problem that I'm fighting with is that the GestureDetector swallows tap events from the text field behind it. This manifests as double tapping to select text being broken (the first tap shows the handle, which blocks the second tap).

@HansMuller I've got a working solution by using Listener instead of GestureDetector, like we talked about. I was able to reuse the dragging logic from GestureDetector, but I reimplemented tap myself (poorly) because reusing any code caused the event to be swallowed. I'll talk to you on Monday about it, and hopefully I can bypass having to write my own tap if I understand a bit more about the event arena.

@justinmc justinmc force-pushed the immovable-selection-handles branch from 4d1e0c9 to 3a848ef Compare May 15, 2019 17:28
@justinmc justinmc force-pushed the immovable-selection-handles branch from 1f2364c to e0be739 Compare May 16, 2019 16:40
@justinmc justinmc force-pushed the immovable-selection-handles branch from 386ab39 to 80382f0 Compare May 28, 2019 20:09
@justinmc justinmc force-pushed the immovable-selection-handles branch from 80382f0 to 43241c3 Compare May 28, 2019 20:10
…enu and expanding selection, so just document it
@justinmc justinmc force-pushed the immovable-selection-handles branch from 19b0ae3 to 3a060b7 Compare May 29, 2019 17:12
@justinmc justinmc requested a review from HansMuller May 29, 2019 17:46
@justinmc justinmc changed the title WIP Text selection handles are sometimes not interactive Text selection handles are sometimes not interactive May 29, 2019
@justinmc
Copy link
Contributor Author

justinmc commented May 29, 2019

@HansMuller This is ready for review. The main things this PR does to achieve its goal of making the handles nicely interactive are:

  1. Refactor how handles are drawn so that they can be easily padded to a nice 48x48 interactive size. TextSelectionControls renders the handles in an encapsulated way, and the parent TextSelectionHandleOverlayState handles padding them and putting them in the right place wrapped in a GestureDetector.
  2. Changed how the selection overlay uses CompositedTransformFollower and the gesture arena to get interactions to work on both handles and text field at the same time. This was the big time sink as there were many gestures that I needed to get right while still allowing 48x48 interaction with and without overlap of the underlying text field. See my big list of sneaky gestures above.

Also, do you have any idea what is going on with the tests?

Copy link
Contributor

@HansMuller HansMuller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, just a few small suggestions for simplifications, etc.

Copy link
Contributor

@HansMuller HansMuller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@justinmc justinmc force-pushed the immovable-selection-handles branch from fd47bef to b8870ac Compare May 30, 2019 16:29
@justinmc justinmc force-pushed the immovable-selection-handles branch from b8870ac to d704bec Compare May 30, 2019 17:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

iOS Textfield caret "start handle"'s top balloon does not allow drag

4 participants