Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix expiry date field issue in CardInputWidget #5547

Merged
merged 9 commits into from
Sep 28, 2022

Conversation

tillh-stripe
Copy link
Collaborator

@tillh-stripe tillh-stripe commented Sep 13, 2022

Summary

This pull request fixes an issue in CardInputWidget that occurred on some devices, such as those from Samsung.

A quick explanation: On the affected devices, the StripeEditText’s OnKeyListener wasn’t called for all key presses, but only for presses of the backspace. With that, the class’s isLastKeyDelete was always true after a backspace press, resulting in the ExpiryDateEditText’s text not being updated.

We’re adding a TextWatcher in addition to the OnKeyListener to make sure that the isLastKeyDelete property is always in the correct state.

Also, we’re removing the !isLastKeyDelete condition in ExpiryDateEditText’s afterTextChanged. This condition prevented a behavior we actually want: If the current text is 12 / 2 and the user presses backspace, we want to end up with 12 instead of 12 /. This behavior was explicitly called out in a comment, but must have been broken some time ago.

Motivation

Resolves: #5539

Testing

  • Added tests
  • Modified tests
  • Manually verified

Screenshots

Before After
before screenshot after screenshot

Changelog

[Fixed] Expiry dates in CardInputWidget, CardMultilineWidget, and CardFormView are no longer formatted incorrectly on certain devices.

@tillh-stripe tillh-stripe changed the title Fix expiry date field issue Fix expiry date field issue in CardInputWidget Sep 13, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Sep 13, 2022

Diffuse output:

OLD: paymentsheet-example-release-master.apk (signature: none)
NEW: paymentsheet-example-release-pr.apk (signature: none)

          │          compressed          │           uncompressed           
          ├──────────┬──────────┬────────┼───────────┬───────────┬──────────
 APK      │ old      │ new      │ diff   │ old       │ new       │ diff     
──────────┼──────────┼──────────┼────────┼───────────┼───────────┼──────────
      dex │ 15.8 MiB │ 15.8 MiB │ +334 B │  53.4 MiB │  53.4 MiB │   +752 B 
     arsc │  1.9 MiB │  1.9 MiB │    0 B │   1.9 MiB │   1.9 MiB │      0 B 
 manifest │  4.1 KiB │  4.1 KiB │    0 B │  18.9 KiB │  18.9 KiB │      0 B 
      res │    1 MiB │    1 MiB │    0 B │   1.8 MiB │   1.8 MiB │      0 B 
   native │  2.5 MiB │  2.5 MiB │    0 B │   5.9 MiB │   5.9 MiB │      0 B 
    asset │    3 MiB │    3 MiB │ +572 B │     3 MiB │     3 MiB │   +572 B 
    other │ 81.8 KiB │ 81.8 KiB │    0 B │ 155.7 KiB │ 155.7 KiB │      0 B 
──────────┼──────────┼──────────┼────────┼───────────┼───────────┼──────────
    total │ 24.3 MiB │ 24.3 MiB │ +906 B │  66.1 MiB │  66.1 MiB │ +1.3 KiB 

         │          raw           │            unique             
         ├────────┬────────┬──────┼────────┬────────┬─────────────
 DEX     │ old    │ new    │ diff │ old    │ new    │ diff        
─────────┼────────┼────────┼──────┼────────┼────────┼─────────────
   files │      4 │      4 │    0 │        │        │             
 strings │ 256373 │ 256379 │   +6 │ 219222 │ 219228 │ +6 (+10 -4) 
   types │  45112 │  45113 │   +1 │  41478 │  41479 │ +1 (+1 -0)  
 classes │  38654 │  38655 │   +1 │  38654 │  38655 │ +1 (+1 -0)  
 methods │ 225226 │ 225228 │   +2 │ 217432 │ 217434 │ +2 (+8 -6)  
  fields │ 164493 │ 164495 │   +2 │ 163523 │ 163525 │ +2 (+2 -0)  

 ARSC    │ old  │ new  │ diff 
─────────┼──────┼──────┼──────
 configs │  334 │  334 │  0   
 entries │ 6246 │ 6246 │  0
APK
    compressed    │    uncompressed    │                                
─────────┬────────┼─────────┬──────────┤                                
 size    │ diff   │ size    │ diff     │ path                           
─────────┼────────┼─────────┼──────────┼────────────────────────────────
 9.8 KiB │ +578 B │ 9.6 KiB │   +578 B │ ∆ assets/dexopt/baseline.prof  
 3.2 MiB │ +334 B │ 9.2 MiB │   +752 B │ ∆ classes4.dex                 
   666 B │   -6 B │   534 B │     -6 B │ ∆ assets/dexopt/baseline.profm 
─────────┼────────┼─────────┼──────────┼────────────────────────────────
 3.3 MiB │ +906 B │ 9.2 MiB │ +1.3 KiB │ (total)
DEX
STRINGS:

   old    │ new    │ diff        
  ────────┼────────┼─────────────
   219222 │ 219228 │ +6 (+10 -4) 
  + �
  
  ���
  
  ���
  
  ��
  
  ���
  ���*��
  �2�0�J*����0�2�������0�2�����0�2�����0�2��	��0�H�¨�
  
  + ��
  ���
  ���
  
  ���
  
  ���
  
  ���
  ���
  ���
  ���
  ���
  
  ���
  ��
  
  ���
  ���
  ���
  ���
  ���
  ���
  ��!
  ���
  ���
  ���
  ���
  ���
  
  ���
  ��
  
  ���
  
  ���
  ���
  ���
  ���
  ���
  ���
  ��
  ���*�6���2�0�:�defghB%��������0��
  ��������0���������0�¢����J��=��0>2��?����0<H�J��@��0>H�J
  �A����0&H�J��B�
   C*���0&0&H�J��D��0.2��E��0�H�J��F��0>H�J��G��0>H�J��H����0I2��J��0KH�J��L��0>2��M��0NH�J��O��0>2��P����0QH�J��R��0QH�J��S��0>2��?����0<H�J��T��0>2��
  ����0�J��U��0>2�������0�J��V��0>2����W��0�J��X��0>2�������0
  J��Y��0>2�� ����0!J��Z��0>H�J��[��0>2��\����0&J��]��0>2��^����0�H�J��]��0>2��_��0�H�J����0>2��a����0bH¢���cR��	����0
  X��¢��
  �����R��
  ����0�X��¢��
  R_����0�2�����0�@AX��¢��
  �����������R�����0�8�@�X��¢��
  R�����0�8G¢��������R�������0�X��¢��
  R�������0
  X��¢��
  ����������R�� ����0!X��¢��
  R������0�X��¢��
  R��#����0�8�@�X��¢��
  ��_R��%����0&X��¢��
  R��'��0
  8@X��¢�����(�R��)������0&0*8�X��¢��
  ���+�,R_�-��0.8@X��¢��
  ���/�0���1�2��3�4R��5��06X��¢��
  ��7R_�8��0.2��8��0.@FX��¢��
  ���9�2��:�4R��;�
  ����0<��0*X��¢��
  ¨�i
  + Lcom/stripe/android/view/StripeEditText_isLastKeyDeleteTextWatcher_1;
  + SMAP
  StripeEditText.kt
  Kotlin
  *S Kotlin
  *F
  + 1 StripeEditText.kt
  com/stripe/android/view/StripeEditText
  + 2 fake.kt
  kotlin/jvm/internal/FakeKt
  + 3 TextView.kt
  androidx/core/widget/TextViewKt
  + 4 _Collections.kt
  kotlin/collections/CollectionsKt___CollectionsKt
  *L
  1#1,341:1
  1#2:342
  58#3,23:343
  93#3,3:366
  1851#4,2:369
  1851#4,2:371
  1851#4,2:373
  *S KotlinDebug
  *F
  + 1 StripeEditText.kt
  com/stripe/android/view/StripeEditText
  *L
  200#1:343,23
  200#1:366,3
  317#1:369,2
  321#1:371,2
  277#1:373,2
  *E
  
  + SMAP
  TextView.kt
  Kotlin
  *S Kotlin
  *F
  + 1 TextView.kt
  androidx/core/widget/TextViewKt_addTextChangedListener_textWatcher_1
  + 2 StripeEditText.kt
  com/stripe/android/view/StripeEditText
  + 3 TextView.kt
  androidx/core/widget/TextViewKt_addTextChangedListener_1
  + 4 TextView.kt
  androidx/core/widget/TextViewKt_addTextChangedListener_2
  *L
  1#1,97:1
  201#2,2:98
  71#3:100
  77#4:101
  *E
  
  + com/stripe/android/view/StripeEditText_isLastKeyDeleteTextWatcher_1
  + isLastKeyDelete_payments_core_release
  + isLastKeyDelete_payments_core_release_annotations
  + isLastKeyDeleteTextWatcher
  + setLastKeyDelete_payments_core_release
  
  - ��
  ���
  ���
  
  ���
  
  ���
  
  ���
  ���
  ���
  ���
  ���
  
  ���
  ��
  
  ���
  ���
  ���
  ���
  ���
  ���
  ��!
  ���
  ���
  ���
  ���
  
  ���
  ���
  ���
  ���
  ���
  
  ���
  ���
  ���
  ���
  ���
  ���
  ��
  ������2�0�:�abcdB%��������0��
  ��������0���������0�¢����J��7��082��9����06H�J��:��08H�J
  �;����0&H�J��<�
   =*���0&0&H�J��>��0.2��?��0�2��@��0AH�J��B��08H�J��C��08H�J��D����0E2��F��0GH�J��H��082��I��0JH�J��K��082��L����0MH�J��N��0MH�J��O��082��9����06H�J��P��082��
  ����0�J��Q��082�������0�J��R��082����S��0�J��T��082�������0
  J��U��082�� ����0!J��V��08H�J��W��082��X����0&J��Y��082��Z����0�H�J��Y��082��[��0�H�J��\��082��]����0^H¢���_R��	����0
  X��¢��
  �����R��
  ����0�X��¢��
  R_����0�2�����0�@AX��¢��
  �����������R�����0�8�@�X��¢��
  R�����0�8G¢��������R�������0�X��¢��
  R�������0
  X��¢��
  ����������R�� ����0!X��¢��
  R������0�X��¢��
  R��#����0�8�@�X��¢��
  ��_R��%����0&X��¢��
  R��'��0
  8@X��¢�����(�R��)������0&0*8�X��¢��
  ���+�,R��-��0.X��¢��
  ���-�/��0�1R_�2��0.2��2��0.@FX��¢��
  ���3�/��4�1R��5�
  ����06��0*X��¢��
  ¨�e
  - SMAP
  StripeEditText.kt
  Kotlin
  *S Kotlin
  *F
  + 1 StripeEditText.kt
  com/stripe/android/view/StripeEditText
  + 2 fake.kt
  kotlin/jvm/internal/FakeKt
  + 3 TextView.kt
  androidx/core/widget/TextViewKt
  + 4 _Collections.kt
  kotlin/collections/CollectionsKt___CollectionsKt
  *L
  1#1,321:1
  1#2:322
  58#3,23:323
  93#3,3:346
  1851#4,2:349
  1851#4,2:351
  1851#4,2:353
  *S KotlinDebug
  *F
  + 1 StripeEditText.kt
  com/stripe/android/view/StripeEditText
  *L
  193#1:323,23
  193#1:346,3
  297#1:349,2
  301#1:351,2
  257#1:353,2
  *E
  
  - SMAP
  TextView.kt
  Kotlin
  *S Kotlin
  *F
  + 1 TextView.kt
  androidx/core/widget/TextViewKt_addTextChangedListener_textWatcher_1
  + 2 StripeEditText.kt
  com/stripe/android/v
...✂

}
}

addTextChangedListener(textWatcher)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thinking out loud: listenForDeleteEmpty() is only called once in the constructor, so this is safe. If this constraint were to change, we wouldn't want to add multiple text watchers.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right, I added a check to make sure we only ever add this once. Just in case this is ever moved anywhere else.

@@ -86,6 +86,12 @@ open class StripeEditText @JvmOverloads constructor(

private var errorMessageListener: ErrorMessageListener? = null

private val isLastKeyDeleteTextWatcher = object : StripeTextWatcher() {
Copy link
Contributor

Choose a reason for hiding this comment

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

should we also add a test in StripeEditTextTest?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added some!

@tillh-stripe tillh-stripe force-pushed the tillh/5539-expiry-date-field-formatting branch from 2576cf9 to 10abb95 Compare September 23, 2022 13:12
@sandyscoffable
Copy link

As mentioned in #5539, I've manually tested the version supplied and it fixes the problem for me (which I could previously reliably replicate)

brnunes-stripe
brnunes-stripe previously approved these changes Sep 27, 2022
@tillh-stripe tillh-stripe merged commit 64a5cfa into master Sep 28, 2022
@tillh-stripe tillh-stripe deleted the tillh/5539-expiry-date-field-formatting branch September 28, 2022 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] CardInputWidget - Expiry Date not always formatted properly
6 participants