implement compute max intrinsic on env scope #2382
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Opening this non-breaking patch PR for a discussion on a breaking change to the Widget trait API.
A little bit of context: I've built a special scroll element that has its own implementation of
compute_max_intrinsic
(I do this so that I can call compute_max_intrinsic on the child instead of deferring to the default implementation (which calls layout on the scroll element). The motivation for my component-specific implementation isn't super important to this discussion, though. What's important is that certain widgets may have a good reason to implement their own version ofcompute_max_intrinsics
.Because the default implementation calls layout (instead of
compute_max_instrinsic
), thecompute_max_intrinsic
call chain is broken whenever an intermediate child uses the blanket implementation. This sucks and makes thecompute_max_intrinsic
method pretty useless in ancestor components whendescendants
have their own implementations.This PR adds the necessary implementation to the
env_scope
component as a partial patch for my use case. But a better fix would remove the default implementation in favor of implementors being required to implement the compute_max_intrinsic (and thus call the correct method on children). This pattern is already leveraged for event, lifecycle, update, layout, and paint.Naturally, removing the default implementation would be breaking so thought I'd bring it up here. But at the moment, the default implementation makes compute max intrinsic pretty useless/broken since any intermediate components that use the default implementation wipe out any widget-specific implementations downstream.
On a positive note, the breaking change would be pretty mechanical to fix, though. Just go through and call
compute_max_intrinsic
on every non-leaf widget and calllayout
on every leaf.