ViewHolders offscreen are not being recycled when adapter is updated

keithsmyth commented 7 years ago

[x] I have searched existing issues and confirmed this is not a duplicate

Issues and steps to reproduce

Run the attached project, we have 4 items Header | About | Fine Print | Map
On a Nexus 5X the Header and About are in view on load, notice in logcat their ViewHolders are created
Fine Print is way out of screen, its ViewHolder is created also (not perfect, but forgivable)
Press the FloatingActionButton to tell the adapter to update, it reuses Header and About ViewHolders, but it then creates new Fine Print and Map ViewHolders every time the adapter is updated.
Scroll to the bottom and back up to the top, Fine Print and Map are created once more, but are then reused from that point onward.

Expected behavior

Should reuse ViewHolders instead of recreating them. Switching to LinearLayoutManager shows this behavior.

Version of the flexbox library

0.3.0, 0.3.1

Link to code

https://github.com/keithsmyth/TestDealDetailsHarness

thagikura commented 6 years ago

Thanks for filing this and my apologies for the late reply. Let me reproduce it using your test project.

stephanenicolas commented 6 years ago

Let us know if you need anything, or have questions about the issue @thagikura

thagikura commented 6 years ago

Hi,

I checked the project and the behavior.

Short answer: For now it's the intended behavior.

Long answer: Let me explain.

FlexboxLayoutManager clears the viewHolders (and FlexLines) starting from the first visible part to the end.

E.g. Let's say I have following ViewHolders and FlexLines. And ViewHolders 5, 6 are the first visible view holders.

[ 1 ][2][ 3 ] FlexLine 0 [ 4 ] FlexLine 1 --------------- Top edge of the screen ---------- [ 5 ][ 6 ] FlexLine 2 [ 7 ][8] FlexLine 3 [ 9 ] FlexLine 4 --------------- Bottom edge of the screen ------ [ 10 ] [ 11 ] FlexLine 5 [ 12 ][ 13 ] FlexLine 6

In that case, every time notifyDatasetChanged is called (onLayoutChildren is called in the LayoutManager), FlexboxLayoutManager clears the FlexLine from 2 to 6 and ViewHolder from 5 to 13. Then recreates FlexLines and ViewHolders initially only the visible part and as the user scrolls to bottom, recreates the new appearing FlexLines and ViewHolders.

The reason for that behavior is that FlexboxLayoutManager can't predict which ViewHolders can be still reused after the first visible position (or the first position where a ViewHolder is updated). Assume that the size of the ViewHolder 5 is changed then the FlexLine 2 now only has ViewHolder 5 instead of having 5 and 6. Then the following FlexLines from 3 need to recalculated because how many ViewHolders each FlexLine is able to have is affected by adding the ViewHolder 6.

I hope the above can explain these behaviors.

Press the FloatingActionButton to tell the adapter to update, it reuses Header and About ViewHolders, but it then creates new Fine Print and Map ViewHolders every time the adapter is updated. Scroll to the bottom and back up to the top, Fine Print and Map are created once more, but are then reused from that point onward.

Also to give an explanation about this

Fine Print is way out of screen, its ViewHolder is created also (not perfect, but forgivable)

This is because when calculating the visible potion of the FlexLines and ViewHolders, FlexboxLayoutManager calculates a little ahead of actual visible potion otherwise it can't know how many ViewHolders the last visible FlexLine is able to have.

thagikura commented 6 years ago

Please let me if it answers your issues/questions.

keithsmyth commented 6 years ago

Hey thanks for the nice, in depth answer.

I can totally understand clearing all the lines from current top edge of screen, but what I still don't understand is why we can't reuse the same ViewHolders for items 10,11,12 and 13?

thagikura commented 6 years ago

Oh sorry part of my explanation could be misinterpreted. Actually 10, 11, 12 and 13 are reused once it's created.

Then recreates FlexLines and ViewHolders initially only the visible part and as the user scrolls to bottom, recreates the new appearing FlexLines and ViewHolders.

The actual behavior is like the following:

notifyDataSetChanged is called, then clears FlexLines 2 to 6 and ViewHolders 5 to 13
FlexboxLayoutManager creates ViewHolders 5 to 13 (assuming the size of them are not changed) and FlexLines 2 to 6
When the user scrolls to the bottom, ViewHolders 10, 11, 12, 13 should be reused and 14 and following ViewHolders are newly created to fill the part the user scrolls.

google / flexbox-layout