Saw the <PR to add LazyItemsScope>. It's a neat way to "magically" make those modifiers do what you'...

# compose

[Zach Klippenstein (he/him) [MOD]]

Saw the PR to add LazyItemsScope. It's a neat way to "magically" make those modifiers do what you'd intuitively expect them to do, but it seems like it would be really easy to accidentally factor out a composable and not realize that you're using the shadowed modifier and forget to add the scope to the factored composable.

[Leland Richardson [G]]

👍 good point, this feels like it could easily be missed

[matvei]

cc @Andrey Kulikov

[Zach Klippenstein (he/him) [MOD]]

Not sure what a better solution would be. There could be an ambient that defines the current “viewport” size, which would be used by those modifiers to determine constraints, but then this ambient would have to potentially be defined at every layout node which seems like the wrong thing to use an ambient for. Also seems maybe over-engineered to introduce a general concept of this sort of viewport thing just for these lazy items composables.

[Andrey Kulikov]

yeah, I agree that it could be a bit dangerous. there is no way to make Modifier.fillMaxSize() work in any other approach. but we can intentionally use a different name, something like Modifier.fillParentSize(). everyone will anyway first try to use Modifier.fillMaxSize() and then realize it doesn't work. We can also add deprecated Modifier.fillMaxSize() in a scope just to shadow the real one and point people to the correct one 🙂 . my initial version had just val parentConstraints: DpConstraints in the scope so you have to explicitly Modifier.size(width = parentConstraints.maxWidth, height = parentConstraints.maxHeight) . and yes, we don't really want to use ambients for something defined not during composition, but during the measurement

[Zach Klippenstein (he/him) [MOD]]

fillParentSize() would be defined only on LazyItemsScope?

[Andrey Kulikov]

yes

[Leland Richardson [G]]

yeah i think that + making fillMaxSize() a deprecated error with replacewith on the scope as well leads to a decent DX

[Leland Richardson [G]]

still possible to do the wrong thing, but at least covers a lot of what people will first try

[Zach Klippenstein (he/him) [MOD]]

Agreed, gives enough of a hint to guide you into doing the right thing the first time you refactor. Also makes the intent of using that modifier extra crystal clear.

[Adam Powell]

I could get behind this plan 👍

[Andrey Kulikov]

by the way, it is nice to get a decent external code review before merging the change and even before adding any documentation or tests 🙂 thanks, Zach!

[Leland Richardson [G]]

i’m gonna start cc’ing zach on my CLs lol

[Zach Klippenstein (he/him) [MOD]]

I usually start my day by looking for new ones and starring the interesting ones, so that would save me some work 😛