Skip to content
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

isSkeletonActive returns incorrect value #421

Closed
4 of 8 tasks
p-nicolaou opened this issue Jul 14, 2021 · 11 comments
Closed
4 of 8 tasks

isSkeletonActive returns incorrect value #421

p-nicolaou opened this issue Jul 14, 2021 · 11 comments
Labels

Comments

@p-nicolaou
Copy link

Description

In our app, we display the SkeletonViews while waiting for remote data to be available. Once received, we hide skeleton and call reloadData()

In addition, we're implementing UITableViewDelegate's method as below

override func tableView(_ tableView: UITableView, willDisplay cell: UITableViewCell, forRowAt indexPath: IndexPath) {
    guard view.isSkeletonActive == false else { return }
}

in order to update the UI with some extra info.

Everything was working as expected while on SkeletonView 1.15.0.

Recently, we attempted to upgrade to a most recent 1.20.0 but doing so we realise that the logic described above is now broken. view.isSkeletonActive is shown as true even though we've called the hideSkeleton() before that.

We've spent few days in order to understand what's going on but eventually it comes down to this merge #402 (comment). If I revert the changes described into this commit then, everything works as before. I understand that this change has been introduced with 1.17.0. The 1.16.0 works ok too.

Do we have to use isSkeletonActive in a different way since 1.17.0 or is this an actual bug?

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

  • bug
  • enhancement (feature request)
  • question
  • documentation related
  • discussion

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

  • I've read and understood the Contributing guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've searched for any related issues and avoided creating a duplicate issue.

Bug Report

Filling out the following details about bugs will help us solve your issue sooner.

SkeletonView Environment:

SkeletonView version: 1.20.0
Xcode version: 12.5.1
Swift version: Swift 5.5

Steps to reproduce:

Please replace this with the steps to reproduce the behavior.

  1. Implement willDisplayCell of UITableViewDelegate protocol
  2. Print the value of view.isSkeletonActive inside
  3. On viewDidLayoutSubviews call view.showAnimatedGradientSkeleton(animation: animation) once
  4. Call view.hideSkeleton() and tableView.reloadData() after a while

Expected result:

view.isSkeletonActive should print false

Actual result:

view.isSkeletonActive prints true

Attachments:

These screenshots show the actual value of isSkeletonActive when is called inside willDisplayCell using versions 1.16.0 and 1.17.0 (as described on reproduce steps above).

Screenshot 2021-07-14 at 22 45 18

Screenshot 2021-07-14 at 22 49 01

@p-nicolaou
Copy link
Author

@Juanpe any chance being able to look into this issue?

@Juanpe
Copy link
Owner

Juanpe commented Jul 20, 2021

Hi @p-nicolaou,

The library reloads the data when the skeleton will hide, although you can omit this step.
Could you try to call view.hideSkeleton(reloadDataAfter: false) instead?

@p-nicolaou
Copy link
Author

Thank you for your response @Juanpe,

I've tried that already with no luck unfortunately.

One thing I noticed is that, by calling view.hideSkeleton(reloadDataAfter: false) and manually calling tableView.reloadData() is not working BUT
if I wrap the later into DispatchQueue.main.asyncAfter(deadline: .now() + .seconds(2)) { } then it works as before. The guard view.isSkeletonActive == false else { return } is false as it should be.

@Juanpe
Copy link
Owner

Juanpe commented Jul 20, 2021

Mm, ok, maybe it's related to the transition. If you dispatch after 0.25secs, it should work as well.

I'm not sure why this issue but with this clue I'm on track of find a possible solution, meanwhile a workaround for you could be dispatching the reload data task after 0.3secs

@p-nicolaou
Copy link
Author

I will try to replicate it in a sample project as well. That way, I can share the full code as well. In the mean time, have a look at this too. Might be helpful somehow,

I've change the isSkeletonActive to this:

var isSkeletonActive: Bool {
      if self is UITableView {
        print(">>> \(subviewsSkeletonables)")
      }
      return status == .on || subviewsSkeletonables.contains(where: { $0.isSkeletonActive })
}

Then, I ran my app using versions SkeletonView 1.16.0 & 1.17.2

1.16.0

>>> []
>>> []
>>> []
>>> []
>>> []
>>> []
>>> []
>>> []
>>> []
>>> [<MyApp.SkeletonTableViewCell: 0x7f8c65c08580; baseClass = UITableViewCell; frame = (0 44; 390 371); autoresize = W; layer = <CALayer: 0x600003fa0440>>, <MyApp.SkeletonTableViewCell: 0x7f8c687e37c0; baseClass = UITableViewCell; frame = (0 415; 390 371); autoresize = W; layer = <CALayer: 0x600003ff96a0>>]
>>> [<MyApp.SkeletonTableViewCell: 0x7f8c65c08580; baseClass = UITableViewCell; frame = (0 44; 390 371); autoresize = W; layer = <CALayer: 0x600003fa0440>>, <MyApp.SkeletonTableViewCell: 0x7f8c687e37c0; baseClass = UITableViewCell; frame = (0 415; 390 371); autoresize = W; layer = <CALayer: 0x600003ff96a0>>]
>>> []
>>> []
>>> []
>>> []

1.17.2

>>> []
>>> []
>>> []
>>> [<MyApp.SkeletonTableViewCell: 0x7f9ec64b29f0; baseClass = UITableViewCell; frame = (0 44; 390 371); autoresize = W; layer = <CALayer: 0x600002c055a0>>]
>>> [<MyApp.SkeletonTableViewCell: 0x7f9ec64b29f0; baseClass = UITableViewCell; frame = (0 44; 390 371); autoresize = W; layer = <CALayer: 0x600002c055a0>>]
>>> [<MyApp.SkeletonTableViewCell: 0x7f9ec64a59c0; baseClass = UITableViewCell; frame = (0 415; 390 371); autoresize = W; layer = <CALayer: 0x600002c05080>>, <MyApp.SkeletonTableViewCell: 0x7f9ec64b29f0; baseClass = UITableViewCell; frame = (0 44; 390 371); autoresize = W; layer = <CALayer: 0x600002c055a0>>]
>>> [<MyApp.SkeletonTableViewCell: 0x7f9ec64a59c0; baseClass = UITableViewCell; frame = (0 415; 390 371); autoresize = W; layer = <CALayer: 0x600002c05080>>, <MyApp.SkeletonTableViewCell: 0x7f9ec64b29f0; baseClass = UITableViewCell; frame = (0 44; 390 371); autoresize = W; layer = <CALayer: 0x600002c055a0>>]
>>> [<MyApp.SkeletonTableViewCell: 0x7f9eccd37640; baseClass = UITableViewCell; frame = (0 786; 390 371); autoresize = W; layer = <CALayer: 0x600002c075a0>>, <MyApp.SkeletonTableViewCell: 0x7f9ec64a59c0; baseClass = UITableViewCell; frame = (0 415; 390 371); autoresize = W; layer = <CALayer: 0x600002c05080>>, <MyApp.SkeletonTableViewCell: 0x7f9ec64b29f0; baseClass = UITableViewCell; frame = (0 44; 390 371); autoresize = W; layer = <CALayer: 0x600002c055a0>>]
>>> [<MyApp.SkeletonTableViewCell: 0x7f9eccd37640; baseClass = UITableViewCell; frame = (0 786; 390 371); autoresize = W; layer = <CALayer: 0x600002c075a0>>, <MyApp.SkeletonTableViewCell: 0x7f9ec64a59c0; baseClass = UITableViewCell; frame = (0 415; 390 371); autoresize = W; layer = <CALayer: 0x600002c05080>>, <MyApp.SkeletonTableViewCell: 0x7f9ec64b29f0; baseClass = UITableViewCell; frame = (0 44; 390 371); autoresize = W; layer = <CALayer: 0x600002c055a0>>]
>>> [<MyApp.SkeletonTableViewCell: 0x7f9eccd37640; baseClass = UITableViewCell; frame = (0 786; 390 371); autoresize = W; layer = <CALayer: 0x600002c075a0>>, <MyApp.SkeletonTableViewCell: 0x7f9ec64a59c0; baseClass = UITableViewCell; frame = (0 415; 390 371); autoresize = W; layer = <CALayer: 0x600002c05080>>, <MyApp.SkeletonTableViewCell: 0x7f9ec64b29f0; baseClass = UITableViewCell; frame = (0 44; 390 371); autoresize = W; layer = <CALayer: 0x600002c055a0>>]
>>> [<MyApp.SkeletonTableViewCell: 0x7f9eccd37640; baseClass = UITableViewCell; frame = (0 786; 390 371); hidden = YES; autoresize = W; layer = <CALayer: 0x600002c075a0>>, <MyApp.SkeletonTableViewCell: 0x7f9ec64a59c0; baseClass = UITableViewCell; frame = (0 415; 390 371); autoresize = W; layer = <CALayer: 0x600002c05080>>, <MyApp.SkeletonTableViewCell: 0x7f9ec64b29f0; baseClass = UITableViewCell; frame = (0 44; 390 371); autoresize = W; layer = <CALayer: 0x600002c055a0>>]
>>> [<MyApp.SkeletonTableViewCell: 0x7f9eccd37640; baseClass = UITableViewCell; frame = (0 786; 390 371); hidden = YES; autoresize = W; layer = <CALayer: 0x600002c075a0>>, <MyApp.SkeletonTableViewCell: 0x7f9ec64a59c0; baseClass = UITableViewCell; frame = (0 415; 390 371); autoresize = W; layer = <CALayer: 0x600002c05080>>, <MyApp.SkeletonTableViewCell: 0x7f9ec64b29f0; baseClass = UITableViewCell; frame = (0 44; 390 371); autoresize = W; layer = <CALayer: 0x600002c055a0>>]
>>> [<MyApp.SkeletonTableViewCell: 0x7f9eccd37640; baseClass = UITableViewCell; frame = (0 786; 390 371); hidden = YES; autoresize = W; layer = <CALayer: 0x600002c075a0>>, <MyApp.SkeletonTableViewCell: 0x7f9ec64a59c0; baseClass = UITableViewCell; frame = (0 415; 390 371); hidden = YES; autoresize = W; layer = <CALayer: 0x600002c05080>>, <MyApp.SkeletonTableViewCell: 0x7f9ec64b29f0; baseClass = UITableViewCell; frame = (0 44; 390 371); hidden = YES; autoresize = W; layer = <CALayer: 0x600002c055a0>>]
>>> [<MyApp.SkeletonTableViewCell: 0x7f9eccd37640; baseClass = UITableViewCell; frame = (0 786; 390 371); hidden = YES; autoresize = W; layer = <CALayer: 0x600002c075a0>>, <MyApp.SkeletonTableViewCell: 0x7f9ec64a59c0; baseClass = UITableViewCell; frame = (0 415; 390 371); hidden = YES; autoresize = W; layer = <CALayer: 0x600002c05080>>, <MyApp.SkeletonTableViewCell: 0x7f9ec64b29f0; baseClass = UITableViewCell; frame = (0 44; 390 371); hidden = YES; autoresize = W; layer = <CALayer: 0x600002c055a0>>]
>>> [<MyApp.SkeletonTableViewCell: 0x7f9eccd37640; baseClass = UITableViewCell; frame = (0 786; 390 371); hidden = YES; autoresize = W; layer = <CALayer: 0x600002c075a0>>, <MyApp.SkeletonTableViewCell: 0x7f9ec64a59c0; baseClass = UITableViewCell; frame = (0 415; 390 371); hidden = YES; autoresize = W; layer = <CALayer: 0x600002c05080>>, <MyApp.SkeletonTableViewCell: 0x7f9ec64b29f0; baseClass = UITableViewCell; frame = (0 44; 390 371); hidden = YES; autoresize = W; layer = <CALayer: 0x600002c055a0>>]
>>> [<MyApp.SkeletonTableViewCell: 0x7f9eccd37640; baseClass = UITableViewCell; frame = (0 786; 390 371); hidden = YES; autoresize = W; layer = <CALayer: 0x600002c075a0>>, <MyApp.SkeletonTableViewCell: 0x7f9ec64a59c0; baseClass = UITableViewCell; frame = (0 415; 390 371); hidden = YES; autoresize = W; layer = <CALayer: 0x600002c05080>>, <MyApp.SkeletonTableViewCell: 0x7f9ec64b29f0; baseClass = UITableViewCell; frame = (0 44; 390 371); hidden = YES; autoresize = W; layer = <CALayer: 0x600002c055a0>>]

