-
Notifications
You must be signed in to change notification settings - Fork 610
Adds opacity slider option to color panel #155
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
Conversation
is this ready/do you need a reviewer? |
I need a reviewer. I can't assign reviewers :( |
Hopefully this is fixed now. I thought just having you listed as a collaborator would give you the ability to request reviews, but it sounds like not; you have write access now which should do it. |
Comments addressed, PTAL! Also, I can assign reviewers now, 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.
Just some small nits, then this is good to go!
@@ -33,7 +33,12 @@ - (NSString *)channel { | |||
- (void)handleMethodCall:(FLEMethodCall *)call result:(FLEMethodResult)result { | |||
BOOL handled = YES; | |||
if ([call.methodName isEqualToString:@(plugins_color_panel::kShowColorPanelMethod)]) { | |||
[self showColorPanel]; | |||
BOOL showAlpha = YES; | |||
if ([call.arguments isKindOfClass:[NSDictionary class]]) { |
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 this isn't true then the call is malformed, so I'd prefer that if the check fails the code NSLog an error message, then call result(FLEMethodNotImplemented);
and return early.
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.
SG
Done, PTAL! |
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.
LGTM modulo re-running clang-format after the latest changes.
Feel free to submit once you upload that change. Remember to do "squash and merge" so that you're submitting a single change instead of all the incremental patches. (You'll need to clean up the commit message in the resulting text field since by default it'll just stick together the commit messages of all the incremental patches, which isn't very useful.)
Left it as an optional parameter to mimic the native color choosers