Juanpe / SkeletonView

☠️ An elegant way to show users that something is happening and also prepare them to which contents they are awaiting
MIT License
12.53k stars 1.1k forks source link

SkeletonViews not removed from reused table view cells #390

Closed lordzsolt closed 3 years ago

lordzsolt commented 3 years ago

Description

Describe your issue here.

What type of issue is this? (place an x in one of the [ ])

Requirements (place an x in each of the [ ])


Bug Report

When the table view cell gets reused, the gray skeleton view is still visible.

Tested on iPad 9.7-inch and iPhone 8 Plus.

SkeletonView Environment:

SkeletonView version: 1.13 and 1.15 Xcode version: 12.5 Swift version: 5

Steps to reproduce:

  1. Run the project
  2. Wait for skeletons to be hidden
  3. Scroll down
  4. The 8th or 9th cell will still have the gray skeleton view

Expected result:

Skeleton view is visible

Actual result:

Skeleton view is no longer visible.

Attachments:

https://github.com/lordzsolt/SkeletonView-Test

aaronvegh commented 3 years ago

I had this issue as well. Hoo boy I thought I was sunk, having just spent a day+ implementing it in this app! I was able to find a solution by implementing something like this in the UITableViewCell's prepareForReuse() method:

findViews(subclassOf: UIView.self).forEach { view in
    view.layer.sublayers?
        .filter { $0 is CAGradientLayer }
        .forEach { layer in
            layer.removeFromSuperlayer(   
    }
}

findViews() is a UIView extension that includes these methods:

func findViews<T: UIView>(subclassOf: T.Type) -> [T] {
            return recursiveSubviews.compactMap { $0 as? T }
    }

    var recursiveSubviews: [UIView] {
        return subviews + subviews.flatMap { $0.recursiveSubviews }
    }

The point here is that I'm manually doing what I figure this framework should be doing, removing the CAGradientLayer from all the relevant views. For reasons I couldn't determine from looking at the code, this simply isn't happening. I suspect some break in the view hierarchy as the SkeletonView code is doing its job. In any event, this solves the problem for me.

lordzsolt commented 3 years ago

I had this issue as well. Hoo boy I thought I was sunk, having just spent a day+ implementing it in this app! I was able to find a solution by implementing something like this in the UITableViewCell's prepareForReuse() method:

findViews(subclassOf: UIView.self).forEach { view in
    view.layer.sublayers?
        .filter { $0 is CAGradientLayer }
        .forEach { layer in
            layer.removeFromSuperlayer(   
    }
}

findViews() is a UIView extension that includes these methods:

func findViews<T: UIView>(subclassOf: T.Type) -> [T] {
            return recursiveSubviews.compactMap { $0 as? T }
    }

    var recursiveSubviews: [UIView] {
        return subviews + subviews.flatMap { $0.recursiveSubviews }
    }

The point here is that I'm manually doing what I figure this framework should be doing, removing the CAGradientLayer from all the relevant views. For reasons I couldn't determine from looking at the code, this simply isn't happening. I suspect some break in the view hierarchy as the SkeletonView code is doing its job. In any event, this solves the problem for me.

I guess one important question at that point is.... What's the performance impact of this.

Since table views/collection views are highly sensitive to performance. Prepare for reuse happens when scrolling, and that's the moment where it's the most important that not a lot is happening, so it doesn't cause hitches.

Juanpe commented 3 years ago

Hi!

Please, check issue #399, I think this issue it's related and it could be duplicated

Juanpe commented 3 years ago

The latest version should fix this problem. Please, could you check it to close this issue?

stale[bot] commented 3 years ago

🤖 This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions 🙂