x2on / simple-share-ios

Simple-Share for iOS is an easy drop-in library for sharing. (Facebook, Twitter, Mail, Safari)
Apache License 2.0
28 stars 13 forks source link

Mailfixes #2

Closed markrickert closed 12 years ago

markrickert commented 12 years ago

Fix some mail related issues and implements a demo in the demo project for using the SimpleMailShare class.

x2on commented 12 years ago

What are the fixes?

markrickert commented 12 years ago

Mostly just adding the mail demo to the demo project. It demonstrates that you need to hold onto the mail composer variable because if you run:

SimpleMailShare *simpleMailShare = [[SimpleMailShare alloc] init];
if ([simpleTwitterShare canSendMail]) {
    [simpleMailShare shareText:@"" subject:@"" isHTML:NO];
}

You get a crash because the memory is released after the modal view is shown and then when you go to dismiss it, the delegate is nil.

You can look at my commits and see what I did.

x2on commented 12 years ago

Ok thanks for that hint. I would only merge this if you cleanup the code and remove the other code. If you won't do that i can fix that.

markrickert commented 12 years ago

I'm not sure what you mean buy "remove the other code". This is a clean pull request.

x2on commented 12 years ago

There is many other stuff in this pull request...

There are changes in AppDelegate.m and SimpleMailShare.h and SimpleMailShare.m there is a new property instead of an ivar.

Also the different nibs for iPad and iPhone were merged in your commits.

If you remove that from your pull request i can merge this...

markrickert commented 12 years ago

Right - that was on purpose... there's absolutely no need to have two xib file in the code when they can autoscale on the correct sizes based on the device screen. We're talking about 3 buttons here, and the code works great on both iPhone and iPad screen sizes.

markrickert commented 12 years ago

I think you'll find that it works beautifully with one less file in the project with my changes.

x2on commented 12 years ago

At this moment were are talking about 3 buttons, but im working on some different features + layout for iPad and iPhone (more sharing options...)

And what about the property? I don't see that this change is needed... And the getRootViewController is also not needed.

markrickert commented 12 years ago

Wahtever.

x2on commented 12 years ago

I added the changes without that stuff.