Skip to content
This repository has been archived by the owner on Dec 5, 2022. It is now read-only.

UICollectionView.reload(changes: ...) section-level equivalent missing. #32

Open
jrogccom opened this issue Feb 28, 2019 · 9 comments
Open

Comments

@jrogccom
Copy link

Wondering what the impact is of replicating the reload logic from the UICollectionView+Extensions with the corresponding section-updating methods (reloadSections, deleteSections, insertSections, moveSections). I could submit a PR for this if this change hasn't been scoped out or hasn't been intentionally omitted.

@jrogccom jrogccom changed the title UICollectionView.reload(changes: ...) UICollectionView.reload(changes: ...) section-level equivalent missing. Feb 28, 2019
@snoozemoose
Copy link

@jrogccom I started doing this some time ago but never fully finished it and I'd be happy to help out with this task. I could probably brush up the work that I've already done this coming weekend to get a starting point. But if you want to do it yourself I'll understand :-)

@jrogccom
Copy link
Author

jrogccom commented Mar 1, 2019

@snoozemoose For my own purposes I just copied the existing reload methods and kind of swapped in the section methods. What you were doing is probably more thorough so we should probably work off that. :)

@snoozemoose
Copy link

snoozemoose commented Mar 1, 2019 via email

@snoozemoose
Copy link

snoozemoose commented Mar 3, 2019

@jrogccom I've spent a couple of hours on this today and I've got bad news, good news and some new code :-). Bad news first: if you implement the section logic in the same manner as the item logic is implemented today then you'll get exceptions thrown by the collection view if you do a section replace where the number of items in that section has changed. Good news is that you'll easily work around that by putting the outsideSectionsUpdate call inside the performBatchUpdates call. However, the animations in this case are so-so IMO since the cells will just cross fade into new cells.
So the new code is that I've built is a section replace logic that calls the reloadItems logic in turn (formerly known as just reload). I've also made it so that all animated updates are done in a single performBatchUpdates call so the animations are synch'ed and nice. The negative is that this code became slightly hard to read and understand. If you're interested I could tidy it up a bit and publish it on my fork.

@snoozemoose
Copy link

@jrogccom @onmyway133 If it's interresting, check out my code in branch section_awareness: https://github.com/snoozemoose/DeepDiff/tree/section_awareness. It does introduce a struct Section which feels a bit limiting for a general implementation so any input on the code would be much appreciated. I've modified the Demo project so the new code is tested by just running that. Of course, the previous Demo code needs to be reinstated but it's just for quick testing.

@jrogccom
Copy link
Author

jrogccom commented Mar 5, 2019

@snoozemoose Interesting about the number of items .. 🤔. I'll checkout your branch when I get the chance!

@onmyway133
Copy link
Owner

onmyway133 commented Mar 6, 2019

@jrogccom @snoozemoose thanks for the contributions to DeepDiff ❤️

For section diffing, the ideal scenario is to make this algorithm work on a matrix level (array of array). The solution that you did @snoozemoose is clever and that's what I intended to do when I started DeepDiff, it's simple and does the job. It misses "moving item across section" changes but I would say that is tricky to do.

Another thing is that, as I understand correctly, like in UITableView https://developer.apple.com/library/archive/documentation/UserExperience/Conceptual/TableView_iPhone/ManageInsertDeleteRow/ManageInsertDeleteRow.html, animation of items and sections function calls must be called in certain order for it to work properly, and sometimes trial and errors.

For what I'm doing now, a lazy solution, is to have just cells. I implement headers and footers as just normal cells so there is only 1 array of items.

@snoozemoose I would love you to submit a PR, with more tests so we can cover other common cases.

@snoozemoose
Copy link

snoozemoose commented Mar 14, 2019

@onmyway133 Your idea for a matrix level algorithm sounds very interesting and would be fun to dig into. "moving item across section" would be tricky to do in an efficient manner, to put it mildly :-), but that makes it such a fun challenge. Meanwhile, I'll work some more on the current section aware code and submit a PR when it's getting near ready. It might take a week or two before I get a chance to work on it but I'll get it done eventually :-)

@ton252
Copy link

ton252 commented Mar 16, 2019

@snoozemoose I saw your solution. Very interesting idea, but it not works in this case:

    // Random swap 2 sections
    let i1 = Int.random(in: 0..<1)
    let i2 = i1 == 0 ? 1 : 0

    let tmp = newSections[i1]
    newSections[i1] = newSections[i2]
    newSections[i2] = tmp

    // Change number of elements in section
 
    newSections[i1].items = DataSet.generateItems()

I think the problem is in diffId in Section class

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

No branches or pull requests

4 participants