Iglistkit: [Automatic cell sizing] Master task for auto-sizing cells / estimated cell sizing and auto-sizing supplementary views

Created on 27 Feb 2017  ·  48Comments  ·  Source: Instagram/IGListKit

There are known issues with auto-sizing / auto-layout / estimated size cells in IGListKit.

A number of issues have been opened:

487, #508, #266, #463, #452, #588, #739, #864, #906

We'll track and discuss all auto-sizing bugs/problems in this task to consolidate the discussion.


If any new issues are opened regarding this, please close and redirect here. 😄

bug help wanted question

Most helpful comment

Hey guys, I guess, I've found a way how to make self-sizing work:
1) In viewDidLoad():

       if let collectionViewFlowLayout = collectionView.collectionViewLayout as? UICollectionViewFlowLayout {
            collectionViewFlowLayout.estimatedItemSize = UICollectionViewFlowLayoutAutomaticSize 
        }

2) In your cell class:

  override func preferredLayoutAttributesFitting(_ layoutAttributes: UICollectionViewLayoutAttributes) -> UICollectionViewLayoutAttributes {
        setNeedsLayout()
        layoutIfNeeded()

        let size = contentView.systemLayoutSizeFitting(CGSize(width:  layoutAttributes.frame.width,
                                                              height: CGFloat.greatestFiniteMagnitude),
                                                       withHorizontalFittingPriority: UILayoutPriorityRequired, verticalFittingPriority: UILayoutPriorityFittingSizeLevel)
        layoutAttributes.frame.size = size
        return layoutAttributes
    }

All 48 comments

This bug hit me hard today.

Commit on Example project to repro bug: https://github.com/dhavalcue/IGListKit/commit/c4f191f0fe7bf0ca66a536f415dbbbb93f47fac2
Video of bug: Video

My repro is only for self-sizing NIB cells.

Two issues:
1) Cells are centered on screen. Workaround is to setup a constraint to set the width to size of screen.

2) While performing updates, the cells immediately size themselves to the size as returned by the section controller and then animate to expected size. See Video
I don't know of any reasonable workarounds for this.

@jessesquires: Is this something you guys expect to look into or is it not a priority for the team right now?

Thanks team, ❤️️ IGListKit!

I tried the same without IGListKit, standard UICollectionView and UICollectionViewFlowLayout. The bug still exists there. This one is not on IGListKit.

https://github.com/dhavalcue/UICollectionViewSelfSizingCellsBug

Is it possible to have a setup which combines self-sizing and fixed size cells within the same collection view/layout? Can only get either to work properly, not both together.

EDIT: err, nevermind. Fixed with some AutoLayout magic.

Is this something you guys expect to look into or is it not a priority for the team right now?

This isn't a huge priority for us, as we don't use this in IG. But, we would like to support it.

This bug is a problem, our entire project uses variable height cells! We will take a look later.
Any idea?

Guys, I'm struggling for few days with self sizing items in collectionview. I used :

  • The technique of estimatedSize + preferredMaxWidth label + contentView width constraint + contentView translatesMaskIntoConstraint false, but it crashed on iPhone Plus models iOS 10

  • The technique of estimatedSize + preferredLayoutAttributesFitting + systemLayoutSizeFitting compression

  • The technique of estimatedSize + systemLayoutSizeFitting:withHorizontalFittingPriority:verticalFittingPriority it works on iPhone Plus but it doesn't work when change iPhone model to iPhone 5 and 6 doesn't have the good width.

I read a ton of articles and nothing seem to be working perfectly. I even tried the fake sizing cell, but it's really low performance, you can see it at scroll even on an iPhone 7 Plus.

I was considering switching to IGListKit but I see that this edge case is not handled neither. For me I have two options : Either I go back to TableView :'( Either like in JSQMessagesViewController or IGListKit raywenderlich tutorial where he computes the height of the cell in helper in the cell.

