-
Notifications
You must be signed in to change notification settings - Fork 0
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
Adds back in a default shadow to CollectionItemViewCell
#292
Adds back in a default shadow to CollectionItemViewCell
#292
Conversation
…Cell` to use thunderbasic's `ShadowComponents` struct
@@ -28,13 +29,25 @@ open class CircleProgressView: UIView { | |||
} | |||
} | |||
|
|||
/// Apply default shadow configuration | |||
/// Apply shadow configuration based on `shadowComponents` property | |||
public var hasShadow = true { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think we can remove this and just use nullability of ShadowComponents
? 😃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh I see your last comment, breaking change... Hmm would just have to handle accordingly in updateShadow
as you have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my comment on your original comment, the only problem is if we do that it's technically a breaking change... and I'm not a massive fan of doing a major release for something so small after we've literally just made it to v3.0.1
😂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though as it's CircleProgressView
wonder if anyone is using hasShadow
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But yeah, more than happy to leave 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably nobody, but I want to be strict on ourselves so if we do move to semantic versioning in the future then we're well-practiced at not causing ourselves any headaches :D
Description
This adds back in a shadow to collection item view cell that seems to have been removed at the time that blended learning was merged into develop. I can't recall any legitimate reason for this removal, so this PR is re-instating the exact same shadow that was present before
Motivation and Context
Solves shadows not appearing where they should on
CollectionItemViewCell
How Has This Been Tested?
Tested running this branch locally in ARC First Aid
Screenshots (if appropriate):
Types of changes
Checklist: