Skip to content

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

Open
dvdciri wants to merge 1 commit into
developfrom
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 into
developfrom
feature/#49_Handle_notify_data_set_changed_and_update_internal_list

Conversation

@dvdciri

@dvdciri dvdciri commented Mar 10, 2017

Copy link
Copy Markdown
Owner

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

@smarques84 smarques84 left a comment

Copy link
Copy Markdown

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

dvdciri commented Mar 13, 2017

Copy link
Copy Markdown
Owner Author

@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

dvdciri commented Aug 2, 2017

Copy link
Copy Markdown
Owner Author

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
Copy Markdown

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