Guys, you who have experience with dynamic cell height, what would you recommend ?

Similar problems here. At the moment I am waiting.

I couldn't get dynamic heights to work on my end. My workaround is to use systemLayoutSizeFitting and cache the size. For this to work I require all self-sizing cells to implement the SelfSizingCell protocol, which takes in an item which implements the IGListDiffable protocol.

The whole thing to make this work is here: https://gist.github.com/dhavalcue/4b081e65ded5c261d301beddbd2ddfcb

@LivioGama Instagram is almost entirely dynamic cell sizes, its just that we don't use any of the UIKit estimated cell sizing b/c of how buggy they can be (evidenced by this issue). Definitely want to find a good recommended way to do self-sizing cells, but it's really low priority for us since we don't use it and honestly don't really recommend it (perf, bugs, etc).

Also as @dhavalcue pointed out, this bug happens outside of IGListKit, maybe our best approach would be to file and dupe some radars?

This is kinda a hacky approach, but my “solution” to this is to just handle supplementary views as regular cells instead.

My method:

  • Tweak the sectionController/add a new one to handle headers/footers/etc.
  • Set an estimatedItemSize for the collectionView’s layout
  • Return a size with the required cell width and some random height value in sizeForItem
  • Use the default preferredLayoutAttributesFitting implementation as shown in the examples

This seems to work well for me. Getting the contentView to return the correct size in preferredLayoutAttributesFitting gets a bit tricky sometimes, but here’s the golden rule I follow: Never touch it other than to add subviews or constraint subviews to it. Also, if you see some overlapping in cells, set the bogus height in sizeForItem to some massive value. Dunno why, but it seems to work.

@rnystrom Hello thanks for your reply. I'm really happy about your recommendation, because I'm not stubborn to use absolutely estimatedCellSize. I just want something that work 100%. So in Instagram you do static function in each cells that calculate the height according to the text ? Something like this https://gist.github.com/gnou/52bd2f31e99c9311ffa0eb0183ac7cfd

@LivioGama almost exactly that 😄

@rnystrom Is there any sample code where I could see this library and calculating the height manually?

@LivioGama Do you have a sample project with your code? Thanks a lot.

BTW, I tried to open the sample code, but it does not compile. I opened a bug here:
https://github.com/Instagram/IGListKit/issues/635

@Ricardo1980: Did you see my comment here: https://github.com/Instagram/IGListKit/issues/516#issuecomment-290156370

@dhavalcue I tried that (without the caching). I mean, I tried to get the cell inside sizeForItem calling cellForItem and them apply something like:

    let cell = FeedCommentCell(frame: .zero)
    cell.fillWithData()
    cell.contentView.setNeedsLayout()
    cell.contentView.layoutIfNeeded()
    let maxSize = CGSize(width: context.containerSize.width, height: 0)
    let properSize = cell.contentView.systemLayoutSizeFitting(maxSize, withHorizontalFittingPriority: UILayoutPriorityRequired, verticalFittingPriority: UILayoutPriorityFittingSizeLevel)
    print(properSize.height)
    return properSize

in order to return that size for that cell.
Is that the main idea? It seems it is working but I see a lot of autolayout conflicts in the console.
I have setup the cell with a mutiple line UILabel pin to the super view (contentView), top, left, right, bottom. Is that right? I guess that should be required for systemLayoutSizeFitting.

Any clue about this? Any recommendation is welcome.
How do you do that part?
Thanks a lot.

I forgot to say my app should work under iOS 9, maybe that is important.

BTW, when UILabel is involved, do I have to use preferredMaxLayoutWidth always? I don't see the reason if constraints are used properly.

@Ricardo1980 i do same like yours, but my first item always got wrong height.
Did you find any solution already ?

👆👆👆👆

It's working fine for me.
I instantiate another cell, I apply autolayout using the context width, I take the height and I return that number in the size method.
Not very nice but it is working perfectly for me in all cases.

