Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Track updates to underlying list in mInternalObjects on notifyDataSetChanged(). #304

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

mattgmg1990
Copy link
Contributor

When working on my project, I realized that this important piece was missing to my previous pull request. If the list that the CardArrayAdapter is referencing is modified outside of the adapter and notifyDataSetChanged() is called, mInternalObjects will be in an incorrect state. This can completely break the undo functionality.

The solution is to totally regenerate mInternalObjects when notifyDataSetChanged is called, since we have no guarantee what state the ArrayList is in anymore. Using notifyDataSetChanged is a relatively common workflow, so this should be patched.

If the list is modified outside of the CardArrayAdapter, re-generate
mInternalObjects in notifyDataSetChanged(), as it's contents are now stale.

Change-Id: I5bc6879970fbc1d8debc21d6dd0a8b8185ab2b30
@gabrielemariotti
Copy link
Owner

True, but this kind of solution can have a performance issue if the adapter has a lot of items.
I have to study this case.

@mattgmg1990
Copy link
Contributor Author

Yes, that is true that the performance will suffer with many items. However, this is necessary since in the worst case the elements in the ArrayList could be completely different than before.

In CardArrayAdapter, notifyDataSetChanged() will now rebuild
mInternalObjects from the current list contents. However, this will
discard removed cards unless they are added back in upon swiping. Fix
this bug by adding removed cards back in.

Change-Id: Ib836fc8773b7a087090760e119094d2decc1ecf6
@mattgmg1990
Copy link
Contributor Author

I still think this commit is necessary, but realized that it caused undo to become non-functional. I have fixed this properly in a57aa8b. The behavior is now what I expect it to be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants