-
Notifications
You must be signed in to change notification settings - Fork 224
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
Darkmode #216
base: master
Are you sure you want to change the base?
Conversation
Yes!! :) |
|
||
extension UIColor { | ||
// Adaptable Colors | ||
static var SystemBlue : UIColor {get {if #available(iOS 13.0, *){return .systemBlue} else {return UIColor.color(0, green: 122, blue: 255)}}} |
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.
How about camelCasing
for color names systemBlue
instead of SystemBlue
?
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.
I believe the reason why @lkeude96 implemented it this way is because if you do camel case such as systemBlue
there will be a naming clash with Apples UIColor systemBlue
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.
yes precisely what @croossin mentionned. I noticed that Black
was previously capitalized so i used the same pattern
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.
I also explored adding them in a nested struct.
Is there anything holding this back from being merged? 😯 |
@TyJPhillips Just fixing up some last second stuff |
I believe we are good to go now!! @badrinathvm Feel free to take another look 👍 |
And a huge shoutout to @lkeude96 for all of his contributions !!!!! |
@croossin Looks Good to me. |
Any plans to merge this? |
1 similar comment
Any plans to merge this? |
Before you make a Pull Request, read the important guidelines:
Issue Link 🔗 #184
Goals of this PR 🎉
How Has This Been Tested 🔍
Please let us know if you have tested your PR and if we need to reproduce the issues. Also, please let us know if we need any relevant information for running the tests.
Test Configuration 👾
Things to check on 🎯