stream-chat-android icon indicating copy to clipboard operation
stream-chat-android copied to clipboard

ChannelListView automatically scrolls to bottom on load

Open jeff-huston opened this issue 3 years ago • 5 comments

Describe the bug Pagination logic in the ChannelListViewModel causes the ChannelListView to scroll to the bottom when it shouldn't.

This manifests slightly differently in our app implementation than it does in the UI Components sample.

  • In our app, every time we load a fresh ChannelListView, it scrolls to the bottom right away.
  • In the Stream UI Components Sample, this doesn't happen at initial load, but it happens if you switch over to the @Mentions tab and then go back to the Chats tab.

SDK version

  • 5.7.0 (and earlier, I believe. I think it's been present for several minor versions at least, but was not present in 4.X)

To Reproduce Steps to reproduce the behavior:

  1. Go to the UI Components Sample app
  2. Let the Chats tab load fully (stay scrolled to the top)
  3. Click on the @Mentions tab
  4. Click on the Chats tab
  5. [Bug] the Chats tab autoscrolls down to the last item in the currently loaded page, shows the Loading More item, and then hides the Loading More item but remains scrolled down.

Expected behavior Chats tab is still scrolled to the top.

Device:

  • Vendor and model: Samsung GS21, Pixel 5
  • Android version: 12

Screenshots

https://user-images.githubusercontent.com/2012846/183758163-432398b5-5129-4a88-9128-018f8974daa4.mov

jeff-huston avatar Aug 09 '22 20:08 jeff-huston

I'm curious what we're doing differently that causes this to happen on initial load of the ChannelListView in the case of our app. I'd be interested if you think there's a workaround that would prevent our app from having this issue. In our case we use a custom fragment with a ChannelListView in the layout, and call channelListViewModel.bindView(binding.channelsView, viewLifecycleOwner) in onViewCreated. Let me know if I can provide any other details that seem relevant to why it might be happening on initial load for us. Thanks!

jeff-huston avatar Aug 09 '22 20:08 jeff-huston

Hey @hustoj2!

Can you confirm this happens for you on the latest develop of our SDK, for the UI Components app? As this is what I'm seeing.

https://user-images.githubusercontent.com/17215808/183842284-a4bb1c20-d5ab-47b8-a847-f9c233e62388.mp4

I even tried it on both debug and release and it seems to be working fine, without any jumps.

In your case - I think there might be more queries happening - if you do have your fragment's setup in onViewCreated, if that function is called multiple times (e.g. like re-creating a fragment), then it will bind the VM/View multiple times, which will trigger the queryChannels calls with the same filter and sort.

Since we cache our queryChannels calls, it could trigger pagination which means there might be item insertions when coming back to the fragment and this in turn can cause a shift in the list.

I can't do much without a better look at the project you're working on (and for obvious reasons that might not be easily doable), so if you could try caching your fragment when going back to it, or checking in onViewCreated if you're calling it multiple times, I think we might see if that's the case.

Otherwise, if you could provide a small sample project where you can repro this and add me as a contributor, I could take a look and see if we can work it out! :]

Hope this helps and makes sense!

Edit: I also tested on Pixel 5, Android 12, so it should be a good comparison. (un)fortunately I don't have a Samsung phone to test

filbabic avatar Aug 10 '22 07:08 filbabic

I was originally testing on main, but I just switched to the latest develop and I do still see the issue on the UI Components app. I just tested on my Pixel 5 on Android 12 as well and see it there also. I can switch back and forth between the tabs and the issue repeats every time on both devices. I wonder what the difference could be - maybe connectivity speed?

jeff-huston avatar Aug 10 '22 14:08 jeff-huston

As for my app, your comment gave me a little better picture and I noticed the behavior is actually different the first time it loads vs. subsequent times.

The first time, there's an odd jump but it ends up scrolled to the top correctly. Subsequent times, there's an odd animation and it scrolls to the bottom (which is this bug). So upon closer inspection, I'm seeing the same behavior in the UI Components sample app as I'm seeing in my own app.

Note: in my app it's a brand new fragment and view model instance each time, but like you said the Stream SDK caches the queries which presumably explains the differing subsequent behavior. onViewCreated is only being called once each time I load the screen.

Can you explain your suggestion that I try "caching the fragment"? This might happen if I put all the sibling Channel lists in my app into a ViewPager, which is actually a direction we might be taking soon. So maybe that change on our end will help alleviate this issue. Did you have a different approach in mind for "caching the fragment" though?

jeff-huston avatar Aug 10 '22 14:08 jeff-huston

Hey @hustoj2!

We seem to be able to reproduce this in one of our guides apps, so we'll take a look and see if we can find the root cause and fix it.

As for caching the fragment - basically not to recreate it every time you go to a given page (e.g. mentions, chat). But just replace the fragments in the manager.

IDK if the onViewCreated (I imagine that's when you're binding the UI) is being called multiple times in this scenario and if it's truly a pagination issue.

Either way - we'll dig a bit more and see if we can find the underlying problem!

filbabic avatar Aug 11 '22 11:08 filbabic

Hi Filip, does your team have any updates on this? Caching the fragment does indeed work, but in our app the Channels list isn't at the top level - you need to drill down to get to it. So I don't see an appropriate way to cache the fragment.

In the meantime, I've implemented this workaround. Instead of calling bindView in onViewCreated, I wait for the paginationState to emit loadingMore=false:

fun ChannelListViewModel.bindViewAfterChannelsLoaded(
    view: ChannelListView,
    lifecycleOwner: LifecycleOwner,
    ) {
        var isBound = false
        paginationState.observe(lifecycleOwner) { paginationState ->
            if (!paginationState.loadingMore && !isBound) {
                isBound = true
                bindView(view, lifecycleOwner)
            }
        }
    }

jeff-huston avatar Sep 06 '22 15:09 jeff-huston

I just looked at the release notes for 5.9.0, and it includes this PR which appears likely to fix the issue. I'll test that out and let you know if it fixes our use case.

jeff-huston avatar Sep 07 '22 13:09 jeff-huston

5.9.0 does fix this issue for me. Thank you! I'll leave it to you to close this issue in case there's a different aspect I'm not aware of. But as far as I know, this can be closed.

jeff-huston avatar Sep 07 '22 14:09 jeff-huston

Hey @hustoj2 sorry for the late reply!

Yeah we've worked on implementing and fixing the issue, there's lots going on (and GH is being wonky).

I'm happy the issue is fixed for you - if you find an aspect that doesn't work, let me know and we'll open a new ticket and take a look!

Once again, thank you for being patient and understanding.

filbabic avatar Sep 07 '22 14:09 filbabic