Conversation
b32a55c to
9fb6971
Compare
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.
|
I'm honestly tempted to say no so that we don't muck up the spacing when the page is "at rest". Curious for the other designers' takes though. |
9fb6971 to
b083791
Compare
|
Hmm that's a fair point... I think I'd be curious to see like 4px of extra space there to see if that helps, as I think it doesn't look great when the green line runs over the buttons like that. |
|
@ZhenjaHorbach 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] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0837916f5
ℹ️ 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".
|
|
||
| const shouldRedirectToExpensifyClassic = useMemo(() => areAllGroupPoliciesExpenseChatDisabled((allPolicies as OnyxCollection<OnyxTypes.Policy>) ?? {}), [allPolicies]); | ||
|
|
||
| const shouldShowBookTravel = useMemo(() => Object.values(allPolicies ?? {}).some((policy) => !!policy?.isTravelEnabled), [allPolicies]); |
There was a problem hiding this comment.
Use a travel-enabled policy for Book travel action
The button visibility is based on any policy having isTravelEnabled, but the click handler always uses activePolicy?.id; when a user has a travel-enabled workspace but their active policy is personal/non-travel, the CTA is shown yet routes to My Trips with the wrong policy and booking is blocked (BookTravelButton rejects non-paid/default workspaces). This creates a user-facing dead end for a common account state (travel enabled somewhere, different active policy).
Useful? React with 👍 / 👎.
| ); | ||
| } | ||
|
|
||
| QuickCreationActionsBar.displayName = 'QuickCreationActionsBar'; |
There was a problem hiding this comment.
We don't need displayName
|
|
||
| return ( | ||
| <> | ||
| {CreateReportConfirmationModal} |
There was a problem hiding this comment.
Can we use <CreateReportConfirmationModal/> ? 😅
There was a problem hiding this comment.
I believe it would require to refactor a bit useCreateEmptyReportConfirmation, we get from this hook React.ReactNode instead of the component function, so that's why it's passed this way.
This notation can also be found in other files
|
🚧 @JmillsExpensify has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
JmillsExpensify
left a comment
There was a problem hiding this comment.
Looks solid! I tested on web and mobile.
| if ( | ||
| !workspaceIDForReportCreation || | ||
| (shouldRestrictUserBillableActions(workspaceIDForReportCreation, undefined, undefined, ownerBillingGraceEndPeriod) && groupPoliciesWithChatEnabled.length > 1) | ||
| ) { | ||
| Navigation.navigate(ROUTES.NEW_REPORT_WORKSPACE_SELECTION.getRoute()); | ||
| return; | ||
| } | ||
|
|
||
| if (!shouldRestrictUserBillableActions(workspaceIDForReportCreation, undefined, undefined, ownerBillingGraceEndPeriod)) { | ||
| if (shouldShowEmptyReportConfirmationForDefaultChatEnabledPolicy) { | ||
| openCreateReportConfirmation(); | ||
| } else { | ||
| handleCreateWorkspaceReport(false); | ||
| } | ||
| return; | ||
| } |
There was a problem hiding this comment.
If there's a possibility this could be a dead button for a case (ie click and do nothing) should we hide it or show a disabled UI?
There was a problem hiding this comment.
This shouldn't happen, handleCreateWorkspaceReport function calls navigate and openCreateReportConfirmation shows a modal window
After using the ad hoc build, I'm also curious to see like 4px of space. I agree the line running over the buttons doesn't feel great. Otherwise testing really well for me! |
|
Removing my review as Jason already approved for product |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari2026-03-17.10.01.32.mov2026-03-17.10.04.24.mov |
|
But overall changes look good! |
|
🚧 @grgia has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 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/grgia in version: 9.3.40-0 🚀
|
|
🚀 Deployed to staging by https://github.com/grgia in version: 9.3.40-0 🚀
|
|
Hi @WojtekBoman. Could this PR be implemented in the native apps? сс @grgia |
|
@IuliiaHerets
|
|
Yes, not in scope for native apps. This is a desktop-only feature. |
|
@WojtekBoman and @joekaufmanexpensify Thanks |
|
🚀 Deployed to production by https://github.com/cristipaval in version: 9.3.41-4 🚀
|


Explanation of Change
QuickActionsBarcomponent to the Home page with quick creation buttons: Expense, Report, Distance, and Book travelcar-plus.svganddocument-plus.svg, and registered them inExpensiconsand the lazy-loadedexpensify-iconschunkQuickActionsBarintoHomePage, rendering it only on desktop/wide layouts (!shouldUseNarrowLayout)Fixed Issues
$ #85084
PROPOSAL:
Tests
1. QuickActionsBar visibility (desktop only)
2. Expense button
receipt-plusicon and "Expense" label3. Report button
document-plusicon and "Report" label4. Distance button
car-plusicon and "Distance" label5. Book Travel button visibility
luggage-with-linesicon and "Book travel" label6. Book Travel button action
/travel) for the onboarding flowOffline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari