Fix bugs around STPPaymentCardTextField becomeFirstResponder

Stripe/STPPaymentCardTextField.m

@@ -10,6 +10,7 @@
 
 #import "STPPaymentCardTextField.h"
 
+#import "NSArray+Stripe.h"
 #import "NSString+Stripe.h"
 #import "STPFormTextField.h"
 #import "STPImageLibrary.h"
@@ -447,42 +448,35 @@ - (void)setEnabled:(BOOL)enabled {
 #pragma mark UIResponder & related methods
 
 - (BOOL)isFirstResponder {
-    return [self.currentFirstResponderField isFirstResponder];
+    return self.currentFirstResponderField != nil;
 }
 
 - (BOOL)canBecomeFirstResponder {
-    return [[self nextFirstResponderField] canBecomeFirstResponder];
+    STPFormTextField *firstResponder = [self currentFirstResponderField] ?: [self nextFirstResponderField];
+    return [firstResponder canBecomeFirstResponder];
 }
 
 - (BOOL)becomeFirstResponder {
-    return [[self nextFirstResponderField] becomeFirstResponder];
+    STPFormTextField *firstResponder = [self currentFirstResponderField] ?: [self nextFirstResponderField];
+    return [firstResponder becomeFirstResponder];
 }
 
-- (STPFormTextField *)nextFirstResponderField {
-    STPFormTextField *currentSubResponder = self.currentFirstResponderField;
-    if (currentSubResponder) {
-        NSUInteger index = [self.allFields indexOfObject:currentSubResponder];
+- (nonnull STPFormTextField *)nextFirstResponderField {
+    STPFormTextField *currentFirstResponder = [self currentFirstResponderField];
+    if (currentFirstResponder) {
+        NSUInteger index = [self.allFields indexOfObject:currentFirstResponder];
         if (index != NSNotFound) {
-            index += 1;
-            if (self.allFields.count > index) {
-                STPFormTextField *nextField = self.allFields[index];
-                if (nextField == self.postalCodeField
-                    && !self.postalCodeEntryEnabled) {
-                    return [self firstInvalidSubField];
-                }
-                else {
-                    return nextField;
-                }
+            STPFormTextField *nextField = [self.allFields stp_boundSafeObjectAtIndex:index + 1];
+            if (self.postalCodeEntryEnabled || nextField != self.postalCodeField) {
+                return nextField;
             }
         }
-        return [self firstInvalidSubField];
-    }
-    else {
-        return [self firstInvalidSubField];
     }
+
+    return [self firstInvalidSubField] ?: [self lastSubField];
 }
 
-- (STPFormTextField *)firstInvalidSubField {
+- (nullable STPFormTextField *)firstInvalidSubField {
     if ([self.viewModel validationStateForField:STPCardFieldTypeNumber] != STPCardValidationStateValid) {
         return self.numberField;
     }
@@ -501,6 +495,10 @@ - (STPFormTextField *)firstInvalidSubField {
     }
 }
 
+- (nonnull STPFormTextField *)lastSubField {
+    return self.postalCodeEntryEnabled ? self.postalCodeField : self.cvcField;
+}
+
 - (STPFormTextField *)currentFirstResponderField {
     for (STPFormTextField *textField in [self allFields]) {
         if ([textField isFirstResponder]) {
@@ -1283,6 +1281,7 @@ - (void)formTextFieldTextDidChange:(STPFormTextField *)formTextField {
                 }
             }
 
+            // This is a no-op if this is the last field & they're all valid
             [[self nextFirstResponderField] becomeFirstResponder];
             break;
         }

Tests/Tests/STPPaymentCardTextFieldTest.m

@@ -23,6 +23,7 @@ @interface STPPaymentCardTextField (Testing)
 @property (nonatomic, readwrite, weak) STPFormTextField *numberField;
 @property (nonatomic, readwrite, weak) STPFormTextField *expirationField;
 @property (nonatomic, readwrite, weak) STPFormTextField *cvcField;
+@property (nonatomic, readwrite, weak) STPFormTextField *postalCodeField;
 @property (nonatomic, readonly, weak) STPFormTextField *currentFirstResponderField;
 @property (nonatomic, readwrite, strong) STPPaymentCardTextFieldViewModel *viewModel;
 @property (nonatomic, copy) NSNumber *focusedTextFieldForLayout;
@@ -394,7 +395,7 @@ - (void)testSetCard_allFields_whileEditingNumber {
     XCTAssertEqualObjects(self.sut.numberField.text, number);
     XCTAssertEqualObjects(self.sut.expirationField.text, @"10/99");
     XCTAssertEqualObjects(self.sut.cvcField.text, cvc);
-    XCTAssertFalse([self.sut isFirstResponder]);
+    XCTAssertFalse([self.sut isFirstResponder], @"after `setCardParams:`, if all fields are valid, should resign firstResponder");
     XCTAssertTrue(self.sut.isValid);
 }
 
@@ -415,7 +416,7 @@ - (void)testSetCard_partialNumberAndExpiration_whileEditingExpiration {
     XCTAssertEqualObjects(self.sut.numberField.text, number);
     XCTAssertEqualObjects(self.sut.expirationField.text, @"10/99");
     XCTAssertEqual(self.sut.cvcField.text.length, (NSUInteger)0);
-    XCTAssertTrue([self.sut.numberField isFirstResponder]);
+    XCTAssertTrue([self.sut.numberField isFirstResponder], @"after `setCardParams:`, when firstResponder becomes valid, first invalid field should become firstResponder");
     XCTAssertFalse(self.sut.isValid);
 }
 
@@ -434,7 +435,7 @@ - (void)testSetCard_number_whileEditingCVC {
     XCTAssertEqualObjects(self.sut.numberField.text, number);
     XCTAssertEqual(self.sut.expirationField.text.length, (NSUInteger)0);
     XCTAssertEqual(self.sut.cvcField.text.length, (NSUInteger)0);
-    XCTAssertTrue([self.sut.cvcField isFirstResponder]);
+    XCTAssertTrue([self.sut.cvcField isFirstResponder], @"after `setCardParams:`, if firstResponder is invalid, it should remain firstResponder");
     XCTAssertFalse(self.sut.isValid);
 }
 
@@ -454,7 +455,7 @@ - (void)testSetCard_empty_whileEditingNumber {
     XCTAssertEqual(self.sut.numberField.text.length, (NSUInteger)0);
     XCTAssertEqual(self.sut.expirationField.text.length, (NSUInteger)0);
     XCTAssertEqual(self.sut.cvcField.text.length, (NSUInteger)0);
-    XCTAssertTrue([self.sut.numberField isFirstResponder]);
+    XCTAssertTrue([self.sut.numberField isFirstResponder], @"after `setCardParams:` that clears the text fields, the first invalid field should become firstResponder");
     XCTAssertFalse(self.sut.isValid);
 }
 
@@ -486,4 +487,63 @@ - (void)testIsValidKVO {
     [self waitForExpectationsWithTimeout:2 handler:nil];
 }
 
+- (void)testBecomeFirstResponder {
+    XCTAssertTrue([self.sut canBecomeFirstResponder]);
+    XCTAssertTrue([self.sut becomeFirstResponder]);
+    XCTAssertTrue(self.sut.isFirstResponder);
+
+    XCTAssertEqual(self.sut.numberField, self.sut.currentFirstResponderField);
+
+    [self.sut becomeFirstResponder];
+    XCTAssertEqual(self.sut.numberField, self.sut.currentFirstResponderField,
+                   @"Repeated calls to becomeFirstResponder should not change the firstResponder");
+
+    self.sut.numberField.text = @"4242" "4242" "4242" "4242";
+
+    XCTAssertEqual(self.sut.expirationField, self.sut.currentFirstResponderField,
+                   @"Once numberField is valid, firstResponder should move to the next field (expiration)");
+
+    XCTAssertTrue([self.sut.cvcField becomeFirstResponder]);
+    XCTAssertEqual(self.sut.cvcField, self.sut.currentFirstResponderField,
+                   @"We don't block other fields from becoming firstResponder");
+
+    XCTAssertTrue([self.sut becomeFirstResponder]);
+    XCTAssertEqual(self.sut.cvcField, self.sut.currentFirstResponderField,
+                   @"Calling becomeFirstResponder does not change the currentFirstResponder");
+
+    self.sut.expirationField.text = @"10/99";
+    self.sut.cvcField.text = @"123";
+
+    XCTAssertTrue(self.sut.isValid);
+    [self.sut resignFirstResponder];
+    XCTAssertTrue([self.sut canBecomeFirstResponder]);
+    XCTAssertTrue([self.sut becomeFirstResponder]);
+
+    XCTAssertEqual(self.sut.cvcField, self.sut.currentFirstResponderField,
+                   @"When all fields are valid, the last one should be the preferred firstResponder");
+
+    self.sut.postalCodeEntryEnabled = YES;
+    XCTAssertFalse(self.sut.isValid);
+
+    [self.sut resignFirstResponder];
+    XCTAssertTrue([self.sut becomeFirstResponder]);
+    XCTAssertEqual(self.sut.postalCodeField, self.sut.currentFirstResponderField,
+                   @"When postalCodeEntryEnabled=YES, it should become firstResponder after other fields are valid");
+
+    self.sut.expirationField.text = @"";
+    [self.sut resignFirstResponder];
+    XCTAssertTrue([self.sut becomeFirstResponder]);
+    XCTAssertEqual(self.sut.expirationField, self.sut.currentFirstResponderField,
+                   @"Moves firstResponder back to expiration, because it's not valid anymore");
+
+    self.sut.expirationField.text = @"10/99";
+    self.sut.postalCodeField.text = @"90210";
+
+    XCTAssertTrue(self.sut.isValid);
+    [self.sut resignFirstResponder];
+    XCTAssertTrue([self.sut becomeFirstResponder]);
+    XCTAssertEqual(self.sut.postalCodeField, self.sut.currentFirstResponderField,
+                   @"When all fields are valid, the last one should be the preferred firstResponder");
+}
+
 @end