@Ricardo1980 ,
i was not using auto layout, but solved it already. Thanks btw.

Does IG recommend using autolayout to configure the things around the comments that don't "really" need to be the dynamic height or is just using CG rect frames the way to go about this?

@jboo1212 we definitely don't for complicated layouts that need to be performant. We don't actually have a single line of Auto Layout code in IG, but that's mostly for consistency. AL is a great framework when used in moderation, and away from perf-tight situations.

For top-notch scroll perf, nothing beats plain CGRect math. But using something like Yoga to compute layouts on a background queue and then mount them is also possible, but can get pretty complicated.

Anyone else here having issues with using multiple section controlle and for one use a supplementary view header? For some reason when I use prefferedLayoutAttributesFitting to return the real cell size the header is staying at the old position before the cells got resize to fit its content. anyone seen this before?

Hey guys, I guess, I've found a way how to make self-sizing work:
1) In viewDidLoad():

       if let collectionViewFlowLayout = collectionView.collectionViewLayout as? UICollectionViewFlowLayout {
            collectionViewFlowLayout.estimatedItemSize = UICollectionViewFlowLayoutAutomaticSize 
        }

2) In your cell class:

  override func preferredLayoutAttributesFitting(_ layoutAttributes: UICollectionViewLayoutAttributes) -> UICollectionViewLayoutAttributes {
        setNeedsLayout()
        layoutIfNeeded()

        let size = contentView.systemLayoutSizeFitting(CGSize(width:  layoutAttributes.frame.width,
                                                              height: CGFloat.greatestFiniteMagnitude),
                                                       withHorizontalFittingPriority: UILayoutPriorityRequired, verticalFittingPriority: UILayoutPriorityFittingSizeLevel)
        layoutAttributes.frame.size = size
        return layoutAttributes
    }

Thank you @artemkalinovsky! I will try this out. What's the difference between the systemLayoutSizeFitting with the extra parameters and the plain contentView.systemLayoutSizeFitting? It's unclear for me when reading the docs

@weyert func systemLayoutSizeFitting(_ targetSize: CGSize) -> CGSize give me wrong size (width and height), so I've used method, where I can set correct width by myself:

func systemLayoutSizeFitting(_ targetSize: CGSize, 
withHorizontalFittingPriority horizontalFittingPriority: UILayoutPriority, 
     verticalFittingPriority: UILayoutPriority) -> CGSize

where I've said that I want cell with fixed width and flexible height value.

@artemkalinovsky Thanks, it work for me. But there are two problems.

  1. UICollectionViewFlowLayoutAutomaticSize NS_AVAILABLE_IOS(10_0)
  2. I still get warnings about two extra NSAutoresizingMaskLayoutConstraint - width and height set in section controller

Any thoughts?

So, how I solved this problem in Objective-C:

  1. Remove "preferredLayoutAttributesFitting" method.
  2. Add any non CGSizeZero "estimatedItemSize", for example:
UICollectionViewFlowLayout *layout = [[UICollectionViewFlowLayout alloc] init];
layout.estimatedItemSize = CGSizeMake(44.0f, 44.0f);
  1. Set content view "translatesAutoresizingMaskIntoConstraints" to false.
- (instancetype)initWithFrame:(CGRect)frame {
    if (self = [super initWithFrame:frame]) {
        self.contentView.translatesAutoresizingMaskIntoConstraints = NO;
    }
    return self;
}
  1. Set constraints for your custom view, label, etc. in cell.
  2. Return size in ListSectionController, for example:
- (CGSize)sizeForItemAtIndex:(NSInteger)index {
    return CGSizeMake(self.collectionContext.containerSize.width, 44.0f);
}

@agsarkisov
Our app is targeting iOS10 and above, so we don't care about UICollectionViewFlowLayoutAutomaticSize NS_AVAILABLE_IOS(10_0)

¯_(ツ)_/¯

@rnystrom What are you guys using for this below image at instagram? Tableview? I need dynamic height for cells with textView embedded, preferably in collectionviewcell.

My bet would be UITableViewController to not have to handle the scroll and keyboard :D

Then, what is the best way to do this?

Wait, does UITableViewController manage the keyboard automatically?

IMO, it is the only advantage of using UITableViewController :D

@artemkalinovsky Did you manage to get your solution working when using adapter.performUpdates(animated: true, completion: nil)? I'm getting the correct size but the same bug as in this video when refreshing data.

@artemkalinovsky

Hey guys, I guess, I've found a way how to make self-sizing work:

Not working for me, it just continues to get cell height from sizeForItem(index:) from sectionController. My layout.estimatedItemSize is set UICollectionViewFlowLayoutAutomaticSize, code provided by you is inside Cell class:

override func preferredLayoutAttributesFitting(_ layoutAttributes: UICollectionViewLayoutAttributes) -> UICollectionViewLayoutAttributes {
        setNeedsLayout()
        layoutIfNeeded()

        let size = contentView.systemLayoutSizeFitting(CGSize(width:  layoutAttributes.frame.width,
                                                              height: CGFloat.greatestFiniteMagnitude),
                                                       withHorizontalFittingPriority: UILayoutPriorityRequired, verticalFittingPriority: UILayoutPriorityFittingSizeLevel)
        layoutAttributes.frame.size = size
        return layoutAttributes
}

It returns correct size for cell, but ListKit is using size from sizeForItem(index:)

P.S. IGListKit version: 3.3.0

@artemkalinovsky @Satchitananda while the proposed method using preferredLayoutAttributesFitting does the job, it's especially low-performance, take a look at the Advanced AutoLayout WWDC 2018 talk.

A new instance of the AutoLayout engine is being created each time the method is invoked and then being discarded after just calculating the sizes. It's extremely costly when working with scrollable lists which essentially need to be high-performance.

My impression is that the whole "Self Sizing Cells" feature was hacked overnight, as it fails to deliver layout of complexity higher than at WWDC demos.
I highly don't recommend using it anywhere beyond prototypes, since it lacks stability and acts like a broken black box.

Unfortunately, you don't know at which stage it will fail for you. In my case I had quite complex UI with supplementary / separator views and custom layout. The "Self Sizing Cells" approach is not only slow, but inconsistent: elements are often misplaced, which is solved after scrolling up and down.

For now I went with the approach of using "dummy" cells for sizing, and a separate object for caching the size. While it feels like a "hack", this hack works better than the multitude of 🍎's hacks in the Self Sizing Cells technology.

Needless to say, that the performance of lists jumped up tremendously after adding the size cache.

I believe IGListCollectionViewLayout needs to implement a couple override methods in order to support self-sizing cells. Is there a sample project I can try my approach with?

I believe it should work like this:

  1. A client sets estimatedItemSize on the layout
  1. Right before the cell is put on screen, UICollectionView calls:
    - (UICollectionViewLayoutAttributes *)preferredLayoutAttributesFittingAttributes:(UICollectionViewLayoutAttributes *)layoutAttributes on the cell.
  • If the cell doesn't override this method then Auto Layout should automatically try to size the cell using constraints (but I've heard there are bugs in this system today, so probably best to override this method and call the AL sizing methods yourself).

    1. IGListCollectionViewLayout will get a chance to see the new layoutAttributes and decide whether to invalidate some part of the layout. This method isn't implemented today!
      - (BOOL)shouldInvalidateLayoutForPreferredLayoutAttributes:(UICollectionViewLayoutAttributes *)preferredAttributes withOriginalAttributes:(UICollectionViewLayoutAttributes *)originalAttributes
  1. If IGListCollectionViewLayout returns YES in step 3, then it gets a chance to specify the invalidated region. That is with this method:
  • - (UICollectionViewLayoutInvalidationContext *)invalidationContextForPreferredLayoutAttributes:(UICollectionViewLayoutAttributes *)preferredAttributes withOriginalAttributes:(UICollectionViewLayoutAttributes *)originalAttributes

  • It should return an invalidation for the cells below the one that just changed.

