Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IGListCollectionViewLayout partial optimization issue in certain situation #1275

Closed
2 tasks done
qhhonx opened this issue Nov 5, 2018 · 2 comments · Fixed by Lightricks/IGListKit#10
Closed
2 tasks done
Labels

Comments

@qhhonx
Copy link
Contributor

qhhonx commented Nov 5, 2018

New issue checklist

General information

  • IGListKit version: 3.4.0
  • iOS version(s): iOS12.1
  • CocoaPods/Carthage version: 1.4.0
  • Xcode version: 10.1
  • Devices/Simulators affected: Both
  • Reproducible in the demo project? (Yes/No): Yes
  • Related issues: No

Debug information

# Please include debug logs using the following lldb command:
po [IGListDebugger dump]

Description

If we use IGListBindingSectionController, and want to update first item of the section that both UI elements and cell size will be changed. Since IGListBindingSectionController can just reload cell not insert+delete, the updating cell will be invoked bindViewModel: again so that we can update its UI elements. However, we have no way to update the size of cell if using IGListCollectionViewLayout because of minimumInvalidatedSection not affected. And the performBatchUpdates: is not triggered by updateAnimated: in IGListBindingSectionController but performUpdatesAnimated: in IGListAdapter.

Below are related code snippets.

in IGListBindingSectionController.m
image
in IGListCollectionViewLayout.mm
image

@rnystrom
Copy link
Contributor

@xohozu is there a way to repro this in an example project so we can take a look?

Also you should be able to get around this by invalidating the layout on the collection context:

https://github.com/Instagram/IGListKit/blob/master/Source/IGListCollectionContext.h#L236-L247

@qhhonx
Copy link
Contributor Author

qhhonx commented Nov 27, 2018

ModelingAndBinding.zip

I uploaded a reproducible example project by reusing existing ModelingAndBinding demo.
In that project, some little changes as below:

  • Dynamic height for ActionCell according to its likes count.
// in `PostSectionController.swift` line48
case let viewModel as ActionViewModel: height = CGFloat(55 + 2 * (viewModel.likes / 10))
  • Use builtin IGListCollectionViewLayout by changing collectionView in Main.storyboard Layout to Custom and Class to IGListCollectionViewLayout.

Some observed results as follow:

  • When I tapped the heart icon, the numbers of likes updated and the size of ActionCell changed by using UICollectionViewFlowLayout.
  • However, if using IGListCollectionViewLayout, When I tapped the heart icon, the numbers of likes updated because of bindViewModel: called, but the size of ActionCell not affected because of
// in `IGListCollectionViewLayout.mm` line447

- (void)_calculateLayoutIfNeeded {
    if (_minimumInvalidatedSection == NSNotFound) {
        return;
    }
    ...
}

always return in advance due to optimization of IGListCollectionViewLayout.

  • Calling invalidateLayoutForSectionController:completion: before or in updateAnimated:completion:, all cells in that section will be re-calculate their sizes ignoring diffing results which mean only few cells should be re-calculate their sizes.

I have a straightforward solution(#1285) to fix this issue probably that those updating(not inserts or deletes) cells need to invalidate their attributes before re-binding new view model in updateAnimated:completion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants