Skip to content

Conversation

@MrPaperMoon
Copy link

I finded problem, that after diffutils work, we just take new groups, but actually not all new groups will be settet do RecyclerView.
Major problem is that we unregister all old groups from adapter, but he can be still used by RecyclerView.
U can see it, if update items by method updateAsync and try to use notifyChanged.

@Zhuinden
Copy link
Collaborator

I feel like this might be a fix for #379 which would be amazing contribution.

I had an attempt at fixing it, but I realized it was not sufficient. This looks promising however. I will need to test it and potentially merge and release it, as it would be a massive help 🙏 🙇

@MrPaperMoon
Copy link
Author

“ after the second call” , “ observers list is empty ”
Y, looks like same issue

@cathalc
Copy link

cathalc commented Nov 15, 2021

@Zhuinden is there any chance this could be merged for a future update? Using groupie with view bindings is causing a couple of issues for us similar to #379 and #412

@Zhuinden
Copy link
Collaborator

Zhuinden commented Nov 15, 2021

@cathalc were you able to verify that this PR resolves the issues you encountered?

I did take a glance at it, I did not eventually reach a conclusion on whether it has any unintended side-effects (other than what's practically API-breaking-change on the change from Collection to List, although that's an easy solve if the API exposes Collection but is converted to List internally if needed).

But I did encounter that isSameAs in some spurious scenarios can cause unintended "erasure of the notifier callbacks" which is a major bug. I'm just uncertain...

@MrPaperMoon
Copy link
Author

@Zhuinden @cathalc

Actually, I found that my solution break work of framework main idea - nested groups. I compare collections flatten, that mean I ignore group as entity at all.
I try to remake it to groups comparing and implemented isSameAs and hasSameContentAs for Group, but I find to impossible describe isSameAs method for Group without any id.
In that case, I believe, we should work at same way, but not just merge items, but merge it into groups of new collection.

Idk due date for fix :(

Btw, I was change Collection to List bcs from outside it used only for List.
And I see that I have problem with tabulation, can u share rule for it?

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