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

Allow For collectionSkeletonView(_:numberOfRowsInSection:) to Return EstimatedNumberOfRows #400

Closed
rlziii opened this issue Jun 8, 2021 · 6 comments · Fixed by #401
Closed

Comments

@rlziii
Copy link
Contributor

rlziii commented Jun 8, 2021

Describe the feature or problem you’d like to solve

Currently if a class conforming to SkeletonTableViewDataSource implements collectionSkeletonView(_:numberOfRowsInSection:), there is no way to fall back on the default implementation (which returns skeletonView.estimatedNumberOfRows) since estimatedNumberOfRows has internal protection level. This makes it difficult for wrappers that implement SkeletonTableViewDataSource to allow for conditional conformance of collectionSkeletonView(_:numberOfRowsInSection:) to return a possible (i.e. "overriding") value since the return type is Int.

Proposed solution

Change the return type of collectionSkeletonView(_:numberOfRowsInSection:) from Int to Int? where returning nil will then cause the default implementation to fallback to returning skeletonView.estimatedNumberOfRows like usual. This will make the full protocol function signature:

func collectionSkeletonView(_ skeletonView: UITableView, numberOfRowsInSection section: Int) -> Int?

This would allow for something like the following:

var configuredRowCount: Int?
func collectionSkeletonView(_ skeletonView: UITableView, numberOfRowsInSection section: Int) -> Int? {
  // If this is non-nil, then the number of rows is set to `configuredRowCount`.
  // If it's not set (i.e. it is nil), then the default `estimatedNumberOfRows` is used instead.
  return configuredRowCount
}
@Juanpe Juanpe self-assigned this Jun 9, 2021
@Juanpe
Copy link
Owner

Juanpe commented Jun 9, 2021

Hi @rlziii!

I don't exactly know what do you mean... 🤔 Now, you can define the number of rows that table will show when the skeleton is active by the data source method:

func collectionSkeletonView(_ skeletonView: UITableView, numberOfRowsInSection section: Int) -> Int {
    1
}

Instead, if you don't implement this method, the library will calculate the estimated number of rows:

This is what you want to do?

@rlziii
Copy link
Contributor Author

rlziii commented Jun 9, 2021

What you described is correct. However, I was previously using a custom abstraction around UITableViewDataSource, and I am now trying to make that flexible enough to use with SkeletonTableViewDataSource. I would like to be able to pass in something like a SkeletonConfiguration that would, among other things, have a rowCounts dictionary (i.e. [Int: Int?]). And then be able to do something like the following:

func collectionSkeletonView(_ skeletonView: UITableView, numberOfRowsInSection section: Int) -> Int? {
    skeletonConfiguration?.rowCounts[section]
}

If the above resulted in a non-nil valve, then I would like that value to be used like it is today. If the above resulted in a nil value, however, I would like it to behave as though the delegate method wasn't implemented at all (i.e. I would like it to fallback to returning skeletonView.estimatedNumberOfRows essentially). That would allow much more flexibility when creating abstractions around SkeletonTableViewDataSource. Right now I don't see how that's possible unless the desired behavior is always the same throughout the app: either having a set number of rows or having a the SkeletonTableViewDataSource calculate the number for you.

@Juanpe
Copy link
Owner

Juanpe commented Jun 9, 2021

Ok, I follow you now. 👍🏼

Yep, good point, and this could be very useful and would make more flexible the library. However, I'm not sure if modifying the signature is a good idea. 🤔

Another possibility could be to create a special case to define that the number of rows is indeterminate. Something like this:
SkeletonCollectionView.automaticNumberOfRows or SkeletonCollectionView.indeterminateNumberOfRows

Then, the code would like this:

func collectionSkeletonView(_ skeletonView: UITableView, numberOfRowsInSection section: Int) -> Int? {
    skeletonConfiguration?.rowCounts[section] ?? SkeletonCollectionView.automaticNumberOfRows
}

WDYT? could be a good solution to this issue?

@rlziii
Copy link
Contributor Author

rlziii commented Jun 9, 2021

That would absolutely work. 👍 In the Objective-C days that was a common solution, or returning something like -1 (which I don't like). The constant approach (i.e. SkeletonCollectionView.automaticNumberOfRows) is nice because, like you said, it doesn't involve having to change the protocol's method signature.

@Juanpe
Copy link
Owner

Juanpe commented Jun 9, 2021

Cool, would you like to contribute to the project? It would be great :)

@rlziii
Copy link
Contributor Author

rlziii commented Jun 9, 2021

Will do. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants