Skip to content
This repository has been archived by the owner on Jan 12, 2019. It is now read-only.

SampleApp: Preview cardImage and support iPhone 6/6+ #124

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cybertk
Copy link

@cybertk cybertk commented Jul 12, 2015

No description provided.

@dgoldman-pdx
Copy link
Member

@cybertk in general we'd like to keep our SampleApp very simple.

Could you please explain your reasons for suggesting these changes?

@cybertk
Copy link
Author

cybertk commented Jul 13, 2015

The reasons are

  • Current SampleApp does not support iPhone 6/6+. It displayed ugly on those newer devices.
  • CardIO has CreditCardInfo.cardImage support, and I cannot preview this feature when I run pod try or start the Demo app from Xcode.

@@ -46,6 +48,8 @@ - (void)userDidProvideCreditCardInfo:(CardIOCreditCardInfo *)info inPaymentViewC
[self dismissViewControllerAnimated:YES completion:nil];

self.infoLabel.text = [NSString stringWithFormat:@"Received card info. Number: %@, expiry: %02lu/%lu, cvv: %@.", info.redactedCardNumber, (unsigned long)info.expiryMonth, (unsigned long)info.expiryYear, info.cvv];
self.logoImage.image = info.cardImage;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems odd. Why isn't setting snapshotImage enough?

@josharian
Copy link
Member

I'm definitely in favor of supporting the iPhone 6/6+. Thanks for doing that.

The cardImage feature is much less commonly used. I think I'm with @dgoldman-ebay on this one: I'd rather keep the sample app extremely simple.

Does your first commit (enabling iPhone 6/6+) work correctly if applied without the second commit?

@hkareemllc
Copy link

I second @josharian

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants