Add view transactions cta to Expensify card and company cards page#55095
Conversation
| <MenuItem | ||
| icon={Expensicons.MoneySearch} | ||
| title={translate('workspace.common.viewTransactions')} | ||
| style={styles.mv1} |
There was a problem hiding this comment.
All of these menu items should not have a gap in between them, so I think we need to remove the vertical margin here. Can you make sure all of the menu items are 64px tall with no gaps between? Screenshots would be great, thanks! cc @Expensify/design for viz
There was a problem hiding this comment.
Doesn't quite look right. We want a gap of 12px below the last push row and above the block of optionRows. They should appear like two distinct sections. It's just that we want all of the option rows in the same block to not have any vertical margin.
For the option rows on your second screenshot, why do some have a green icon and others a gray icon? I think we want them all to be gray.
There was a problem hiding this comment.
I just noticed that the color of icons is different for update card cta. Normally these links will be green on hovering(active state). What should the color be here on default state?
|
@allgandalf 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] |
| planType: 'Tipo de plan', | ||
| submitExpense: 'Envíe los gastos utilizando el chat de su espacio de trabajo:', | ||
| defaultCategory: 'Categoría predeterminada', | ||
| viewTransactions: 'Ver transacciones', |
There was a problem hiding this comment.
Did we confirm this translation ?
| const queryString = policyID | ||
| ? `type:${type} status:${Array.isArray(status) ? status.join(',') : status} policyID:${policyID}` | ||
| : `type:${type} status:${Array.isArray(status) ? status.join(',') : status}`; | ||
| let queryString = `type:${type} status:${Array.isArray(status) ? status.join(',') : status}`; |
There was a problem hiding this comment.
did you try to mock some transaction data and see if we get the search results ?
There was a problem hiding this comment.
This is the same as before. I haven't tested this part. Did you notice any issues?
|
Also please fix failing tests |
|
Seems like there is still some feedback to address here |
|
I think the design feedback has been taken care of, so we're good there. We can also run a test build when we are close to merge to make sure it's looking good. |
|
I have made all requested changes and retested. |
|
🚧 @mountiny has triggered a test build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
almost done with testing, gonna mock some onyx data for card transactions and then approve this one |
|
One thing worth mentioning is that the card filter does not work for the cards that are owned by someone else. Even though I can see the card in my list when I the admin of a workspace but the search page does not show card filters for it. |
I don't feel strongly either but agree it would make sense. Can't think of a reason not to add it here other than it's more work. |
Yes I think we should |
|
Changes done. Its ready. |
|
Nice, let us know when we have updated screenshots/videos to review as well. We need them from Workspace Editor as well as Wallet > Cards |
|
@allgandalf This is ready for testing. |
|
I will share screenshots shortly. @allgandalf Can you please start testing this in the meantime? |
|
Closed mistakenly... |
|
🚧 @mountiny has triggered a test build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪
|
|
Vids for web and native for wallet page. Others are attached to OP. cc: @shawnborton 22.01.2025_02.39.11_REC.mp422.01.2025_02.31.45_REC.mp4 |
|
Seems to test well for me! Screen.Recording.2025-01-21.at.23.38.29.mp4 |
|
@allgandalf how is it looking? |
|
Looks great! |
Reviewer Checklist
Screenshots/VideosAndroid: NativeScreen.Recording.2025-01-22.at.12.40.12.PM.movAndroid: mWeb ChromeScreen.Recording.2025-01-22.at.12.42.16.PM.moviOS: NativeScreen.Recording.2025-01-22.at.12.31.27.PM.moviOS: mWeb SafariScreen.Recording.2025-01-22.at.12.33.03.PM.movMacOS: Chrome / SafariScreen.Recording.2025-01-22.at.12.19.57.PM.movMacOS: DesktopScreen.Recording.2025-01-22.at.12.24.07.PM.mov |
allgandalf
left a comment
There was a problem hiding this comment.
Code LGTM and tests well
|
@parasharrajat can you please resolve the conflicts |
|
@mountiny Done,. |
|
✋ 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.89-0 🚀
|
|
🚀 Deployed to production by https://github.com/yuwenmemon in version: 9.0.89-8 🚀
|
| description: getDescriptionForPolicyDomainCard(card.domainName), | ||
| shouldShowRightIcon: false, | ||
| interactive: false, | ||
| interactive: true, |
There was a problem hiding this comment.
Came from this issue
Interaction should be disabled when we delete a card in offline mode
| }} | ||
| anchorRef={paymentMethodButtonRef as RefObject<View>} | ||
| > | ||
| <View style={[styles.mv5, !shouldUseNarrowLayout ? styles.sidebarPopover : {}]}> |
There was a problem hiding this comment.
Styles are little bit off, showing to much space at the bottom. #57774







Explanation of Change
Fixed Issues
$ #54800
PROPOSAL: #54800 (comment)
Tests
Prerequisite:
Expensify Card
Company Card
Assigned Cards
Offline tests
QA Steps
Same as tests
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
15.01.2025_14.13.10_REC.mp4
Android: mWeb Chrome
15.01.2025_14.19.33_REC.mp4
iOS: Native
15.01.2025_14.57.38_REC.mp4
22.01.2025_02.39.11_REC.mp4
iOS: mWeb Safari
15.01.2025_14.16.48_REC.mp4
MacOS: Chrome / Safari
15.01.2025_14.11.28_REC.mp4
22.01.2025_02.31.45_REC.mp4
MacOS: Desktop
15.01.2025_14.12.33_REC.mp4