linebender / druid

A data-first Rust-native UI design toolkit.
https://linebender.org/druid/
Apache License 2.0
9.44k stars 569 forks source link

implement compute max intrinsic on env scope #2382

Open matthewgapp opened 1 year ago

matthewgapp commented 1 year ago

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 of compute_max_intrinsics.

Because the default implementation calls layout (instead of compute_max_instrinsic), the compute_max_intrinsic call chain is broken whenever an intermediate child uses the blanket implementation. This sucks and makes the compute_max_intrinsic method pretty useless in ancestor components when descendants 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 call layout on every leaf.

matthewgapp commented 1 year ago

Moving discussion of the breaking change to https://github.com/linebender/druid/issues/2383