search

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

Feature - Configure Cell #413

Closed nemanjachoco closed 3 years ago

nemanjachoco commented 3 years ago

Add method "collectionSkeletonViewConfigureCell" to "SkeletonCollectionDataSource". This will allow configuration of the cell before being displayed by SkeletonView.

Summary

Issue Link

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

Juanpe commented 3 years ago

Hi @nemanjachoco,

First, thanks for contributing to SkeletonView 🎉

Some pull requests allowed the developers to create their own skeleton cells, but they were declined because there wasn't any issue related to this. But I think this is your case.

To clarify your problem and try to find the best solution, could it be useful for you deque the cell as well?

BTW, I'll do some suggestions:

Juanpe commented 3 years ago

Please, I've added some new methods, so rebase your branch in order to be up to date :)

You can check more details here: #414

nemanjachoco commented 3 years ago

Hi @nemanjachoco,

First, thanks for contributing to SkeletonView 🎉

Some pull requests allowed the developers to create their own skeleton cells, but they were declined because there wasn't any issue related to this. But I think this is your case.

To clarify your problem and try to find the best solution, could it be useful for you deque the cell as well?

BTW, I'll do some suggestions:

  • It could be a good idea to do the same for collection view items.
  • You should update the README file to add this new feature.

Hi @Juanpe, it was my pleasure to contribute to SkeletonView and thank you for such a quick reply!

I will rebase my branch soon, rename the method name, add it to collection view as well and updated to readme file. I will notify you once all those changes are done.

nemanjachoco commented 3 years ago

Please, I've added some new methods, so rebase your branch in order to be up to date :)

You can check more details here: #414

@Juanpe I just checked this. I think this PR does the same thing as mine does. IMO it is enough to have only one solution, do we feel we should close the current PR?

Juanpe commented 3 years ago

Please, I've added some new methods, so rebase your branch in order to be up to date :)

You can check more details here: #414

@Juanpe I just checked this. I think this PR does the same thing as mine does. IMO it is enough to have only one solution, do we feel we should close the current PR?

Not at all, I mean, now, the library allow dequeue the cell manually, but sometimes you only need to make some changes and not modify the logic that dequeues the cells.

So, I would add your proposal, IMHO, the more flexible the library is the more useful is.

WDYT?

nemanjachoco commented 3 years ago

Please, I've added some new methods, so rebase your branch in order to be up to date :)

You can check more details here: #414

@Juanpe I just checked this. I think this PR does the same thing as mine does. IMO it is enough to have only one solution, do we feel we should close the current PR?

Not at all, I mean, now, the library allow dequeue the cell manually, but sometimes you only need to make some changes and not modify the logic that dequeues the cells.

So, I would add your proposal, IMHO, the more flexible the library is the more useful is.

WDYT?

Sure thing, i am more than glad to finish the proposal.

nemanjachoco commented 3 years ago

Will create new PR, there is an issue with rebase and do not have time atm to fix it :)