-
Notifications
You must be signed in to change notification settings - Fork 84
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
UINavigationBar wrappers #34
Conversation
Codecov Report
@@ Coverage Diff @@
## master #34 +/- ##
==========================================
+ Coverage 99.74% 99.74% +<.01%
==========================================
Files 10 11 +1
Lines 385 393 +8
==========================================
+ Hits 384 392 +8
Misses 1 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.
thanks for adding these in! been on my list of to-dos
#endif | ||
|
||
public typealias UnderlineStyle = NSUnderlineStyle | ||
public typealias StrikethroughStyle = NSUnderlineStyle | ||
public typealias ParagraphStyle = NSParagraphStyle | ||
public typealias DrawingContext = NSStringDrawingContext |
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.
could we remove all these new typealiases since they're not used?
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.
could we remove all these new typealiases since they're not used?
I planned to use them for some new functions but then I moved them to another pull request.
#elseif os(watchOS) | ||
#else | ||
import UIKit | ||
public typealias NavigationBar = UINavigationBar |
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 don't think we need the typealias since there's no conflicting navigation bar type
#else | ||
extension NavigationBar { | ||
|
||
public var swiftyTitleTextAttributes: [Attribute]? { |
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.
could you add some documentation to both of these variables? thanks 🙏
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.
could you add some documentation to both of these variables? thanks 🙏
Added description for these new properties, if you want you can add something better.
|
||
#if os(macOS) | ||
#elseif os(watchOS) | ||
#else |
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 think you could just extend the above macros for the entire file so that you don't need to repeat them
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.
Looks great 👍🏻
@RomanPodymov do you have more PRs coming? If you do I'll hold off on releasing a new version and just bundle in all those changes. |
I added an extension for |
Hello.
In this pull request I added wrappers for
UINavigationBar.titleTextAttributes
andUINavigationBar.largeTitleTextAttributes
.Use cocoapods 1.7.0.