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 inconsistent usage of relative attributes on card icon #1395

Conversation

freszu
Copy link
Contributor

@freszu freszu commented Aug 19, 2019

Summary

Use relative drawable methods when retrieving and setting drawables in multiline widget.

Motivation

When updating stripe to newest version 8.7.0->10.3.0 credit card icon displayed on multiline widget became black by default in our app. I took a look on what caused the issue and figured out that this regression occurred on 8.7.0->9.0.0 update. The issue was introduced in 8dae459#diff-5877ed2e4c28b26a5719752e15b01b8f
Change would cause the card image to never be updated on rtl languages and
was causing issues with initial card image on ltr as tint was not
applied because of check in update drawable method:

        Drawable[] drawables = mCardNumberEditText.getCompoundDrawablesRelative();
        Drawable original = drawables[0];
        if (original == null) {
            return;
        }

Testing

Force RTL mode card image should change on Create Card PaymentMethods when entering new credit card number. When on LTR the default black image card is not reproducible because of additional focus change event (this is not occurring in our app) but can be observed with debugger the image card image will be black until focus change event.

Screenshots

RTL screenshot
Initial black image

This would cause the card image to never be updated on rtl languages and
was causing issues with initial card image on ltr as tint was not
applied.
@mshafrir-stripe
Copy link
Collaborator

@freszu thanks for fixing this!

@mshafrir-stripe mshafrir-stripe merged commit f166680 into stripe:master Aug 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants