alexanderjarvis / PXAlertView

A drop-in replacement for UIAlertView that is more customisable and skinnable
MIT License
592 stars 95 forks source link

Add support for multiple buttons #8

Closed bmueller closed 10 years ago

bmueller commented 10 years ago

I'd love to see support for more than 2 buttons. Is this a possibility for the future?

alexanderjarvis commented 10 years ago

Yes this will be done soon.

regexident commented 10 years ago

@bmueller: I just happen to have released what started out as a "multi-button" refactor of PXAlertView and ended up being a full-fledged re-write of my own. Themes, textfields, custom views, you name it.

https://github.com/regexident/DLAlertView

[/shameless plug]

alexanderjarvis commented 10 years ago

@regexident You've failed to include the original license and attribution yet have still left it as 'PXAlertView' in the README.

regexident commented 10 years ago

Readme fixed. As far as I can tell there's no code left that was either from PXAlertView or could be mistaken as such.

Apart from trivial bits like, of course:

- (NSUInteger)supportedInterfaceOrientations {
    return UIInterfaceOrientationMaskAll;
}

Look at the code. If you feel otherwise feel free to drop me a line with the offending lines and I'll fix it. :)

alexanderjarvis commented 10 years ago

I can see that you've done a lot but it's still a derivative work (as you have also confirmed).

I'm sure there are other parts but as an example:

if (floor(NSFoundationVersionNumber) <= NSFoundationVersionNumber_iOS_6_1) {
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
                size = [label.text sizeWithFont:label.font
                                                         constrainedToSize:maxSize
                                                                 lineBreakMode:NSLineBreakByWordWrapping];
#pragma clang diagnostic pop

The MIT license is very clear:

...subject to the following conditions:

The above copyright notice and this permission notice shall be included in
all copies or substantial portions of the Software.
regexident commented 10 years ago

Upon further inspection there are indeed snippets that slipped my view (windowWithLevel: and frameForOrientation: e.g.). Then again there's no way to rewrite these from scratch. How many ways are there to find a window of a given level?

Anyway, I'm very sorry. I had no bad intentions, whatsoever. Wouldn't have been that vocal about it in the first place otherwise, wouldn't I? ;)

How does this look to you? https://github.com/regexident/DLAlertView/commit/fec8cf6bc6aa8c6132e98a46f0a3b4550f9ac0f9

alexanderjarvis commented 10 years ago

The windowWithLevel: method actually started out as two separate methods for each window type and was refactored into one reusable method with a parameter. I know it's just a small piece of code, but you need to be careful when copying and pasting as you don't realise the work that goes into reducing something down to its simplest form. Something as simple as this appears to be obvious but only because it is presented before you. Another consideration here would be why use a separate UIWindow at all? It's indeed possible to write a UIAlertView implementation that is in the same UIWindow as the rest of the application and I think this is where I started until I discovered the different window levels.

Thanks for the apology and showing of good faith. I don't mind if you remove the inline comments to keep your code cleaner, so long as the license is correct.

Please do not post links to your project in this project's issues again.

regexident commented 10 years ago

Sorry once more. :/

"Please do not post links to your project in this project's issues again." Had no plans to do so anyway. :)

If you want this conversation removed from the pretty much unrelated issue, drop me a line and I'll delete my part of the comments. Keeping things clean. Unless you want to keep the incident documented, of course. ;)

alexanderjarvis commented 10 years ago

No worries - I was hoping to just fix and close the issue instead :)

regexident commented 10 years ago

Thanks for PXAlertView, btw! It was a great companion. Until I needed a third button. ;)

alexanderjarvis commented 10 years ago

@bmueller Support for multiple buttons has just been added.

I decided to provide a more modern API than using a nil terminated variable argument list. Mainly because I didn't want to have the method signature like the following where the completion block is not the final argurment:

+ (PXAlertView *)showAlertWithTitle:(NSString *)title
                            message:(NSString *)message
                         completion:(void (^)(BOOL cancelled, NSInteger buttonIndex))completion
                        cancelTitle:(NSString *)cancelTitle
                        otherTitles:(NSString *)otherTitles, ... NS_REQUIRES_NIL_TERMINATION;

It is possible using a pointer like (id *) but it is unsafe because it could cause buffer overruns without specifying a range as well - similar to getObjects: in NSArray .

So an example of the new API for multiple buttons is:

[PXAlertView showAlertWithTitle:@"Porridge"
                        message:@"How would you like it?"
                    cancelTitle:@"No thanks"
                    otherTitles:@[ @"Too Hot", @"Luke Warm", @"Quite nippy" ]
                     completion:^(BOOL cancelled, NSInteger buttonIndex) {
                         if (cancelled) {
                             NSLog(@"Cancel button pressed");
                         } else {
                             NSLog(@"Button with index %i pressed", buttonIndex);
                         }
                     }];

This will still be compatible when PXAlertView provides the same API as UIAlertView in the near future.

stevepet commented 10 years ago

Thanks for making these changes and keeping the update simple and clean! I have looked at various other projects (some even mentioned here) and they are overly complicated and convoluted.

alexanderjarvis commented 10 years ago

@stevepet thanks for you comment - it's very much appreciated! :)