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
[TC-5357] Support destructive alert style #6697
Conversation
@@ -134,7 +136,7 @@ -(void)show:(id)args | |||
NSString* btnName = [TiUtils stringValue:btn]; | |||
if (!IS_NULL_OR_NIL(btnName)) { | |||
UIAlertAction* theAction = [UIAlertAction actionWithTitle:btnName | |||
style:((curIndex == cancelIndex) ? UIAlertActionStyleCancel : UIAlertActionStyleDefault) | |||
style:((curIndex == cancelIndex) ? UIAlertActionStyleCancel : (curIndex == destructiveIndex) ? UIAlertActionStyleDestructive :UIAlertActionStyleDefault) |
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.
This line is a bit confusing and not easy to follow due to the multiple ternary operators. If would be better to spread the out for clarity purposes, something like this:
UIAlertActionStyle style;
if (curIndex == cancelIndex) {
style = UIAlertActionStyleCancel;
} else if (curIndex == destructiveIndex) {
style = UIAlertActionStyleDestructive;
} else {
style = UIAlertActionStyleDefault;
}
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.
Also, if this is iOS 8 an above only, make sure you check for that with:
if ([TiUtils isIOS8OrGreater]) { ..... }
PR reviewed, left a couple of comments. |
Thank you! Comments are reasonable, will have a look at it! |
Updated docs and code readability as well as a bounding-check like it is done with the cancel-button. I think the iOS 8 check is not necessary in this case, because the code is already in the conditional iOS 8 block to support the UIAlertController. |
One more thing... the destructive index is missing from the click event. For consistency, add it here as well
|
Added the destructive index and implemented a quick sample example code to test: var win = Ti.UI.createWindow({
backgroundColor: "#fff"
});
var button = Ti.UI.createButton({
title: "Show destructive dialog"
});
var dialog = Ti.UI.createAlertDialog({
title: "Proceed",
message: "Do you really want to proceed?",
destructive: 0,
cancel: 1,
buttonNames: ["Destructive", "Cancel", "Proceed"]
});
button.addEventListener("click", function() {
dialog.show();
});
dialog.addEventListener("click", function(e) {
Ti.API.info(e);
});
win.add(button);
win.open(); Output in the console:
|
i have opened a new PR with your commits for Titanium master branch #6842 |
There is one minor thing missing in the documentation. The
|
Closing this outdated PR in favor of #6842 |
JIRA reference: https://jira.appcelerator.org/browse/TC-5357