evant / binding-collection-adapter

Easy way to bind collections to listviews and recyclerviews with the new Android Data Binding framework
Apache License 2.0
1.92k stars 255 forks source link

Jetpack Paging v3 support #194

Closed kedzie closed 4 years ago

kedzie commented 4 years ago

193

Added support for Jetpack Paging v3 with LoadState/retry()/refresh() support

ezgif com-video-to-gif

evant commented 4 years ago

This is a great start thanks!. For the LoadState stuff, I think it might make sense to push it up a level into a MergeObservableList instead of using the ConcatAdapter/LoadStateAdapter. This should make it play better with the other parts of the lib. Working on a branch to do just that, will put it up when I've got something. Still need to think about how to connect the retry callback though.

kedzie commented 4 years ago

I added a commit which adds a itemBinding based LoadStateAdapter. But I see you point, if we can merge all into the list then we can provide bindings all in the same adapter. instead of managing ConcatAdapter.

Either way the itemBinding based LoadState allows consumer to do this

val footerItemBinding = OnItemBindClass<LoadState>().apply {
        map<LoadState.Error>(BR.item, R.layout.network_state_item_error)
        map<LoadState.Loading>(BR.item, R.layout.network_state_item_progress)
    }.toItemBinding()

I suggest we provide a tatarka interface with retry() and refresh() methods. And we can bind an instance of that to the consumer's loadstate ItemBinding.

kedzie commented 4 years ago

I just pushed a commit to tie in retry() and refresh(). I used the factory method

val loadStateItemBinding = LoadStateItemBindingFactory { callback: PagedListCallback ->
        OnItemBindClass<LoadState>().apply {
            map<LoadState.Error>(BR.item, R.layout.network_state_item_error)
            map<LoadState.Loading>(ItemBinding.VAR_NONE, R.layout.network_state_item_progress)
        }.toItemBinding().bindExtra(BR.callback, callback)
    }

it works. I'd like to circle back to the MergeObservableList and see if I can help out.

kedzie commented 4 years ago

Challenges I see with the MergeObservableList is how the consumer will specify the bindings. If it is all a single itemBinding no way to distinguish between header and footer elements.

For example: (no way to know if the error/progress bindings are for the header or footer)

val allItems = LoadStateItemBindingFactory { callback: PagedListCallback ->
        OnItemBindClass<Any>().apply {
            map<ImmutableItem>(BR.item, R.layout.item_immutable)
            map<LoadState.Error>(BR.item, R.layout.network_state_item_error)
            map<LoadState.Loading>(ItemBinding.VAR_NONE, R.layout.network_state_item_progress)
        }.toItemBinding().bindExtra(BR.callback, callback)
    }

Be nice if consumer could specify separate bindings for items and the header/footer. Then we could make a way to combine those into a single ItemBinding. Then we could just maintain 3 ObservableLists, all merged together.

Only other issue is that our code would basically duplicate logic from LoadStateAdapter. Instead of notifying adapter it would update the underlying observableList.

evant commented 4 years ago

Alright, pushed up https://github.com/evant/binding-collection-adapter/pull/195, thoughts?

kedzie commented 4 years ago

I dig it. posted some comments related to retry/refresh. I like the custom OnItemBindLoadState; i was thinking same regarding the merged list approach. could we pass in a variable name for the retry callback in the OnItemBindLoadState constructor and then bind it the list? We could pass in the list reference when it is created in the binding adapter.

Only concern is that by not extending LoadStateAdapter we are forced to duplicate the logic it contained. it is pretty simple but it is something you will have to keep in sync if it changes.

kedzie commented 4 years ago

Pushed new PR #196