OverlayPortal.overlayChildLayoutBuilder
#164034
Conversation
Such that nobody `implements` it. Currently it has private APIs used by the `RenderObject` class (e.g., the dirty lists). If someone were to implement a `PipelineOwner` outside of object.dart, they will get a runtime crash since they won't be able to provide those private APIs and RenderObjects actually expect those APIs to exist.
This allows `overlyChild` to inspect the layout of the regular child (for example, computing the size and the paint transform from the Overlay to the regular child in `overlayChild`'s performLayout method).
…ays-add-to-dirty-list
…nt-transform-builder
…nt-transform-builder
…ays-add-to-dirty-list
…ongCatIsLooong/flutter into OverlayPortal-always-add-to-dirty-list
…tal-paint-transform-builder
OverlayPortal.overlayChildLayoutBuilder (placeholder name)OverlayPortal.overlayChildLayoutBuilder
…nt-transform-builder
…nt-transform-builder
be61564 to
8fe1a1f
Compare
| // [ConstrainedLayoutBuilder.builder] needs to be called. | ||
| ConstraintType? _previousConstraints; | ||
| // The LayoutInfoType that was used to invoke the layout callback with last time, | ||
| // during layout. The `_previousLayoutInfo` value is compared to the new ones |
There was a problem hiding this comment.
typo:
| // during layout. The `_previousLayoutInfo` value is compared to the new ones | |
| // during layout. The `_previousLayoutInfo` value is compared to the new one |
| this.renderObject; | ||
| assert(renderObject.child == child); | ||
| renderObject.child = null; | ||
| assert(renderObject == this.renderObject); | ||
| } | ||
| } | ||
|
|
||
| /// Generic mixin for [RenderObject]s created by [ConstrainedLayoutBuilder]. | ||
| /// Generic mixin for [RenderObject]s created by [AbstractLayoutBuilder]. |
There was a problem hiding this comment.
I am a little confused by this class. Is this supposed to work with generic LayoutInfoTypes (like AbstractLayoutBuilder does) or with Constraints specifically? The doc here and the LayoutInfoType generic argument seem to imply its supposed to be generic. But then the name (RenderConstrainedLayoutBuilder) and the fact that it hardcodes Constraints in multiple places below seem to indicate the opposite. Could you clarify this and make it clear whether it is one or the other? Or make it clearer why this is both?
There was a problem hiding this comment.
I think it's the former. Added a lot more text to the class / method documentation.
There was a problem hiding this comment.
I'll try to rename this to AbstractLayoutBuilderMixin. Hopefully I can get away with a rename.
There was a problem hiding this comment.
Looks like all registered tests passed.
|
|
||
| /// Change the layout callback. | ||
| void updateCallback(LayoutCallback<ConstraintType>? value) { | ||
| void _updateCallback(void Function(Constraints)? value) { |
There was a problem hiding this comment.
Should this be typed LayoutCallback<Constraints>? Also above.
|
|
||
| /// The additional layout information available to the | ||
| /// [OverlayPortal.overlayChildLayoutBuilder] callback. | ||
| extension type OverlayChildLayoutInfo._( |
There was a problem hiding this comment.
:)
Personally, I would have gone with a regular class here for slightly better readability and ergonomics. But this is fine.
There was a problem hiding this comment.
That way I don't have to manually write the == and hashCode implementation.
| void rebuildIfNecessary() { | ||
| assert(_callback != null); | ||
| invokeLayoutCallback(_callback!); | ||
| } | ||
|
|
||
| /// The (subset of) information to invoke the `builder` callback with. |
There was a problem hiding this comment.
nit: lets specify here where one can find the builder callback since it isn't on this class (e.g. link this to [AbstractLayoutBuilder.builder]).
This API allows the widget tree of an
OverlayPortal.overlayChildto depend on various layout information (e.g. the incoming constraints, or the size of a RenderObject) from another child subtree ofOverlaythat does layout before theoverlayChild.Most RenderObject subclasses can only access its child's or children's layout info, but not the layout info of its other descendants because of the "relayout boundary" optimization. Such locality makes the layout dependencies easier to reason about but it also makes it difficult to implement certain common UI patterns (see the examples in the description of the previous PR)
The API is currently only available on
OverlayPortalas it is the only Overlay API (AFAIK) that guarantees every render object in a "path" within the render tree has finished doing layout.TODO: polish the API docs and code comments
TODO: more tests?
TODO: markNeedsLayout should not imply markNeedsPaint in this case (or in layout builders in general).
part1: #163575
diff this ... part1 LongCatIsLooong/flutter@OverlayPortal-always-add-to-dirty-list...LongCatIsLooong:flutter:OverlayPortal-paint-transform-builder
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.