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

Why MagazineLayoutCollectionViewLayoutAttributes is final? #67

Open
AntonBelousov opened this issue Jan 29, 2020 · 3 comments
Open

Why MagazineLayoutCollectionViewLayoutAttributes is final? #67

AntonBelousov opened this issue Jan 29, 2020 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@AntonBelousov
Copy link

The problem
I use UICollectionView with custom layout in my app. I wanted to migrate to MagazineLayout. But I use custom UICollectionViewLayoutAttributes, so I need to somehow extend MagazineLayoutCollectionViewLayoutAttributes to use with my cells.
But since MagazineLayoutCollectionViewLayoutAttributes is final - I can't just subclass it.

The solution I'd like
Make it non-final

Alternatives I've considered
a) Make MagazineLayout non-final, and let subclasses provide type of UICollectionViewLayoutAttributes which conforms to protocol:

protocol IMagazineLayoutCollectionViewLayoutAttributes {
    public var shouldVerticallySelfSize: Bool { get set}
}

Make MagazineLayoutCollectionViewLayoutAttributes conform this protocol :

extension MagazineLayoutCollectionViewLayoutAttributes: IMagazineLayoutCollectionViewLayoutAttributes {}

b) Use objc runtime to extend MagazineLayoutCollectionViewLayoutAttributes
c) Add property into MagazineLayoutCollectionViewLayoutAttributes (some key-value collection), to hold additional parameters

@bryankeller bryankeller self-assigned this Feb 6, 2020
@bryankeller bryankeller added the enhancement New feature or request label Feb 6, 2020
@bryankeller
Copy link
Contributor

@AntonBelousov I think, in general, MagazineLayout probably shouldn't enforce anything to be final. This was leftover from when MagazineLayout was used only internally at Airbnb, but now that it's public, I don't think forcing things to be final is really helping.

With that said, I think this change will take more than just making MagazineLayoutCollectionViewLayoutAttributes open - if you look at the source for MagazineLayout.swift, you'll see several places that MagazineLayoutCollectionViewLayoutAttributes is referenced. If you have a custom layout attributes subclass, I'm not sure how it will integrate with with MagazineLayout. For example, in the prepare function, we make instances of MagazineLayoutCollectionViewLayoutAttributes - how would we replace this with your custom instance?

May I ask what your end goal is? What properties are you using in your custom layout attributes subclass?

@AntonBelousov
Copy link
Author

AntonBelousov commented Feb 6, 2020

For example, in the prepare function, we make instances of MagazineLayoutCollectionViewLayoutAttributes - how would we replace this with your custom instance?

There are only 6 places in code where an instance of MagazineLayoutCollectionViewLayoutAttributes is created: 2 for cells, 4 for supplementary views.

I can suggest
a) replacing a direct call init methods with a methods call that can be overridden

func newAttributes(forCellWith: IndexPath) -> MagazineLayoutCollectionViewLayoutAttributes {
...
}

or

b) calling initializer like this:

let attribuesType = type(of: self).layoutAttributesClass as! MagazineLayoutCollectionViewLayoutAttributes.Type
        let attrs = attribuesType.init(forCellWith: ...)

May I ask what your end goal is? What properties are you using in your custom layout attributes subclass?

I have some cells that may change their appearance/layout depending on the context. Now it’s convenient for me to pass context through attributes

@bryankeller
Copy link
Contributor

@AntonBelousov sorry for the delay - I think making MagazineLayout open, marking most of the functions as final, and having an open function called makelayoutAttributes(forCellAtIndexPath indexPath: IndexPath) -> MagazineLayoutCollectionViewLayoutAttributes seems like a reasonable change. If this is something you still need / have already implemented, feel free to make a pull request with the changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants