Repository navigation
Standardize Expense Report and Expense header with predictable button placement and carousel counter - #90403
Conversation
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
@ShridharGoel 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] |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ea131bb78
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
🚧 @shawnborton has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
When I open an expense from the expenses list, I noticed it doesn't have the carousel in the top right: Part of this issue is implementing that feature as well. |
|
Let's give the "X of X" box some kind of min-width so that we prevent the slight jumping when we change numbers: Like a min-width of 32px and text-align: right or something? |
|
Curious what @Expensify/design thinks but I do feel like the top row and bottom row in the header could be closer together: From Figma: BUT the main thing is that in Figma, we're doing it like this: However in the product, we are doing this: 
So if we wanted to avoid a huge refactor of the top bar, the easiest thing might be to give the secondary bar a negative top margin of -4px? |
|
@shawnborton yeah that sounds good to me 👍 |
|
Big fan. I think we're maybe a bit too generous with spacing in the header so that makes sense to me 👍 |
|
Thanks for the feedback, I will address them today |
|
Hi all, I have fixed this point and this point But... for this point, I couldn't reproduce it anymore. Can you please re-test it @shawnborton Screen.Recording.2026-05-20.at.19.12.28.mov |
|
🚧 @shawnborton has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
Still missing when I go to Spend > Expenses and open an expense: |
|
Try getting some "one expense reports" in your account, where a report only has a single expense attached to it. |
| return ( | ||
| <View style={[styles.flexRow, styles.alignItemsCenter, styles.gap2]}> | ||
| {!shouldDisplayNarrowVersion && <Text style={styles.mutedTextLabel}>{`${currentIndex + 1} of ${allReportsCount}`}</Text>} | ||
| {!shouldDisplayNarrowVersion && <Text style={[styles.mutedTextLabel, styles.textAlignRight, styles.mnw8]}>{`${currentIndex + 1} of ${allReportsCount}`}</Text>} |
There was a problem hiding this comment.
Looks like we missed the i18n for this copy? Is it intentional? If not, I can fix it @shawnborton
There was a problem hiding this comment.
We should update that to be internationalized, yes!
There was a problem hiding this comment.
@shawnborton It is fixed. Could you please run the translation script?
There was a problem hiding this comment.
I'm not sure how to do that, maybe @trjExpensify or @ShridharGoel can help?
There was a problem hiding this comment.
I think you can follow this comment @shawnborton #90403 (comment)
There was a problem hiding this comment.
Oh, I see you did already. Lol, never mind.. translations twice!
|
@shawnborton I need to confirm about carousel behavior when opening a one-transaction report from the Expenses list When clicking an expense from the flat Spend > Expenses list, the destination view depends on the parent report's transaction count:
This means two different view types are reachable from the same list, which makes a single carousel behavior tricky. Current behavior in this PR
|
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx ts-node ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
|
I would think the carousel is just going to take you through the available expenses you see from Spend > Expenses. Does that make sense? It shouldn't have anything to do with if the expense is on a report or not. It's really just a carousel for whatever list you are in when you view the expense. In this case, the list is Spend > Expenses. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ 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/JS00001 in version: 9.4.41-0 🚀
|
|
🤖 Help site review: no changes required. This PR standardizes the Expense Report and Expense header UI — predictable button placement (report actions moved down by the next steps, carousel arrows pinned top-right), a consistent I reviewed
Because the help site describes what you can do (view, filter, navigate reports and expenses) rather than the exact header layout, this UI standardization doesn't require any article edits. I did not open a draft docs PR. If you'd like a short note added to the Spend page article calling out that a navigation carousel now appears when opening an expense from the Spend page, reply and I'll draft one. |
|
Deploy Blocker #96545 was identified to be related to this PR. |
|
Deploy Blocker #96548 was identified to be related to this PR. |
|
Deploy Blocker #96563 was identified to be related to this PR. |
|
Deploy Blocker #96566 was identified to be related to this PR. |
|
Deploy Blocker #96575 was identified to be related to this PR. |
Revert "Merge pull request #90403 from thelullabyy/fix-lullabyy/90366"
|
🚀 Deployed to production by https://github.com/yuwenmemon in version: 9.4.41-4 🚀
Bundle Size Analysis (Sentry): |
|
🚀 Deployed to staging by https://github.com/JS00001 in version: 9.4.42-0 🚀
|
Help site review — no changes requiredI reviewed the changes in this PR against Expensify's help site files under Why: This PR is a UI layout/standardization change to the expense report and expense headers. It:
None of this adds, removes, or changes a documented feature, workflow, setting, or button label. The carousel and header action buttons already existed — this only makes their placement and the counter consistent across the two header types. The only help article that references a "carousel" is The new Since no changes are required, I did not create a draft help site PR. @thelullabyy — flagging you in case you disagree with this assessment. If you believe a help article should be updated for this behavior, reply with |
The merge of main re-introduced changes from PR Expensify#90403 (fix-lullabyy/90366), which had been reverted on main. This restores MoneyRequestReportTransactionsNavigation.tsx to the genuine PR Expensify#96355 change (thread personalDetails through) and removes the redundant MoneyRequestReportTransactionsNavigation.test.tsx that belonged to the reverted PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01964hgbhMgCovsXubPWdcEv
|
🚀 Deployed to production by https://github.com/yuwenmemon in version: 9.4.42-1 🚀
Bundle Size Analysis (Sentry): |
13 participants
Explanation of Change
Fixed Issues
$ #90366
PROPOSAL: N/A
Tests
Log in with a test account that has access to a paid workspace with multiple expense reports and expenses
Have these in your test workspace:
Offline tests
QA Steps
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))npm run compress-svg)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
Screen.Recording.2026-05-17.at.23.29.29.mov
Android: mWeb Chrome
Screen.Recording.2026-05-17.at.23.25.25.mov
iOS: Native
iOS: mWeb Safari
Screen.Recording.2026-05-17.at.23.21.16.mov
MacOS: Chrome / Safari
Screen.Recording.2026-05-17.at.23.15.10.mov