Hopefully that's it!

Hi @bridger, please, take a look at my previous post regarding Self Sizing cells and specifically calling - (UICollectionViewLayoutAttributes *)preferredLayoutAttributesFittingAttributes:(UICollectionViewLayoutAttributes *)layoutAttributes.

You're correct about numerous bugs in the system. What's more important, the consequence of this approach is very low performance and janky scrolling experience.

I recommend not using Self Sizing cells for any configuration beyond "tag cloud", where the size could be easily determined.

If you have to resort to preferredLayoutAttributesFittingAttributes, you're already fixing problems Apple should fix.

image

My impression is that the whole "Self Sizing Cells" feature was hacked overnight, as it fails to deliver layout of complexity higher than at WWDC demos.
I highly don't recommend using it anywhere beyond prototypes, since it lacks stability and acts like a broken black box.

Unfortunately, you don't know at which stage it will fail for you. In my case I had quite complex UI with supplementary / separator views and custom layout. The "Self Sizing Cells" approach is not only slow, but inconsistent: elements are often misplaced, which is solved after scrolling up and down.

For now I went with the approach of using "dummy" cells for sizing, and a separate object for caching the size. While it feels like a "hack", this hack works better than the multitude of 🍎's hacks in the Self Sizing Cells technology.

Needless to say, that the performance of lists jumped up tremendously after adding the size cache.

@richardtop In my situation, I have a "dummy cells for sizing" solution.
Every section controller hold a sizing cell to calculate cell height,

@implementation QSUserSectionController

- (CGSize)sizeForItemAtIndex:(NSInteger)index {
    _sizeCell.userTextLabel.text = self.user.nickname;
    _sizeCell.subTitleLabel.text = [self.feed.createAt timeAgoSinceNow] ?: self.user.bio;
    _sizeCell.contentInset = self.contentInset;
    CGSize size = [_sizeCell sizeThatFits:CGSizeMake(SCREEN_WIDTH, CGFLOAT_MAX)];    
    return size;
}

@end

@implementation QSUserCell

- (CGSize)sizeThatFits:(CGSize)size {
    UIEdgeInsets contentInset = UIEdgeInsetsEqualToEdgeInsets(self.contentInset, UIEdgeInsetsZero) ? kInsets : self.contentInset;
    CGSize resultSize = CGSizeMake(size.width, 0);
    CGFloat contentLabelWidth = size.width - UIEdgeInsetsGetHorizontalValue(contentInset) - self.followButton.qmui_left - kMargin;

    CGFloat resultHeight = CGRectGetHeight(self.avatarImageView.bounds);

    CGSize contentSize1 = CGSizeZero;
    CGSize contentSize2 = CGSizeZero;

    if (self.userTextLabel.text.length > 0) {
        contentSize1 = [self.userTextLabel sizeThatFits:CGSizeMake(contentLabelWidth, CGFLOAT_MAX)];
        resultHeight = MAX(resultHeight, contentSize1.height);
    }

    if (self.subTitleLabel.text.length > 0) {
        contentSize2 = [self.subTitleLabel sizeThatFits:CGSizeMake(contentLabelWidth, CGFLOAT_MAX)];
        resultHeight = MAX(resultHeight, contentSize1.height + contentSize2.height);
    }

    resultSize.height = UIEdgeInsetsGetVerticalValue(contentInset) + resultHeight;
    return resultSize;
} 

@end

My problem is hight memory using when there is a large number of sections scrolling, could you give some suggestions?

@frankxzx I think, you'll need to share a sizing cell between multiple SectionControllers of the same kind.

It doesn't make a lot of sense to create a sizing cell per section controller, as you end up with a sizing cell for each controller + on-screen cells.

Naturally, in that case, your memory usage would be extremely high and you hardly ever get any performance improvement.

Hi @joseph-francis , Did this comment resolve? cc @rnystrom
https://github.com/Instagram/IGListKit/issues/516#issuecomment-372064517

@SoyaTakahashi Indeed. You can close it :)

Somebody find any solutions for a cell with UITextView autosizing?

@amarunko

487

Somebody find any solutions for a cell with UITextView autosizing?

I haven't not found any solutions yet.
So, It seems good to use UITableView!

What do you think everyone?

fyi @weyert

906

https://github.com/Instagram/IGListKit/issues/516#issuecomment-323556561

This problem may be solved in the following way.

If the UIView has collectionView
```~View.swift
override func layoutSubviews() {
super.layoutSubviews()

collectionView.collectionViewLayout.invalidateLayout()

}


If the UIViewController has collectionVIew
```~ViewController.swift
override func viewWillLayoutSubviews() {
    super.viewWillLayoutSubviews()

    collectionView.collectionViewLayout.invalidateLayout()
}

I hope I can be of any help to you.

I have been looking at this problem a lot longer than I expected. Is it safe to assume the following is true?

  • IGListKit is not made for or intended to be used for resizing cells?
  • Once a cells size is calculated it is cached and reused? I am seeing preferredLayoutAttributesFitting continually called when scrolling but the results not used. I am seeing sizeForItem in my ListSectionController only called when cell is first displayed.
  • If I want resizing cells I should look at a different technology

I have read through everything I could find about this topic and IGListKit, Sorry if some of these above questions have been answered in part else where but I need to know before investing a lot of time in a different technology. Any help greatly appreciated.

For those who is looking for dynamic resizing of UITextView during user input, here is my approach:

  • Pass initial text for UITextView to section controller.
  • Calculate text size manually using boundingRect(with:options:attributes:context:).
  • Subscribe section controller for UITextView delegation. On textViewDidChange:

    • Update text you are using to calculate cell size .

    • Call collectionContext?.invalidateLayout(for: self)

    • Pass new text to your ViewController / Presenter / other business logic handler.

    • Don't call adapter.performUpdates, since it stops text editing.

  • End editing on collection view scroll.

Example of screen with three UITextViews inside UICollectionViewCell each. Works perfect with animation.

Снимок экрана 2020-04-29 в 07 41 29

@joseph-francis I know I'm a little bit late, but still :)

In case anybody is looking to dynamically set the height for a cell containing UILabels, the solution below is simple and works quite well.

Bonus, it doesn't depend on Auto Layout (you can avoid preferredLayoutAttributesFitting).

In the relevant ListSectionController:

private var model: MyModel! // where MyModel.someValue is of type String

override func sizeForItem(at index: Int) -> CGSize {
   // Define fixed width
   let width = collectionContext!.containerSize.width
   // Get NSString
   let labelString = model.someValue as NSString
   // Compute rect for string
   let dynamicRect = labelString.boundingRect(
      with: CGSize(width: width, height: .greatestFiniteMagnitude), // use fixed width from above
      options: .usesLineFragmentOrigin, // most accurate display option for multi-line labels
      attributes: [NSAttributedString.Key.font: MyCell.labelFont], // use exact font that will be used in the label
      context: nil
   )
return CGSize(width: width, height: dynamicRect.height)
}

Notes:

  • UILabel numberOfLines should be 0
  • For multiple labels, compute boundingRect for each and add the heights.
Was this page helpful?
0 / 5 - 0 ratings

Related issues

iwasrobbed picture iwasrobbed  ·  3Comments

Przemyslaw-Wosko picture Przemyslaw-Wosko  ·  3Comments

shuhrat10 picture shuhrat10  ·  3Comments

kleiberjp picture kleiberjp  ·  3Comments

alexwillrock picture alexwillrock  ·  3Comments