Instagram / IGListKit

A data-driven UICollectionView framework for building fast and flexible lists.
https://instagram.github.io/IGListKit/
MIT License
12.87k stars 1.54k forks source link

Does not work with collection view self-sizing cells (.estimatedItemSize) #80

Closed sunbohong closed 7 years ago

sunbohong commented 8 years ago

I was trying to remove layout logic from view to SectionController, and i add some layoutConstraints to cell's subviews and estimatedItemSize to UICollectionViewFlowLayout

But IGListKit does not work on auto-size.iOS will log craze.

    static var flowLayout = { () -> UICollectionViewFlowLayout in
        var layout = UICollectionViewFlowLayout()
            layout.estimatedItemSize = CGSize(width: 300, height: 300)
        return layout
    }()
sunbohong commented 8 years ago

You can see this when you select Mixed Data cell.

jessesquires commented 8 years ago

Thanks @sunbohong ! 😄

It's hard to say if this is actually a ListKit bug or not.

  1. What errors do you see? What happens?
  2. Can you reproduce this in the sample projects?
rnystrom commented 8 years ago

Sample project would be helpful. We don't use the estimated sizing methods at all, and IGListAdapter implements the UICollectionViewFlowLayoutDelegate methods for sizing. I'm not sure how all those play together.

sunbohong commented 8 years ago

@jessesquires @rnystrom

https://github.com/sunbohong/IGListKit/commit/4cf113c39ef5f1ffd75a570cf86716c8efc79db0

This commit is the sample project.

When you select Mixed Data cell,it will log follow meassage infinitely.

2016-10-19 10:14:58.801 IGListKitExamples[41495:832483] Please check the values returned by the delegate.
2016-10-19 10:14:58.801 IGListKitExamples[41495:832483] The relevant UICollectionViewFlowLayout instance is <UICollectionViewFlowLayout: 0x7ff2ad917eb0>, and it is attached to <IGListCollectionView: 0x7ff2ab823800; baseClass = UICollectionView; frame = (0 0; 320 568); clipsToBounds = YES; gestureRecognizers = <NSArray: 0x60000005d0d0>; layer = <CALayer: 0x600000034760>; contentOffset: {0, -64}; contentSize: {320, 202}> collection view layout: <UICollectionViewFlowLayout: 0x7ff2ad917eb0>.
2016-10-19 10:14:58.801 IGListKitExamples[41495:832483] Make a symbolic breakpoint at UICollectionViewFlowLayoutBreakForInvalidSizes to catch this in the debugger.
2016-10-19 10:14:58.801 IGListKitExamples[41495:832483] The behavior of the UICollectionViewFlowLayout is not defined because:
2016-10-19 10:14:58.802 IGListKitExamples[41495:832483] the item width must be less than the width of the UICollectionView minus the section insets left and right values, minus the content insets left and right values.
rnystrom commented 8 years ago

@sunbohong ok I think I figured out what is going wrong here, to use estimated sizing cells you need to implement preferredLayoutAttributesFitting(...) in your custom cell when not using auto layout to do the sizing. I added this line to CollectionCell.swift and it works:

override func preferredLayoutAttributesFitting(_ layoutAttributes: UICollectionViewLayoutAttributes) 
    -> UICollectionViewLayoutAttributes {
  return layoutAttributes
}

Can you give this a try? If this works for you, can you close this issue. I'll open a new issue on how to use IGListKit with estimated sizing cells.

sunbohong commented 8 years ago

@rnystrom

if we return layoutAttributes directyly,the crazy log will disappear;but,the layout will incorrect.

    override func preferredLayoutAttributesFitting(_ layoutAttributes: UICollectionViewLayoutAttributes) -> UICollectionViewLayoutAttributes {
        return layoutAttributes;
    }

if we use super's preferredLayoutAttributesFitting method,the crazy log will not disappear.

    override func preferredLayoutAttributesFitting(_ layoutAttributes: UICollectionViewLayoutAttributes) -> UICollectionViewLayoutAttributes {
        let attributes = super.preferredLayoutAttributesFitting(layoutAttributes);
        return attributes;
    }
sunbohong commented 8 years ago

@rnystrom i add one unsafe Constraint to cell.contentView,the crazy log will disappear;but,the layout will incorrect when we select the CollectionCell.

        cell.contentView.snp.makeConstraints({ (make) in
            make.width.lessThanOrEqualTo(collectionContext!.containerSize.width)
        })
rnystrom commented 8 years ago

@sunbohong I think we need to find an example of a project using:

I'm honestly just not all that familiar w/ the estimated cell sizing and how the estimatedSize property plays with the layout delegate values. As I understand it the value returned by the delegate should be the true value (fully sized).

Have you used this API before? Do you have an example of how this is supposed to work when setting up a UICollectionView from scratch?

sunbohong commented 8 years ago

@rnystrom

I have upload a new demo.

https://github.com/sunbohong/Demo-Collection/commit/e2273de157e45d9d05250390b95b34cce52b3a17

saucym commented 8 years ago

@rnystrom if we use self-sizing and call performUpdates the Animation is not perfect. this function is not Optional. -> public func sizeForItem(at index: Int) -> CGSize Is there any way to perfect support self-sizing? Especially the animation.

let collectionView : IGListCollectionView = { let layout = UICollectionViewFlowLayout() if #available(iOS 10.0, *) { layout.estimatedItemSize = UICollectionViewFlowLayoutAutomaticSize } else { layout.estimatedItemSize = CGSize(width: 75, height: 75) } let cv = IGListCollectionView(frame: CGRect.zero, collectionViewLayout: layout) return cv; }()

override func preferredLayoutAttributesFitting(_ layoutAttributes: UICollectionViewLayoutAttributes) -> UICollectionViewLayoutAttributes { let attributes = super.preferredLayoutAttributesFitting(layoutAttributes) attributes.size.width = 100; attributes.size.height = 40; return attributes; }

testaa

rnystrom commented 8 years ago

@saucym so to use estimatedSize cells we can't implement collectionView:layout: sizeForItemAtIndexPath:?

jessesquires commented 8 years ago

@rnystrom - that doesn't sound correct. Layout should still query this method to get the final, actual size... (i think)

Or maybe is does just use preferredLayoutAttributesFitting:... 🤔

zhubofei commented 8 years ago

@sunbohong To useestimatedSize you need to override the frame size in preferredLayoutAttributesFittingAttributes, like this post http://stackoverflow.com/a/25896386/2977647 did.

zhubofei commented 8 years ago

@jessesquires After reading https://developer.apple.com/reference/uikit/uicollectionviewflowlayout/1617709-estimateditemsize?language=objc, I think if we set estimatedSize to a non-zero value, the layout will only query preferredLayoutAttributesFittingAttributes for item size.