-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[Automatic cell sizing] Master task for auto-sizing cells / estimated cell sizing and auto-sizing supplementary views #516
Comments
This bug hit me hard today. Commit on Example project to repro bug: https://github.com/dhavalcue/IGListKit/commit/c4f191f0fe7bf0ca66a536f415dbbbb93f47fac2 My repro is only for self-sizing NIB cells. Two issues:
@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. |
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. |
Guys, I'm struggling for few days with self sizing items in collectionview. I used :
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 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:
This seems to work well for me. Getting the |
@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: |
@Ricardo1980: Did you see my comment here: #516 (comment) |
@dhavalcue I tried that (without the caching). I mean, I tried to get the cell inside sizeForItem calling cellForItem and them apply something like:
in order to return that size for that cell. Any clue about this? Any recommendation is welcome. 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. 👆👆👆👆 |
It's working fine for me. |
@Ricardo1980 , |
@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 |
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:
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 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. 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:
Hopefully that's it! |
Hi @bridger, please, take a look at my previous post regarding Self Sizing cells and specifically calling 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 |
@richardtop In my situation, I have a "dummy cells for sizing" solution.
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 |
@SoyaTakahashi Indeed. You can close it :) |
Somebody find any solutions for a cell with UITextView autosizing? |
fyi @weyert This problem may be solved in the following way. If the UIView has collectionView override func layoutSubviews() {
super.layoutSubviews()
collectionView.collectionViewLayout.invalidateLayout()
} If the UIViewController has collectionVIew 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?
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
Example of screen with three @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 In the relevant 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:
|
Is there any sample code for implementing textview dynamic height while user input on keyboard? I dont see any example for IGListKit. I am trying to achieve similar to this #516 (comment) |
If you need dynamic height for cells with PublishViewController.swiftimport IGListKit
class PublishViewController: UIViewController {
private lazy var collectionView: UICollectionView = {
let flowLayout = UICollectionViewFlowLayout()
// setting a non-zero size enables cells that self-size via -preferredLayoutAttributesFittingAttributes:
flowLayout.estimatedItemSize = UICollectionViewFlowLayout.automaticSize
let collectionView = UICollectionView(frame: .zero, collectionViewLayout: flowLayout)
collectionView.autoresizingMask = [.flexibleHeight, .flexibleWidth]
collectionView.backgroundColor = .systemGroupedBackground
return collectionView
}()
private lazy var adapter: ListAdapter = {
let updater = ListAdapterUpdater()
return ListAdapter(updater: updater, viewController: self, workingRangeSize: 1)
}()
override func viewDidLoad() {
super.viewDidLoad()
navigationItem.leftBarButtonItem = UIBarButtonItem(barButtonSystemItem: .close, target: self, action: #selector(navigationLeftBarButtonTapped))
view.addSubview(collectionView)
adapter.collectionView = collectionView
adapter.dataSource = self
}
override func viewDidLayoutSubviews() {
super.viewDidLayoutSubviews()
collectionView.frame = view.bounds
}
// MARK: - Actions
@objc func navigationLeftBarButtonTapped() {
self.dismiss(animated: true)
}
}
extension PublishViewController: ListAdapterDataSource {
func objects(for listAdapter: ListAdapter) -> [ListDiffable] {
return [PublishTitleViewModel(title: "Title", iconImage: nil, placeholder: "placeholder", lengthLimit: 100)]
}
func listAdapter(_ listAdapter: ListAdapter, sectionControllerFor object: Any) -> ListSectionController {
return TitleSectionController()
}
func emptyView(for listAdapter: ListAdapter) -> UIView? {
return nil
}
} TitleSectionController.swiftimport IGListKit
class TitleSectionController: ListSectionController {
var publishTitleViewModel: PublishTitleViewModel?
override func didUpdate(to object: Any) {
guard let currentViewModel = object as? PublishTitleViewModel else {
return
}
publishTitleViewModel = currentViewModel
}
override func numberOfItems() -> Int {
return 1
}
override func cellForItem(at index: Int) -> UICollectionViewCell {
guard let ctx = collectionContext,
let cell = ctx.dequeueReusableCell(of: PublishTitleCell.self, withReuseIdentifier: PublishTitleCell.identifier, for: self, at: index) as? PublishTitleCell else {
return UICollectionViewCell()
}
// Configure cell delegate to receive textViewDidChange Notification
cell.cellDelegate = self
return cell
}
override func sizeForItem(at index: Int) -> CGSize {
let width = collectionContext?.containerSize.width ?? 0
return CGSize(width: width, height: 50)
}
}
// MARK: - PublishTitleCellProtocol
extension TitleSectionController: PublishTitleCellProtocol {
func updateHeightOfRow(_ cell: PublishTitleCell, _ textView: UITextView) {
collectionContext?.invalidateLayout(for: self)
}
} PublishTitleCell.swiftimport UIKit
protocol PublishTitleCellProtocol: AnyObject {
func updateHeightOfRow(_ cell: PublishTitleCell, _ textView: UITextView)
}
class PublishTitleCell: UICollectionViewCell {
weak var cellDelegate: PublishTitleCellProtocol?
private lazy var iconImageView: UIImageView = {
let imageView = UIImageView()
imageView.translatesAutoresizingMaskIntoConstraints = false
imageView.contentMode = .scaleAspectFit
let config = UIImage.SymbolConfiguration(font: UIFont.preferredFont(forTextStyle: .body))
imageView.image = UIImage(systemName: "highlighter", withConfiguration: config)
imageView.tintColor = UIColor(red: 83/255.0, green: 202/255.0, blue: 195/255.0, alpha: 1.0)
return imageView
}()
private lazy var textView: UITextView = {
let textView = UITextView()
textView.translatesAutoresizingMaskIntoConstraints = false
textView.isScrollEnabled = false
textView.delegate = self
return textView
}()
private lazy var separatorView: UIView = {
let view = UIView()
view.translatesAutoresizingMaskIntoConstraints = false
view.backgroundColor = .secondarySystemBackground
return view
}()
public static var identifier: String {
return String(describing: self)
}
override init(frame: CGRect) {
super.init(frame: frame)
contentView.addSubview(iconImageView)
contentView.addSubview(textView)
contentView.addSubview(separatorView)
}
@available(*, unavailable)
public required init?(coder aDecoder: NSCoder) {
fatalError("init(coder:) has not been implemented")
}
override func layoutSubviews() {
super.layoutSubviews()
NSLayoutConstraint.activate([
iconImageView.topAnchor.constraint(equalTo: contentView.topAnchor, constant: 15),
iconImageView.leadingAnchor.constraint(equalTo: contentView.readableContentGuide.leadingAnchor),
textView.topAnchor.constraint(equalTo: contentView.topAnchor, constant: 10),
textView.leadingAnchor.constraint(equalTo: iconImageView.trailingAnchor, constant: 5),
textView.trailingAnchor.constraint(equalTo: contentView.readableContentGuide.trailingAnchor),
textView.bottomAnchor.constraint(equalTo: contentView.bottomAnchor, constant: -10),
separatorView.leadingAnchor.constraint(equalTo: contentView.readableContentGuide.leadingAnchor),
separatorView.trailingAnchor.constraint(equalTo: contentView.trailingAnchor),
separatorView.bottomAnchor.constraint(equalTo: contentView.bottomAnchor),
separatorView.heightAnchor.constraint(equalToConstant: 1)
])
}
override func prepareForReuse() {
super.prepareForReuse()
textView.text = nil
}
override func preferredLayoutAttributesFitting(_ layoutAttributes: UICollectionViewLayoutAttributes) -> UICollectionViewLayoutAttributes {
setNeedsLayout()
layoutIfNeeded()
let size = contentView.systemLayoutSizeFitting(layoutAttributes.size)
var newFrame = layoutAttributes.frame
newFrame.size.height = ceil(size.height)
layoutAttributes.frame = newFrame
return layoutAttributes
}
}
// MARK: - UITextViewDelegate
extension PublishTitleCell: UITextViewDelegate {
func textViewDidChange(_ textView: UITextView) {
if let delegate = cellDelegate {
delegate.updateHeightOfRow(self, textView)
}
}
} |
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. 😄
The text was updated successfully, but these errors were encountered: