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

collectionview optional? #32

Closed
onsissond opened this issue Jan 9, 2019 · 3 comments
Closed

collectionview optional? #32

onsissond opened this issue Jan 9, 2019 · 3 comments

Comments

@onsissond
Copy link

onsissond commented Jan 9, 2019

Hello! Why is collection view in DTCollectionViewManageable optional? It will be better if it is nonoptional value like in DTTableViewManageable

public protocol DTTableViewManageable : class
{
    /// Table view, that will be managed by DTTableViewManager
    var tableView : UITableView! { get }
}
public protocol DTCollectionViewManageable : class
{
    /// Collection view, that will be managed by DTCollectionViewManager
    var collectionView : UICollectionView? { get }
}
@DenTelezhkin
Copy link
Owner

Hey!
The reasons are actually funny - initially i was aligning those protocols with UICollectionViewController and UITableViewController, which had different interfaces in Swift 2 and 3, tableView was implicitly unwrapped and collectionView was optional.

In Xcode 10 however, both are implicitly unwrapped, and collectionView is no longer optional. Undoing this protocol requirement though would be a breaking change for all users of the framework, so to work around this issue I created one additional protocol DTCollectionViewNonOptionalManageable, which you can use for non-optional collectionView. Name is definitely ugly, ideally i would want to actually have one protocol, but I don't want to make a breaking release just to change one protocol name.

Can you leave this issue open for now, please?

@onsissond
Copy link
Author

Hey!
The reasons are actually funny - initially i was aligning those protocols with UICollectionViewController and UITableViewController, which had different interfaces in Swift 2 and 3, tableView was implicitly unwrapped and collectionView was optional.

In Xcode 10 however, both are implicitly unwrapped, and collectionView is no longer optional. Undoing this protocol requirement though would be a breaking change for all users of the framework, so to work around this issue I created one additional protocol DTCollectionViewNonOptionalManageable, which you can use for non-optional collectionView. Name is definitely ugly, ideally i would want to actually have one protocol, but I don't want to make a breaking release just to change one protocol name.

Can you leave this issue open for now, please?

Thank you very much!

@DenTelezhkin
Copy link
Owner

Hi!

I've replaced DTCollectionViewNonOptionalManageable protocol with additional property on DTCollectionViewManageable protocol in ecd9c31. Once released, DTTableViewManager and DTCollectionVIewManager will be consistent on optionality of table/collection views, and will not rely on UIKit or Swift changes.

However, specifically for DTCollectionViewManager, it's a breaking change, that will lead to crashes if code is not updated(collectionView property in protocol is now implicitly unwrapped instead of optional). Because of that I will be rolling this functionality in the major release somewhere in the fall, after iOS 13 release with other features of iOS 13 and Xcode 11.

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

No branches or pull requests

2 participants