airbnb / epoxy

Epoxy is an Android library for building complex screens in a RecyclerView
https://goo.gl/eIK82p
Apache License 2.0
8.5k stars 733 forks source link

Automatic scrolling when new data set #224

Closed davidbilik closed 7 years ago

davidbilik commented 7 years ago

I have a weird problem with autoscrolled position if I set data to my controller. I have a screen with pagination, i have ProgressModel that is at the bottom of the list and I show/hide it according to loading state. It looks that when I display progress and then show data (not hiding progress), recyclerview is scrolled so the progress is still visible.

My controller is really simple

class ProductsEpoxyController : EpoxyController() {
    var products: List<ApiProduct> by adapterProperty(this, listOf())
    var showError by adapterProperty(this, false)
    var showProgress by adapterProperty(this, true)

    override fun buildModels() {
        products.forEach {
            ProductModel_()
                    .id(it.id)
                    .product(it)
                    .addTo(this)
        }
        ProgressModel()
                .id(-100)
                .addIf(showProgress, this)
    }
}

adapterProperty is just a shortcut for rebuilding models when property changes.

The flow of commands is

showProgress = true ... (few hundred millis) products = listOf(...)

The recyclerview is scrolled at the bototm and progress is visible.

Im not really sure if this problem is related to this library, but I have a feeling that I had this issue with first version of Epoxy and I solved that by replacing Epoxy with own solution.

I can provide any more info if needed. Thanks

elihart commented 7 years ago

Hm, we do something very similar in many places in our app and haven't had this problem. This is probably just RecyclerView being weird, Epoxy never tells the recyclerview to scroll, it only dispatches item change notifications (in this case that items were inserted).

One reason I have seen RecyclerView auto scroll is if a view is focusable. In that case it scrolls to keep it in position. If that is the case you could make your loader not focusable, or set android:descendantFocusability="beforeDescendants" on your RecyclerView.

As another sanity check I would recommend turning on logging (https://github.com/airbnb/epoxy/wiki/Epoxy-Controller#debug-logs) and seeing what change notifications your controller is doing.

I believe this might also happen if you have your LayoutManager set to stack from bottom, or some other layout manager setting to keep the bottom in focus.

davidbilik commented 7 years ago

Thanks for reply, I'll try investigate it a little bit more, maybe create new sample project that I could share.

elihart commented 7 years ago

@davidbilik I can reproduce it in the Epoxy sample, the key is that the items have to be inserted at position 0. If that happens RecyclerView tries to keep the previous items in frame since it assumes that is what the user was looking at. Generally in our UI's we have a header at position 0 so we don't have the scrolling problem.

I think this behavior is often good for the user, but definitely not what we want in the case of a loader. I'm not sure the best way to disable this behavior for your use case. The easiest thing to do is have a header item at position 0. Another option is to temporarily remove the loader when you insert the new items. A third option is to register an adapter change listener and force a scroll to the top

controller.getAdapter().registerAdapterDataObserver(new RecyclerView.AdapterDataObserver() {
    @Override
    public void onItemRangeInserted(int positionStart, int itemCount) {
        if(positionStart == 0) {
            layoutManager.scrollToPosition(0);
       }
    }
});
davidbilik commented 7 years ago

Thank you very much for your investigation, I will use DataObserver solution. Since this is not related to Epoxy Im closing issue :)

jjhesk commented 7 years ago

@davidbilik do you have any dataObserver solution to implement pagination requests? thanks

ghost commented 6 years ago

I just ran into this same issue and implemented the code that @elihart suggested above, which worked. Then, I decided to get fancy and fixed my models code so it properly diff'd. Now, when I call a method that changes the sort order and it diffs instead of creating new models, my recyclerview gets pushed to the bottom.

Fragment resetScroll
Controller: Models built: 6.794ms
Controller: Item moved. From: 5 To: 0
Controller: Item moved. From: 4 To: 1
Controller: Item moved. From: 5 To: 3
Controller: Item moved. From: 6 To: 5
Controller: Models diffed: 0.808ms

When changing sort, I call the fragment's resetScroll to scroll to position 0. Then, when the sort occurs, it pushes the list to look at the bottom element.

I'm also wondering if, since the user probably doesn't want to see the entire list animate the sort, it's possible to force the models to bypass diffing and recreate themselves but I'm unsure of how to actually do that.

So a two-fer:

  1. Wondering if there's a way to apply the same sort of fix for item moved - if I check linear layout manager's first visible position, even if it's zero and I scroll to zero after, it still pushes the recyclerview to the bottom afterwards
  2. Is it possible to force models to recreate themselves (or maybe reset the controller)?
elihart commented 6 years ago

@josephyanks you can reset the models without animation by just creating a new one and using RecyclerView#swapAdapter. Another possibility to skip animations might be a custom ItemAnimator. Besides that though you're just fighting against RecyclerView defaults and there isn't a lot of flexibility there.

I would have thought that the fix would work the same for item moves. I believe the same principle is at play, where RecyclerView tries to keep the previously focused item in view. It could be that item move animations take longer or are delayed, so you could try delaying your scrolling (post or postDelayed)

ghost commented 6 years ago

@elihart thanks for the tip on RecyclerView#swapAdapter - that works perfectly. The postdelayed also worked but it occasionally caused some weird scrolling (scrolling to the bottom, then back up).

jkreiser commented 4 years ago

I was experiencing a similar problem, where my paged list would scroll to the bottom intermittently on first page load. @elihart was right about the RecyclerView trying to keep the previously focused item in view. In my setup, I had a "loading" item with a progress spinner that would show before initial page load, then when the page had loaded, the loading item would be removed. However, the loading item wasn't removed before the RecyclerView would identify it and automatically scroll to it, where it was now on the bottom, as my first page of items had been inserted before it. To fix this, I gave my loading item a different id depending on whether this was the first page or a subsequent page, and the RecyclerView no longer saw these 2 loading items as the same, and it stopped automatically scrolling to the bottom.

  override fun addModels(models: List<EpoxyModel<*>>) {
    super.addModels(models)

    when {
      isLoading -> {
        loadingItem {
          // When the initial loading item has the same id as the subsequent loading items,
          // the RecyclerView will intermittently treat the initial loading item as the same item
          // as the subsequent loading item and scroll all the way to the bottom after the first
          // page has loaded because it's trying to keep what it identifies as the same view in focus.
          // To prevent this, give the initial and subsequent loading items different ids.
          id(if (models.isEmpty()) "loading_initial" else "loading_next")
        }
      }
      models.isEmpty() -> {
        emptyItem {
          id("empty")
        }
      }
    }
  }
Abhishekhm777 commented 3 years ago
controller.getAdapter().registerAdapterDataObserver(new RecyclerView.AdapterDataObserver() {
    @Override
    public void onItemRangeInserted(int positionStart, int itemCount) {
        if(positionStart == 0) {
            layoutManager.scrollToPosition(0);
       }
    }
});

I had the same issue with epoxy and pagination, whenever i dump the list for the first time it used to scroll down. Fixed that issue with the above code.

vishalkumarsinghvi commented 3 years ago

it work just I added this line my carousel onBind { , view, -> view?.scrollToPosition(0) } that's it

boldijar commented 3 years ago

In my case the first item of the list contains an determinate progress bar, and my solution was in the epoxy model to add a click listener that does nothing to the root element, and now the list is scrolled to the top and not to the half of the first item.