Doist / RecyclerViewExtensions

RecyclerView made easier.
MIT License
493 stars 42 forks source link

Correct item count value #23

Closed panpeter closed 6 years ago

panpeter commented 6 years ago

This PR changes how we check for the number of items in the adapter.

It is possible that during data update, mCount - itemCount is 0 and the empty view is shown. In fact, it is just a temporary state and the real number of items is different.

Checking for #getItemCount() fixes this issue.

goncalossilva commented 6 years ago

If we're outright checking for the final size, shouldn't we use a Handler and post so that we only do this calculation one time once all updates are done?

goncalossilva commented 6 years ago

Well I guess the commit message answers my own question :smile: https://github.com/Doist/RecyclerViewExtensions/commit/e26b836660b31ed9eeb172530fcc7b33f27a39d7

goncalossilva commented 6 years ago

Hum, I can't see a reason why post() wouldn't work. It would be processed as the list changes are also processed. :thinking:

panpeter commented 6 years ago

Yeah, I'm also not sure why post() wouldn't work but there must have been a reason to remove it in the https://github.com/Doist/RecyclerViewExtensions/commit/e26b836660b31ed9eeb172530fcc7b33f27a39d7 commit 😄

The code without post() was working fine, except the bug we're fixing now, so I suggest we go without post() again. In some cases we might call checkCount() few times more but it wasn't an issue in the current version so I doubt it will be a problem now.

goncalossilva commented 6 years ago

The current version is not comparable though as it keeps track of the count by itself, while in the new one we make (potentially many) calls to getItemCount which we don't know how is implemented. Still, let's give it a try. :+1:

panpeter commented 6 years ago

Ok 👍

And thanks for the review 😄