SkeletonTableViewCell is the dummy cell we're using while fetching remote data

@p-nicolaou
Copy link
Author

@Juanpe I can confirm that by passing transition = .none here,
view.hideSkeleton(transition: .none) also solves the issue. So, as you correctly speculated, has something to do with the transition animation. But I can't figure out why is causing this issue with 1.17.2.

@Juanpe
Copy link
Owner

Juanpe commented Jul 22, 2021

Yeah, made sense :) I've created a branch with a possible fix. I think it'll be enough.

Please, could you point to this branch to check if the problem is solved?
The branch is bug/set_skeleton_off_before_transition, so if you are using Cocoapods, you should replace the current line for this one:

pod 'SkeletonView', :git => 'https://github.com/Juanpe/SkeletonView.git', :branch => 'bug/set_skeleton_off_before_transition'

@p-nicolaou
Copy link
Author

p-nicolaou commented Jul 23, 2021

I can confirm that the problem is solved under the branch 'bug/set_skeleton_off_before_transition'

Good catch. 🙂

@Juanpe
Copy link
Owner

Juanpe commented Jul 23, 2021

Great! I'll create a new version that includes this fix. Thanks for your help 🤙🏼

@p-nicolaou
Copy link
Author

Thank you for taking the time looking into this

@Juanpe
Copy link
Owner

Juanpe commented Jul 23, 2021

Version 1.21.2 has been released 🚀

@Juanpe Juanpe closed this as completed Jul 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants