README and documentationIGListKit version: main branchIGListCollectionView has a hardcoded background color specified:
- (void)commonInit {
self.backgroundColor = [UIColor whiteColor];
self.alwaysBounceVertical = YES;
}
Can it be removed? Having it hard coded does not play well with the UIAppearance APIs.
Hmm we did remove the background color setter in #285, but I'm not sure how this effects UIAppearance. At what stage in the view's lifecycle does UIAppearance actually set the background color?
Here's how to demonstrate the issue. In the sample app, add the following to the App Delegate:
UICollectionView.appearance().backgroundColor = UIColor.red
You'll see that none of the backgrounds are red except for the Storyboard example. Next, comment out the hard coded background color in IGListCollectionView:
- (instancetype)initWithFrame:(CGRect)frame collectionViewLayout:(UICollectionViewLayout *)layout {
if (self = [super initWithFrame:frame collectionViewLayout:layout]) {
// self.backgroundColor = [UIColor whiteColor];
self.alwaysBounceVertical = YES;
}
return self;
}
You'll see all the background now pick up the UIAppearance background color.
Some pretty old code, but here's how to accommodate UIAppearance in subclasses:
https://github.com/jessesquires/BButton/blob/develop/BButton/Classes/BButton.m#L188-L201
It's pretty easy. We should probably fix this for 2.0. cc @rnystrom
@jeffbailey -- would you want to submit a PR for this?
Yes, I'd be happy to submit a pull request. First, I guess I expected the outcome of this would be to just remove the background color from the ctor since it doesn't seem to add much value. Am I missing something?
@jessesquires were you thinking this is the appropriate fix:
- (instancetype)initWithFrame:(CGRect)frame collectionViewLayout:(UICollectionViewLayout *)layout {
if (self = [super initWithFrame:frame collectionViewLayout:layout]) {
UIColor *backgroundAppearanceColor = [[[self class] appearance] backgroundColor];
if (!backgroundAppearanceColor) {
self.backgroundColor = [UIColor whiteColor];
}
self.alwaysBounceVertical = YES;
}
return self;
}
Or were you thinking about overriding the getter for background color and doing the following:
I think we'd need a shadow backgroundColor property in IGListCollectionView to support that approach.
@jeffbailey personally I like the example you have, @jessesquires should weigh in too
Also we're pushing to cut 2.0.0 this week, so if you don't have time to PR this in the next day or two let us know!
@jeffbailey @rnystrom
Or were you thinking about overriding the getter for background color
I'm pretty sure this is how we have to do it. Doing this via init will have the same problem. You can change UIAppearance at any time.
I think we'd need a shadow backgroundColor property
I don't think so, but maybe. We should try without.
Actually -- backgroundColor is inherited from UICollectionView. I'm surprised this isn't supported automatically. Are we sure this is a bug?
Yeah, it's definitely an issue that can be reproduced in the sample project as described above. I submitted a pull request for handling it in the ctor, but you're correct about that only working if UIAppearance is setup before that.
I'd be happy to submit a pull request for the shadow property if you want.
You know, we should have added unit tests for both the storyboard as well as UIAppearance proxy changes. Filing an issue.
Most helpful comment
Here's how to demonstrate the issue. In the sample app, add the following to the App Delegate:
UICollectionView.appearance().backgroundColor = UIColor.redYou'll see that none of the backgrounds are red except for the Storyboard example. Next, comment out the hard coded background color in IGListCollectionView:
You'll see all the background now pick up the UIAppearance background color.