tumblr / Bookends

A UI widget for adding headers and footers to RecyclerView
539 stars 62 forks source link

Updating data in wrapped adapter does not cause list to update #8

Closed Turbo87 closed 9 years ago

Turbo87 commented 9 years ago

In the add() method of the original adapter I'm caling notifyItemInserted() to let the list know that it should update itself. If I just use that adapter with the list that works just fine. Once I wrap it in a Bookends adapter this stops working though and the list is never updated again.

Turbo87 commented 9 years ago

I haven't tested this yet, but I guess you would have to register the Bookends instance as an AdapterDataObserver via registerAdapterDataObserver() on the original adapter, then shift the positions in the callback methods to account for the header/footer items and then pass them to the notify methods of the Bookends instance.

KevinTCoughlin commented 9 years ago

@Turbo87 are you calling add on the adapter returned from Bookends#getWrappedAdapter() and also notifying that same adapter that the data set has changed?

Turbo87 commented 9 years ago

@KevinTCoughlin yes, exactly

KevinTCoughlin commented 9 years ago

That should work. Can you provide a sample of your code that's calling these methods?

Turbo87 commented 9 years ago

Unfortunately I don't have the exact code anymore but it was roughly like this:

adapter = new SimpleAdapter();
adapter.add(1);
adapter.add(2);
adapter.add(3);

bookends = new Bookends<SimpleAdapter>(adapter);

recyclerView.setAdapter(bookends);

// some time later ...

adapter.add(4);
adapter.add(5);
adapter.add(6);
KevinTCoughlin commented 9 years ago

Can you try:

adapter = new SimpleAdapter();
adapter.add(1);
adapter.add(2);
adapter.add(3);

bookends = new Bookends<SimpleAdapter>(adapter);

recyclerView.setAdapter(bookends);

// some time later ...

bookends.getWrappedAdapter().add(4);
bookends.getWrappedAdapter().add(5);
bookends.getWrappedAdapter().add(6);
Turbo87 commented 9 years ago

looking at https://github.com/tumblr/Bookends/blob/master/bookends/src/main/java/com/tumblr/bookends/Bookends.java#L63 I don't think that would make any difference

KevinTCoughlin commented 9 years ago

@Turbo87 this gist resulted in the screenshot below. I'm still unable to reproduce.

ss

Turbo87 commented 9 years ago

@KevinTCoughlin ah sorry, I think you misinterpreted my // some time later ... comment. What I meant was something like "after a few seconds", not "later in the same method". Your gist works fine because the list isn't rendered until after the onCreate() method has finished. If you delay the add() calls with e.g. something like new Handler().postDelayed() you will see that adding those values will not result in the list being rerendered again.

KevinTCoughlin commented 9 years ago

@Turbo87 does notifying the Bookends adapter instance (accounting for header count) with the modified position(s) work?

Turbo87 commented 9 years ago

@KevinTCoughlin yes, from the testing I did that seems to work. As I suggested above, in the Bookends constructor you could register a AdapterDataObserver on the wrapped adapter and transform the notifications accounting for the headers/footers.

KevinTCoughlin commented 9 years ago

@Turbo87 commented 2 hours ago @KevinTCoughlin yes, from the testing I did that seems to work. As I suggested above, in the Bookends constructor you could register a AdapterDataObserver on the wrapped adapter and transform the notifications accounting for the headers/footers.

Closing this since Bookends does work as mentioned above. Would totally welcome a PR with the AdapterDataObserver approach as a way of automatically handling inner adapter changes though.

Turbo87 commented 9 years ago

@KevinTCoughlin I don't really agree on marking that as "works as designed".

Bookends was designed as a "wrapper" API, and it is wrapping my adapter that has an add() method. That method automatically calls the corresponding notify method, but it knows nothing about the surrounding adapter and IMHO shouldn't know.