Instagram / IGListKit

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

[Example project] Self-sizing cells crash bug #260

Open PhilCai1993 opened 7 years ago

PhilCai1993 commented 7 years ago

Self-sizing cells crash bug

General information

image

image

PhilCai1993 commented 7 years ago

After checking StackOverFlow This can be solved

final class ManuallySelfSizingCell/FullWidthSelfSizingCell/NibSelfSizingCell: UICollectionViewCell {
      var hasCalculated = false
      ......
      override func preferredLayoutAttributesFitting(_ layoutAttributes: UICollectionViewLayoutAttributes) -> UICollectionViewLayoutAttributes {
        if hasCalculated {
          return layoutAttributes
        }
        hasCalculated = true
        setNeedsLayout()
        layoutIfNeeded()
        let size = contentView.systemLayoutSizeFitting(layoutAttributes.size)
        var newFrame = layoutAttributes.frame
        // note: don't change the width
        newFrame.size.height = ceil(size.height)
        layoutAttributes.frame = newFrame
        return layoutAttributes
    }
}

This can be solved on iOS 9. But on iOS 10 the layout becomes strange. I don't know what about iOS 8 or even iOS 7. Anyway, this is not a bug in IGListKit. @rnystrom

jessesquires commented 7 years ago

Thanks for the update @PhilCai1993 ! 😄

jeffbailey commented 7 years ago

I'm currently using self sizing cells with IGListKit in a client app and had this crash. I tracked it down to unneeded calls to:

setNeedsLayout() layoutIfNeeded()

in preferredLayoutAttributesFitting. I wasn't able to reproduce the issue in the demo app, but you might want to try removing those methods.

I think it makes sense that these calls aren't needed by systemLayoutSizeFitting to calculate the size.

rnystrom commented 7 years ago

@jeffbailey good to know. I should try this out on iOS 9/10, maybe even put a version check in there if the behavior changes. Did you hit this crash on 10?

FYI I pasted the solution from http://stackoverflow.com/a/25896386/940936 and removed the caching bit that @PhilCai1993 called out as fixing iOS 9 b/c I ran into some weird behavior on 10.

jeffbailey commented 7 years ago

Yes, the crash was on 10, but went away as soon as I removed those lines. I believe it had to do with those methods being called on cells that were about to be reused, but that's just a guess.

rnystrom commented 7 years ago

@jeffbailey if you run into it again or are able to make the example project do this I'd love to take a deeper look. I'll play around w/ different setups to see what works best here in the meantime.

ArthurChi commented 7 years ago

How to solve this crash on 10?

rnystrom commented 7 years ago

@sun-fox-cj have you given @jeffbailey's solution a try?

ArthurChi commented 7 years ago

I have solved this problem without version check. Before I fix this crash, I did it as follows:

  1. First, add a width constraint equal to screen's width on contentView,
  2. and then fill those UIControls which in contentView with the model's data.
  3. Next step is that invoke layoutIfNeeded and systemLayoutSizeFitting method to calculate the height of content view, then cache it.

It works well before iOS10, but crashes in iOS10.

So I change it like this: I add inner UIControls constraint with contentView to certain the width of contentView instead of add a width constraint directly. It works well, again.

shuhrat10 commented 7 years ago

@rnystrom I'm getting a jerky animation when I'm using the searchBar and self-sizing cells. Have you seen this before? It's happening on iOS 10

shuhrat10 commented 7 years ago

I'm still getting crash on iOS 9.3 in Example iOS App (Self-sizing cells).