Search code examples
iosuicollectionviewuiimageviewdidselectrowatindexpath

UICollectionView cell loads one image on top of another image


I had everything working perfectly in test.

In production, a user saved a few images, two are ok but for some reason, two are doubling up on top of another image.

When tapping on the image (didSelectItemAt)

collectionView.reloadData()

Gets called and each tap, changes the image to clear it up into just one image.

I've worked back from this point but I'm stuck.

Images loaded in viewDidLoad

db.collection("SAVED IMAGE IDS").getDocuments()
{
    (querySnapshot, err) in
    if let err = err
    {
        print("Error getting documents: \(err)")
    }
    else
    {
        for document in querySnapshot!.documents
        {
            let id = document.documentID
                
            let Ref = Storage.storage().reference(forURL: "SavedUserImages/\(id)")

            Ref.getData(maxSize: 1 * 1024 * 1024)
            {
                data, error in
                if error != nil
                {
                    print("Error: Image could not download!")
                }
                else
                {
                    let image = UIImage(data: data!)
    
                    self.picArray.append(image!)
                          
                    self.imageID.append(id)
                            
                    self.collectionView.reloadData()

                }
            }
        }
    }
}

Image loads in cell for row

func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell
{
     let cell = collectionView.dequeueReusableCell(withReuseIdentifier: cellID, for: indexPath as IndexPath)
        
     let data = picArray[indexPath.row]
        
     cell.layer.shouldRasterize = true
     cell.layer.rasterizationScale = UIScreen.main.scale
        
     let iv = UIImageView()
     cell.contentView.addSubview(iv)
     iv.frame = cell.contentView.frame
     iv.image = data
        
     collectionView.selectItem(at: IndexPath(item: 0, section: 0) as IndexPath, animated: false, scrollPosition: .init())
        
     return cell
}

Thanks in advance for any help


