Saw the <PR to add LazyItemsScope>. It's a neat wa...
# compose
z
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.
l
👍 good point, this feels like it could easily be missed
m
cc @Andrey Kulikov
z
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.
a
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
👍 1
z
fillParentSize()
would be defined only on
LazyItemsScope
?
a
yes
👍 1
l
yeah i think that + making fillMaxSize() a deprecated error with replacewith on the scope as well leads to a decent DX
👍 1
still possible to do the wrong thing, but at least covers a lot of what people will first try
z
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.
a
I could get behind this plan 👍
a
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!
👀 1
l
i’m gonna start cc’ing zach on my CLs lol
😄 4
😂 1
z
I usually start my day by looking for new ones and starring the interesting ones, so that would save me some work 😛
👌 1