Correctly support Arrangement.spacedBy in ScrollbarAdapters for lazy lists/grids#380
Conversation
| 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.
36db25f to
f7efc4e
Compare
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