Search code examples
iosswiftuibezierpathcashapelayer

CAShapeLayer not going away


I have a class that adds a progress view to a given UIView. I use this in a collection view in its cells. It looks good on the first couple of cells, but after swiping a few times you can see faint marks that should be there. I will attach screenshots of what I mean. Not sure why this is happening?

First couple cells looks good

After swiping a few times you can see faint progress layers

Here is cellForItemAt

guard let cell = collectionView.dequeueReusableCell(withReuseIdentifier: self.cellId, for: indexPath) as? TakeCollectionViewCell else {return UICollectionViewCell()}
let service = TakeProgressView()
let random = Double(indexPath.row) / 10
service.progress = random
service.addView(view: cell.takeProgressView, withParticipants: false)
return cell

where takeProgressView is a UIView implemented in storyboard

Here is the function that adds the view:

func addView(view: UIView, withParticipants: Bool){
        disagreePath.removeAllPoints()
        agreePath.removeAllPoints()
        path.removeAllPoints()
        circlePath.removeAllPoints()
        agreeLayer.removeFromSuperlayer()
        disagreeLayer.removeFromSuperlayer()
        backgroundFill.removeFromSuperlayer()
    
        let width:CGFloat = 270
        let height:CGFloat = 360

        
        let viewX = (view.frame.size.width - width) / 2
        let viewY = (view.frame.size.height - height) / 2
        
        subView = UIView(frame: CGRect(x: viewX - 20, y: viewY, width: width + 40, height: height))

        path = UIBezierPath(ovalIn: CGRect(x: 20, y: 0, width: width, height: height))
        
        let rect = CGRect(x: 60, y: 40, width: width - 80, height: height - 80)
        
        circlePath = UIBezierPath(ovalIn: rect)
        path.append(circlePath)
        path.usesEvenOddFillRule = true
        
                
        backgroundFill = CAShapeLayer()
        backgroundFill.path = path.cgPath
        backgroundFill.lineWidth = 40.0
        backgroundFill.fillRule = .evenOdd
        backgroundFill.fillColor = #colorLiteral(red: 0.9371728301, green: 0.9373074174, blue: 0.9371433854, alpha: 1)
        backgroundFill.opacity = 0.9

        
        disagreePath = UIBezierPath(ovalIn: CGRect(x: 30, y: 10, width: width - 20, height: height - 20))
        disagreeLayer = CAShapeLayer()
        disagreeLayer.path = disagreePath.cgPath
        disagreeLayer.fillColor = UIColor.clear.cgColor
        disagreeLayer.strokeColor = #colorLiteral(red: 1, green: 0.4117892087, blue: 0.4117360711, alpha: 1)
        disagreeLayer.strokeStart = 0.0
        disagreeLayer.strokeEnd = CGFloat(1 - progress)
        disagreeLayer.lineWidth = 20.0
        disagreeLayer.lineCap = .round
        
        backgroundFill.addSublayer(disagreeLayer)
        
        agreePath = UIBezierPath(ovalIn: CGRect(x: 50, y: 30, width: width - 60, height: height - 60))
        agreeLayer = CAShapeLayer()
        agreeLayer.path = agreePath.cgPath
        agreeLayer.fillColor = UIColor.clear.cgColor
        agreeLayer.strokeColor = UIColor.uStadium.primary.cgColor
        agreeLayer.strokeStart = CGFloat(1 - progress) + 0.02
        agreeLayer.strokeEnd = 0.98
        agreeLayer.lineWidth = 20.0
        agreeLayer.lineCap = .round
        
        
        backgroundFill.addSublayer(agreeLayer)
        subView.layer.addSublayer(backgroundFill)
        view.addSubview(subView)
    }

Solution

  • Cells (in UITableView or UICollectionView) are reused.

    Your code is too much coupled, you should change the architecture of it.

    If I understood the logic:

    TakeCollectionViewCell should have a property TakeProgressView:

    class TakeCollectionViewCell: UICollectionViewCell {
        var takeProgress = TakeProgressView()
        ...
    
    }
    

    So your code should be then:

    guard let cell = collectionView.dequeueReusableCell(withReuseIdentifier: self.cellId, for: indexPath) as? TakeCollectionViewCell else {return UICollectionViewCell()}
    let service = TakeProgressView()
    let random = Double(indexPath.row) / 10
    cell.takeProgress.progress = random
    cell.takeProgress.addView(view: cell.takeProgressView, withParticipants: false)
    return cell
    

    In func addView(view: UIView, withParticipants: Bool), you only use view for: frame calculation + addSuview:

    ...
    let viewX = (view.frame.size.width - width) / 2
    let viewY = (view.frame.size.height - height) / 2
    ...
    view.addSubview(subView)
    ...
    
    

    So instead, let's return the subview and give the frame (or even the size since that's what you use):

    func progressSubview(with size: CGSize, withParticipants: Bool) -> UIView {
        ...
        // let viewX = (view.frame.size.width - width) / 2
        let viewX = (size.width - width) / 2
        // let viewY = (view.frame.size.height - height) / 2
        let viewY = (size.height - height) / 2
        ...
        // view.addSubview(subView)
        return subview
        ...
    }
    

    Let's keep that subview as a property of TakeCollectionViewCell and let's use the new method.

    class TakeCollectionViewCell: UICollectionViewCell {
        var subProgressView: UIView?
        ...
        func updateProgressView(with progress: Double, withParticipants: Bool) {
            takeProgress.progress = progress
            subProgressView = takeProgress.progressSubview(with: self.takeProgressView.size, withParticipants: withParticipants)
        }
    }
    

    Now let's override the reuse:

    class TakeCollectionViewCell: UICollectionViewCell {
        override func prepareForReuse() {
            super.prepareForReuse()
            subProgressView?.removeFromSuperview()
        }
    }
    

    In cellForRow should be now:

    guard let cell = collectionView.dequeueReusableCell(withReuseIdentifier: self.cellId, for: indexPath) as? TakeCollectionViewCell else {return UICollectionViewCell()}
    let random = Double(indexPath.row) / 10
    cell.updateProgressView(with: random, withParticipants: false)
    return cell
    

    Additional rework may be done, but that should be a start...