-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Exposed NavigationDrawerController methods for objc #1248
Exposed NavigationDrawerController methods for objc #1248
Conversation
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.
Awesome!
Great! But is necessary to add to some other methods:
|
@OrkhanAlikhanov I will wait to merge this in until all the necessary function declarations have been updated. |
Hey @emauro! I added
|
@OrkhanAlikhanov Is this something we want to do to other classes across the board? As well, remove any |
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.
Nice cleanup version of the same affect.
@@ -132,6 +132,7 @@ public protocol NavigationDrawerControllerDelegate { | |||
} | |||
|
|||
@objc(NavigationDrawerController) | |||
@objcMembers |
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.
Hey! Can we add this feature to more classes and cleanup the single @objc
declarations? As well to the other frameworks?
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.
If @emauro confirms that it works properly, we surely can!
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.
Cool, so let's wait for that. Thank you!
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.
For me it is ok. @objc permits to fine grain the access to class methods but I think here it is not an issue.
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.
@emauro so you tested it and it works?
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.
@DanielDahan Yes!
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.
Great, then we will add this where needed and make a new release. Thank you!
Adding
@objc
s for #1247