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

Ensure a supplementary view is of kind _ASCollectionReusableView before setting its layoutAttributes #32

Conversation

nguyenhuy
Copy link
Member

Fixes #29.

@nguyenhuy nguyenhuy force-pushed the HNASCollectionReuseableViewCrash branch from 55839e8 to 3491495 Compare April 18, 2017 19:18
@nguyenhuy
Copy link
Member Author

@ay8s Could you please help test this diff? Thank you!

@ay8s
Copy link
Collaborator

ay8s commented Apr 18, 2017

Working for me @nguyenhuy! 👍

@nguyenhuy nguyenhuy force-pushed the HNASCollectionReuseableViewCrash branch from 3491495 to f39da70 Compare April 19, 2017 10:19
Copy link
Member

@Adlai-Holler Adlai-Holler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm if we're not sure that the supplementary view is an _ASCollectionReusableView – we should investigate further why that's happening because it's not expected – we should change this method signature to willDisplaySupplementaryView:(UICollectionReusableView *) and then class-check, and then cast before setting the layout attributes.

@nguyenhuy
Copy link
Member Author

@Adlai-Holler It's not an _ASCollectionResuableView under IGListKit mode, as well as interop (here).

@Adlai-Holler
Copy link
Member

Under IGListKit it should be an _ASCollectionReusableView and good catch, I had forgotten we support interop for supplementaries.

@plarson
Copy link
Contributor

plarson commented Apr 19, 2017

Shouldn't this be fixed inside -[ASIGListSupplementaryViewSourceMethods viewForSupplementaryElementOfKind:atIndex:sectionController]?

@plarson
Copy link
Contributor

plarson commented Apr 19, 2017

I made this pull request, it's a fix for the original issue: #40

@nguyenhuy
Copy link
Member Author

Closing in favor of #40. Thanks, @plarson!

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

Successfully merging this pull request may close these issues.

4 participants