Insert top bar on column instead of scaffold

[pipe01]

It seems like whenever the home top bar changes height due to scrolling up or down, everything gets recomposed causing a ton of lag. It seems like this is caused by the Scaffold component calculates the height of the top bar to calculate the top padding, which I guess is enough to cause the recompositions.

To solve this I've moved the MainTopBar component inside the Scaffold's content, wrapping it along with the listings into a Column. This is basically what the scaffold does so from what I can tell there should be no side effects.

[dessalines]

Could you do this same check on the other screens that use topBar = { ... ?

Seems like any screen which uses a LazyList (which is I think all of them), might suffer from this.

---

Also maybe we're just handling this the wrong way, passing something we shouldn't.. idk. I find it hard to believe that this extremely common case, using a Scaffold with a topBar, and content containing LazyList, is broken and inefficient for all jetpack compose apps.

[pipe01]

I've traced down the issue to using the PaddingValues passed to the Scaffold's content. What's really weird is that it doesn't matter how you use it, even logging it causes everything to recompose while not referencing it at all doesn't recompose.

Basically, this is laggy:

                content = {
                    Log.d("test", "Padding: $it")

                    MainPostListingsContent(
                        padding = PaddingValues(0.dp),
                        homeViewModel = homeViewModel,
                        siteViewModel = siteViewModel,
                        postEditViewModel = postEditViewModel,
                        appSettingsViewModel = appSettingsViewModel,
                        account = account,
                        ctx = ctx,
                        navController = navController,
                        postListState = postListState,
                        showVotingArrowsInListView = showVotingArrowsInListView,
                    )
                },

and this isn't:

                content = {
                    MainPostListingsContent(
                        padding = PaddingValues(0.dp),
                        homeViewModel = homeViewModel,
                        siteViewModel = siteViewModel,
                        postEditViewModel = postEditViewModel,
                        appSettingsViewModel = appSettingsViewModel,
                        account = account,
                        ctx = ctx,
                        navController = navController,
                        postListState = postListState,
                        showVotingArrowsInListView = showVotingArrowsInListView,
                    )
                },

[pipe01]

Okay, I've traced down the (sort of) root cause. It seems like, for whatever reason, when in MainPostListingsContent we create lambdas for the PostListings callbacks (e.g. onUpvoteClick) we must make sure not to reference any function parameters, otherwise all posts will be recomposited. We can mitigate this by adding remembers like val homeViewModel by remember { derivedStateOf { homeViewModel } }.

This solves the posts recompositing, but the bottom bar also seems to want to recomposite just like the posts. This can be "solved" by disabling the floating button on the Scaffold. We could instead add a floating button some other way, shouldn't be too hard.

However, I think that all of this feels like more of a hack than what this PR does...

[dessalines]

Could we just hard-code the padding, or would there still be issues?

Also could you open up an issue on stackoverflow? Seems like such a weird bug that neither of us can wrap our heads around.

[shortwavesurfer2009]

I am no expert by any means, so grains of salt are needed. With that said, i think part of this is how many posts are requested at once from the server. From what i could tell it only loads 10 at a time. I sort by "new" by default so afrer long enough away i have to go back 8+ hours of posts which requires much scrolling and requesting. I am not sure how many the lemmy API can give at once, but i would think 50 would be better.

Edit: i think i remember reading somewhere that either Apollo or RedReader would request 100 posts at once.

[dessalines]

We could increase the post fetch limit, as long as its not too high.

[dessalines]

Stale PR. I can re-open if necessary.