Skip to content

WIP: #49 handle notify data set changed and update internal list#54

Open
dvdciri wants to merge 1 commit intodevelopfrom
feature/#49_Handle_notify_data_set_changed_and_update_internal_list
Open

WIP: #49 handle notify data set changed and update internal list#54
dvdciri wants to merge 1 commit intodevelopfrom
feature/#49_Handle_notify_data_set_changed_and_update_internal_list

Conversation

@dvdciri
Copy link
Owner

@dvdciri dvdciri commented Mar 10, 2017

This is WIP PR that is going to close #49.

I'll need to add unit tests and instrumentation tests and i'll be ready. The functionality is already there.
@smarques84

@dvdciri dvdciri self-assigned this Mar 10, 2017
@dvdciri dvdciri changed the base branch from master to develop March 10, 2017 17:21
@dvdciri dvdciri force-pushed the feature/#49_Handle_notify_data_set_changed_and_update_internal_list branch from 1a4a0df to e33e5c7 Compare March 10, 2017 17:32
Copy link

@smarques84 smarques84 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The refreshDataSetInternal seems to have a bug, in the else if (mItemList.size() > getItemCount) i think you are supposed to remove items from the list not add them am I right?

My fix just flushes mItemList and repopulate with new data i did this because i have a custom filter option where i move and filter items on the fly on my recycler view i that case i think its best to just clear the list and repopulate

Or do you have another idea?

@dvdciri
Copy link
Owner Author

dvdciri commented Mar 13, 2017

@smarques84 Do the changes here if you can (not sure about the permissions) and add some test otherwise i'm not sure when i'll be ablet to pick this up.

@dvdciri dvdciri force-pushed the feature/#49_Handle_notify_data_set_changed_and_update_internal_list branch 3 times, most recently from 707662c to a2b60de Compare August 2, 2017 23:11
@dvdciri
Copy link
Owner Author

dvdciri commented Aug 2, 2017

I'm gonna resume this PR, i've added the implementation as you said for just refreshing the dataset with a custom notifyAdapterDataSetChanged() as i can't override the notifyDataSetChanged() one. Will provide some testing and will be ready in the next release.

@adevezin
Copy link

Can we get an official look at this so it can be merged?

@dvdciri dvdciri force-pushed the feature/#49_Handle_notify_data_set_changed_and_update_internal_list branch from b62f181 to 97adeef Compare March 24, 2018 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants