Correctly support Arrangement.spacedBy in ScrollbarAdapters for lazy lists/grids #380
Conversation
| @@ -147,4 +153,5 @@ internal object EmptyLazyStaggeredGridLayoutInfo : LazyStaggeredGridLayoutInfo { | |||
| override val viewportEndOffset: Int = 0 | |||
| override val beforeContentPadding: Int = 0 | |||
| override val afterContentPadding: Int = 0 | |||
| override val mainAxisItemSpacing: Int = 0 | |||
There was a problem hiding this comment.
Better to wait for https://android-review.googlesource.com/c/platform/frameworks/support/+/2397134 to merge, and then cherry-pick it straight into jb-main
There was a problem hiding this comment.
Done. The cherry-pick is part of this PR, so you can review it too.
Once approved, I will merge --ff-only this branch into jb-main, to avoid squashing the two commits.
There was a problem hiding this comment.
Let's push the cherry-pick straight into jb-main.
The alternative process is as you described. It is better because we review everything that adds to the main branch, but worse because:
- we have to enable merging of PR's. If we do that, someone can use it, and don't bother about commits in the PR. It will spoil the git history.
- the review of a cherry-pick and a fix together is harder than just fixes.
- the 2. can be fixed by squashing the fix into a single commit, but it requires force-push, that is also not good for review.
There was a problem hiding this comment.
On the other hand, we also lose ability to run CI (we'll configure it in the future) on the cherry-pick.
And one more issue. When it will be merged, the code is become common. And when users tried to use it, they encounter noSuchMethod on Android. I don't know good way to solve it. We have 2 options:
- wait for the Jetpack release, and point to it in our sources. But if it will be 1.5, we can't merge it right now
- make public API internal, fix conflicts later (we should add comments near internal to describe how to solve conflicts)
There was a problem hiding this comment.
I'll adjust the cherry-picked commit to make the new properties internal for now.
m-sasha
force-pushed
the
bugfix/scrollbar-with-arrangement-spacedby
branch
3 times, most recently
from
36db25f to
f7efc4e
Compare
m-sasha
force-pushed
the
bugfix/scrollbar-with-arrangement-spacedby
branch
from
f7efc4e to
14075d9
Compare
Proposed Changes
mainAxisItemSpacingproperty to LazyListLayoutInfo, LazyGridLayoutInfo and LazyStaggeredGridLayoutInfo, per discussion with Andrey Kulikov from Google.mainAxisItemSpacingproperty to fix the line size approximation inLazyListScrollbarAdapterandLazyGridScrollbarAdapterTesting
Test: manually and with a new unit test
Issues Fixed
Fixes: JetBrains/compose-multiplatform#2644