Solution

  • With this code:

    enter image description here

    you are adding another image view every time you reload the cell.

    Instead, you need to design your cell to already have the image view and change your code to:

    func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell
    {
         let cell = collectionView.dequeueReusableCell(withReuseIdentifier: cellID, for: indexPath as IndexPath)
            
         let data = picArray[indexPath.row]
            
         cell.iv.image = data
            
         return cell
    }
    

    Edit - further explanation and examples...

    Based on the code you've shown, you are using a default UICollectionViewCell instead of a custom subclassed cell.

    So, if we do a complete example, using SF Symbol images from 0 to 14 for the picArray, using your approach:

    class ViewController: UIViewController, UICollectionViewDataSource, UICollectionViewDelegate {
        
        let cellID: String = "cell"
        
        var collectionView: UICollectionView!
        var cvWidth: CGFloat = 0
        
        // let's use an array of images for this example
        var picArray: [UIImage] = []
        
        override func viewDidLoad() {
            super.viewDidLoad()
            
            // create images 0 through 14
            for i in 0..<15 {
                if let img = UIImage(systemName: "\(i).circle") {
                    picArray.append(img)
                }
            }
            
            let fl = UICollectionViewFlowLayout()
            fl.scrollDirection = .vertical
            
            collectionView = UICollectionView(frame: .zero, collectionViewLayout: fl)
            collectionView.translatesAutoresizingMaskIntoConstraints = false
            view.addSubview(collectionView)
            
            let g = view.safeAreaLayoutGuide
            NSLayoutConstraint.activate([
                collectionView.topAnchor.constraint(equalTo: g.topAnchor),
                collectionView.leadingAnchor.constraint(equalTo: g.leadingAnchor),
                collectionView.trailingAnchor.constraint(equalTo: g.trailingAnchor),
                collectionView.bottomAnchor.constraint(equalTo: g.bottomAnchor),
            ])
            
            collectionView.dataSource = self
            collectionView.delegate = self
            
            // it appears you're using a default collection view cell class
            collectionView.register(UICollectionViewCell.self, forCellWithReuseIdentifier: cellID)
    
        }
    
        override func viewDidLayoutSubviews() {
            super.viewDidLayoutSubviews()
            
            // only do this is the collection view frame has changed
            if cvWidth != collectionView.frame.width {
                cvWidth = collectionView.frame.width
                if let fl = collectionView.collectionViewLayout as? UICollectionViewFlowLayout {
                    fl.itemSize = CGSize(width: cvWidth, height: 200.0)
                }
            }
        }
        
        func collectionView(_ collectionView: UICollectionView, numberOfItemsInSection section: Int) -> Int {
            return picArray.count
        }
        
        func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell
        {
            let cell = collectionView.dequeueReusableCell(withReuseIdentifier: cellID, for: indexPath as IndexPath)
    
            let data = picArray[indexPath.row]
    
            cell.layer.shouldRasterize = true
            cell.layer.rasterizationScale = UIScreen.main.scale
    
            // this is wrong... we're Adding ANOTHER image view every time
            let iv = UIImageView()
            cell.contentView.addSubview(iv)
            iv.frame = cell.contentView.frame
            iv.image = data
    
            // this makes no sense, but I'll leave it here
            collectionView.selectItem(at: IndexPath(item: 0, section: 0) as IndexPath, animated: false, scrollPosition: .init())
    
            return cell
        }
        
    }
    

    It looks like this at the start:

    enter image description here

    if we scroll all the way down - to where the "14" image should be the bottom cell, it looks like this:

    enter image description here

    If we scroll back to the top:

    enter image description here

    and after scrolling up and down several times:

    enter image description here

    As we can see, as the cells are reused we keep adding more and more image views on top of each other.

    So, instead, let's create a simple custom cell subclass that creates and adds an image view when it is created:

    class SimpleImageCell: UICollectionViewCell {
        let imgView = UIImageView()
        override init(frame: CGRect) {
            super.init(frame: frame)
            commonInit()
        }
        required init?(coder: NSCoder) {
            super.init(coder: coder)
            commonInit()
        }
        private func commonInit() {
            imgView.translatesAutoresizingMaskIntoConstraints = false
            contentView.addSubview(imgView)
            let g = contentView.layoutMarginsGuide
            NSLayoutConstraint.activate([
                imgView.topAnchor.constraint(equalTo: g.topAnchor),
                imgView.leadingAnchor.constraint(equalTo: g.leadingAnchor),
                imgView.trailingAnchor.constraint(equalTo: g.trailingAnchor),
                imgView.bottomAnchor.constraint(equalTo: g.bottomAnchor),
            ])
        }
    }
    

    and we'll use an almost identical view controller - the only differences are registering our SimpleImageCell class, and using a correct cellForItemAt func:

    class ViewController: UIViewController, UICollectionViewDataSource, UICollectionViewDelegate {
        
        let cellID: String = "cell"
        
        var collectionView: UICollectionView!
        var cvWidth: CGFloat = 0
        
        // let's use an array of images for this example
        var picArray: [UIImage] = []
        
        override func viewDidLoad() {
            super.viewDidLoad()
            
            // create images 0 through 14
            for i in 0..<15 {
                if let img = UIImage(systemName: "\(i).circle") {
                    picArray.append(img)
                }
            }
            
            let fl = UICollectionViewFlowLayout()
            fl.scrollDirection = .vertical
            
            collectionView = UICollectionView(frame: .zero, collectionViewLayout: fl)
            collectionView.translatesAutoresizingMaskIntoConstraints = false
            view.addSubview(collectionView)
            
            let g = view.safeAreaLayoutGuide
            NSLayoutConstraint.activate([
                collectionView.topAnchor.constraint(equalTo: g.topAnchor),
                collectionView.leadingAnchor.constraint(equalTo: g.leadingAnchor),
                collectionView.trailingAnchor.constraint(equalTo: g.trailingAnchor),
                collectionView.bottomAnchor.constraint(equalTo: g.bottomAnchor),
            ])
            
            collectionView.dataSource = self
            collectionView.delegate = self
            
            // register cell class that already has an image view
            collectionView.register(SimpleImageCell.self, forCellWithReuseIdentifier: cellID)
        }
        override func viewDidLayoutSubviews() {
            super.viewDidLayoutSubviews()
            
            // only do this is the collection view frame has changed
            if cvWidth != collectionView.frame.width {
                cvWidth = collectionView.frame.width
                if let fl = collectionView.collectionViewLayout as? UICollectionViewFlowLayout {
                    fl.itemSize = CGSize(width: cvWidth, height: 200.0)
                }
            }
        }
        
        func collectionView(_ collectionView: UICollectionView, numberOfItemsInSection section: Int) -> Int {
            return picArray.count
        }
        
        func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell {
            let cell = collectionView.dequeueReusableCell(withReuseIdentifier: cellID, for: indexPath) as! SimpleImageCell
            
            let data = picArray[indexPath.item]
            cell.imgView.image = data
            
            return cell
        }
    
    }
    

    The results:

    enter image description here enter image description here

    We can scroll up and down all we want, and we will never have images "on top of" each other.