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

ctb-ios-changes #78

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

ctb-ios-changes #78

wants to merge 1 commit into from

Conversation

kmfree
Copy link

@kmfree kmfree commented Sep 18, 2016

@kmfree kmfree changed the title Added changes for latest code. ctb-ios-changes Sep 18, 2016
@braebot
Copy link
Member

braebot commented Oct 4, 2016

@josharian or @dgoldman-ebay, do you have any feedback on this PR?

@@ -1016,7 +1016,7 @@ + (BOOL)cardExpiryIsValid:(CardIOCreditCardInfo*)info {
}

// we are under the assumption of a normal US calendar
NSCalendar *cal = [[NSCalendar alloc] initWithCalendarIdentifier:NSCalendarIdentifierGregorian];
NSCalendar *cal = [[NSCalendar alloc] initWithCalendarIdentifier:NSGregorianCalendar];
Copy link
Member

Choose a reason for hiding this comment

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

if(!result.usable) {

if(!result.usable)
{
Copy link
Member

Choose a reason for hiding this comment

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

The style used within this codebase is to keep opening braces on the previous line. Please stick to this style. (Doesn't apply, of course, when you bring whole new libraries into the project.)

@@ -504,7 +504,7 @@ - (void)cardIOView:(CardIOView *)cardIOView didScanCard:(CardIOCreditCardInfo *)
dataEntryViewController.manualEntry = self.context.suppressScannedCardImage;

CGPoint newCenter = [self.view convertPoint:cardIOView.transitionView.cardView.center fromView:cardIOView.transitionView];
newCenter.y -= NavigationBarHeightForOrientation([[UIApplication sharedApplication] statusBarOrientation]);
newCenter.y -= NavigationBarHeightForOrientation(self.interfaceOrientation);
Copy link
Member

Choose a reason for hiding this comment

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

Orientation stuff is tricky and often runs into issues with unexpected edge cases. I hope that you've tried every possible experiment!

@dgoldman-pdx
Copy link
Member

I've added a few extremely minor comments. Otherwise, it looks like this huge PR mostly comprises a couple of large libraries, on which I really can't comment. I hope that they don't increase the shipped size of card.io terribly much...

At first, you might want to branch card.io, with a new branch that incorporates these and future related changes, and make both versions of card.io available to clients.

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.

3 participants