[Company Cards] Update the Amex cards format#52578
Conversation
|
@DylanDylann Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Reviewer Checklist
Screenshots/VideosMacOS: Chrome / SafariScreen.Recording.2024-11-18.at.17.52.15.mov |
|
@DylanDylann I've updated the PR! |
|
@Expensify/design Can you please check out of you happy with these changes? The numbers are not completely aligned yet as each number has slightly different width, but I am not sure if there is much we should do about it. Probably not worth the time now. |
mountiny
left a comment
There was a problem hiding this comment.
Waiting for the design sign off, looks good, couple questions
|
Looking much better than before IMO. Android Native doesn't seem to have the same spacing format as the others, is that a known thing?
I think the only thing we could do about it is use tabular/monospace numerals for these, but like you, I'm not totally sure we need to anything about it right now. Will let the other designers weigh in on that! |
@dannymcclain Do you mean the |
|
Ah no, I was talking about the spacing in the actual card numbers. In the reviewer screenshots above, Android Native looks different from the rest. But that just might be a goof. They look right in the issue description screenshots.
|
|
I've just re-checked android native and it looks as expected for me @DylanDylann, Could you check why you had a different result? |
|
@dannymcclain sorry guys, my bad. I updated the Android screenshot again |
|
@mountiny @dannymcclain So can we move on with the PR? |
|
I'd say so! |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.0.65-0 🚀
|
|
@VickyStash I tested this one and the card number looks good on:
However, we're still using the |
|
Regardless, that is minor issue, so checked off for QA |
@joekaufmanexpensify It looks like it was missed on this page, I can create a follow-up PR tomorrow 👌 |
|
Sweet. TY! |
|
@VickyStash If it is minor, we can add it to #52893 |
|
@DylanDylann Updated it in the PR |
|
🚀 Deployed to production by https://github.com/chiragsalian in version: 9.0.65-5 🚀
|










Explanation of Change
[Company Cards] Update the Amex cards format to show as 4 digits, 6 digits, 5 digits
Fixed Issues
$ #52551
PROPOSAL: N/A
Tests
1234 56•••• •7890) on:Offline tests
Same, as in the Tests section.
QA Steps
1234 56•••• •7890) on:PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop