search

jessesquires / ReactiveCollectionsKit

Data-driven, declarative, reactive, diffable collections (and lists!) for iOS. A modern, fast, and flexible library for UICollectionView done right.
https://jessesquires.github.io/ReactiveCollectionsKit/
MIT License
140 stars 6 forks source link

refactor: split steps with reconfigure views #144

Closed nuomi1 closed 2 weeks ago

nuomi1 commented 3 weeks ago

Have you read the Contributing Guidelines?

Issue #141

Describe your changes

Clearly and concisely describe what's in this pull request. Include screenshots, if necessary.

  1. Split steps for reconfiguring supplementary views.
jessesquires commented 2 weeks ago

thanks @nuomi1! can you explain this change a bit more?

I'm not convinced we should merge this change. Are there any benefits here?

The logic seems a little harder to follow to me.

nuomi1 commented 2 weeks ago

Similar to _findItemsToReconfigure, split _findViewsToReconfigure can be called separately to calculate the supplementary views that needs to be updated, and it is convenient for logger to log debug informations.

jessesquires commented 2 weeks ago

Similar to _findItemsToReconfigure, split _findViewsToReconfigure can be called separately to calculate the supplementary views that needs to be updated, and it is convenient for logger to log debug informations.

Ah ok, I see. So... I do appreciate the consistency. And I see the motivation for logging. Thanks for putting the effort and thought into this! 😄

However, the issue with this refactor is that we're now doing extra work. First, we loop through everything to find the views then we have to loop again to reconfigure them. It's more efficient to do it all in one pass.

The difference with items/cells is that we simply have to find the identifiers, and then UIKit handles the rest via destinationSnapshot.reconfigureItems(itemsToReconfigure). Unfortunately, UIKit does not provide a similar API for supplementary views. ☹️

So, I think for now we should not merge this change and keep our logging simple — just logging the view model before/after the changes and maybe some other lightweight info. But, let's discuss all the logging in #141.