Would it make sense to add this to SemanticsActionHandler?

Only RenderObjects that the user can interact with (tap, etc) implement SemanticsActionHandler (= very few). However, every single RenderObject needs to implement showOnScreen for this PR to work. We could make RenderObject require to implement SemanticsActionHandler, but that seems to defeat the purpose of that interface?

The default implementation just defers up, though, right? It's semantically equivalent to the tap or scroll handlers. One render object knows how to handle it, but the others just defer up.

Not sure I get your last comment. The default implementation of what? SemanticsActionHandler is used as an interface and doesn't have a default implementation per se.

What do you mean by RenderObjects defer up if they don't know how to handle an action? As far as I can tell, actions are directly dispatched to the exact RenderObject (implementing a SemanticsActionHandler) that can handle it.

I mean if you have a gesture detector that contains a text, and you tap on the text, the gesture detector gets the tap. It happens to be because we merge them into one node and the top node is the one with the handler. But yeah, it's not "real" in the code per se.

I dunno. It feels a bit less satisfying to have every semantic node have this function pointer now. I mean we might as well just make it be a pointer to the render object really, and I was vaguely trying to avoid that, because a SemanticsNode represents many render objects, not just one.

But I don't have a good recommendation.

I tried to make it a pointer to the render object in my previous PR, but as you pointed out correctly that is a violation of the layer cake. 😄

As always, I am open to suggestions. Here are other solutions I've considered:

• The one you see in this PR.
• The one in the previous PR, violates the layer cake.
• Have a ShowOnScreenHandler interface (contains only method showOnScreen) that's implemented by RenderObject and we pass the RenderObject casted to that to the SemanticsNote. This basically follows the pattern established by SemanticsActionHandler. However, our analyzer setup discourages one-method-interfaces and it also seemed like that approach just adds more complexity without much value over the approach in this PR (given that RenderObject will be the only thing implementing that interface).
• Have every RenderObject implement the SemanticsActionHandler. In the RenderObject class we implement the logic in performAction for showOnScreen. Subclasses can override performAction to add other actions (tap, etc), but must call super to properly behave for the showOnScreen action. I think this approach is more error-prone for Flutter users and also kinda makes the SemanticsActionHandler interface useless (see previous argument).

... and then there were many ideas that don't work.

I concur with your description of the situation.

We're testing handler against null twice here. It would be more readable (and would make the comment unnecessary) if you just did this as an if/else.

But it's not clear to me why we don't call showOnScreen if there's a handler. A comment explaining that would be useful. :-)

The idea is that if a SemanticsActionHandler chooses to implement a custom behavior for the showOnScreen semantics action, the framework should not interfere with that.

Fixed the double null check.

nice

@abarth Do you have any opinions on adding jumpTo to ViewportOffset?

This seems wrong. The whole point of a _FixedViewportOffset is that it's, well, fixed.

The duplication here is unfortunate. If you can't find a clean way to deduplicate, can you add comments in both places that clearly point to the other place so that any bug fixes get applied to both?

Added comments to point to the other. If somebody has a good idea to deduplicate, let me know!

trivial nit: hyperlink [RenderObject] as well

nit: no braces