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.

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

cc @Andrey Kulikov

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.

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

fillParentSize()

would be defined only on

LazyItemsScope

?

yes

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

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.

I could get behind this plan 👍

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!

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

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