Fix issues #22576: Text edit tootip arrow is misplaced - #31676

I think that you shouldn't describe something like a default here where the enum is defined, because it's far from where the code that defines the default is actually located. This enum could be reused somewhere else with a different default, or the default behavior could change below and the author might forget to update this documentation.

triangleOffsetX is final, so it can never change, and this check shouldn't be necessary, right? Same for arrowDirection.

Can you explain this more? I worry about adding a timer like this.

This timer workaround looks like a hack to me.
Maybe the toolbar should use multi child layout, so you can have the size of the toolbar ready when you position the triangle?

Could this be provided in the constructor instead of starting as nan and needing to call updateTriangleOffsetX?

This seems to me like it should just be passed into _TextSelectionToolbar like a normal parameter rather than manually updated like this. Maybe you were running into problems that caused you to use this pattern and a timer?

The exact position of the triangle requires the true width of the toolbar to be determined to prevent the triangle from appearing outside the toolbar.

Within the _TextSelectionToolbar class, we only know the number and type of menus to display but don't know the true width of the toolbar. Only the childSize parameter in the Offset getPositionForChild(Size size, Size childSize) method in _TextSelectionToolbarLayout class is the final true width of the toolbar, so it is necessary in _TextSelectionToolbarLayout class to calculate the true position of the triangle and pass the data out.

As for why a Timer is needed to delay execution of the setState(() {}); method by 50 milliseconds? Immediate execution causes the toolbar to always appear in the upper left corner of the screen. : (

Of course, the current implementation is not a very good solution, I hope we can discuss a better one. : )

@jiisd I just dug into this and I think I agree with @LongCatIsLooong that MultiChildLayoutDelegate is the way to go. You should be able to position the triangle inside the delegate without having to pass around triangleOffsetX and without having to use a timer.

That will require a bit of refactoring existing code. Let me know if you don't mind taking that on, otherwise I can take over the PR and do it myself if you want.

Although this solution now achieves this effect, it is very difficult to understand, and the responsibilities of the class are not clear.;(

I think the MultiChildLayoutDelegate approach is better than the current one. Of course, it would be better if you had time to work on it together.🤝

Hi @jiisd sorry for the late response. That sounds good to me! I'm going to work on this and create a pull request based on your changes. I'll let you know when the PR is open.

I see 3 names here for the same thing: notch, arrow, and triangle. It looks like we use multiple names in the existing code too though :( Maybe try to choose the most common one and stick with it if you think it will help.

This seems like it's from your other PR (#29809) and it verifies the top/bottom position, but you should write a test that verifies the arrow position.

I think if you just cover the 3 cases it should be fine. Test that selecting in the middle of an input puts the arrow in the middle (as before), selecting the first word puts the arrow left of center, and selecting the last word puts the arrow right of center.

It might be hard to find the arrow in a test, but I think you can do it. Here is a snippet from another test that finds something by name and descendant, it should help here:

Finder findFadeUpwardsPageTransition() {
  return find.descendant(
    of: find.byType(MaterialApp),
    matching: find.byWidgetPredicate((Widget w) => '${w.runtimeType}' == '_FadeUpwardsPageTransition'),
  );
}