Simplified actions on report header#58084
Conversation
|
@marcochavezf 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] |
|
cc @luacmartins |
…ied-report-actions-header-ui
…ied-report-actions-header-ui
…ied-report-actions-header-ui
…ied-report-actions-header-ui
…ied-report-actions-header-ui
67ca9f5 to
8db6c58
Compare
|
🚀 Deployed to staging by https://github.com/luacmartins in version: 9.1.25-0 🚀
|
|
This PR is failing because of issue #59889 |
|
🚀 Deployed to production by https://github.com/jasperhuangg in version: 9.1.25-4 🚀
|
| return hasViolations && (isSubmitter || isApprover) && areWorkflowsEnabled; | ||
| } | ||
|
|
||
| function getReportPreviewAction( |
There was a problem hiding this comment.
@jnowakow FYI, there were already two different methods named getReportPreviewAction and you added a third one 🙈
There was a problem hiding this comment.
Fact, but this one this one does something different. The other two refers to ReportAction entity and this one refers to action that user can undertake from report preview and we followed naming convention established in design doc - https://docs.google.com/document/d/1JZLr_5gX348cMZXNr6HUwi4QscOcDg3IY368FMAu3H4/edit?tab=t.0#bookmark=id.n099p6o4vamj
|
|
||
| if (isRemoveHoldAction(report, reportTransactions)) { | ||
| return CONST.REPORT.PRIMARY_ACTIONS.REMOVE_HOLD; | ||
| if (isMarkAsCashAction(report, reportTransactions, violations, policy)) { |
There was a problem hiding this comment.
From issue:
Q:
is this perhaps known that the Mark as cash button is missing with the new simplified action buttons?
A:
It is not. We do have a check for it here, but I think we need to bump the priority on it so it shows above submit.
leading to this issue being created:
which addressed the bump the priority on it so it shows above submit.
| const isOneExpenseReport = reportTransactions.length === 1; | ||
| const transaction = reportTransactions.at(0); | ||
|
|
||
| if (!isOneExpenseReport || !transaction) { |
There was a problem hiding this comment.
Coming from this issue, we needed "to implement another way to verify that there is only one transaction."
| const isOpenReport = isOpenReportUtils(report); | ||
| const isSubmitter = isCurrentUserSubmitter(report.reportID); | ||
|
|
||
| if (isOpenReport && isSubmitter) { |
There was a problem hiding this comment.
Came from this issue
We had issue that when report is unsubmitted, Hold option is only seen in expense preview and not report
Here is a fix
| return isExpense && isSubmitter && isOpen && isManualSubmitEnabled && !hasViolations; | ||
| } | ||
|
|
||
| function canApprove(report: Report, violations: OnyxCollection<TransactionViolation[]>, policy?: Policy, transactions?: Transaction[]) { |
There was a problem hiding this comment.
In canApprove logic, we don't have any logic to check if the request is still scanning, which caused this bug
| return parentReport?.invoiceReceiver?.accountID === getCurrentUserAccountID(); | ||
| } | ||
|
|
||
| return policy?.role === CONST.POLICY.ROLE.ADMIN; |
There was a problem hiding this comment.
We missed looking at the invoice receiver policy's role, which caused #64376
|
|
||
| const shouldShowBackButton = shouldDisplayBackButton || shouldUseNarrowLayout; | ||
|
|
||
| const isMoreContentShown = shouldShowNextStep || !!statusBarProps; |
There was a problem hiding this comment.
This condition does not account for medium sized screen, which caused #65419
| // It's necessary to allow payment animation to finish before button is changed | ||
| if (isPaidAnimationRunning) { | ||
| return CONST.REPORT.PRIMARY_ACTIONS.PAY; | ||
| } |
There was a problem hiding this comment.
Coming from #68992, we add isApprovedAnimationRunning to the above condition to animate the "Approve" button since AnimatedSettlementButton is displayed base on the same condition
App/src/components/MoneyReportHeader.tsx
Lines 705 to 708 in c46f457
Explanation of Change
UI part of: #57436 and #57438
Fixed Issues
$ #57439
$ #57441
$ #57440
$ #59210 (this PR also fixes this bug)
PROPOSAL: N/A
Tests
Test cases:
Prerequisites: Workspace without accounting with approvals enabled, workspace with connected accounting system
Test case 1:
Submitas primary action and all secondary actions workApproveand all secondary actions work as expectedTest case 2:
Export to [Accounting system]primary option.Test case 3:
UnholdUnholdaction works.Test case 4:
Payis primary action for workspace adminOffline tests
QA Steps
Test cases:
Prerequisites: Workspace without accounting with approvals enabled, workspace with connected accounting system
Test case 1:
Submitas primary action and all secondary actions workApproveand all secondary actions work as expectedTest case 2:
Export to [Accounting system]primary option.Test case 3:
UnholdUnholdaction works.Test case 4:
Payis primary action for workspace adminPR 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
mobile.mov
Android: mWeb Chrome
android-web.mov
iOS: Native
mobile.mov
iOS: mWeb Safari
iosWe.mov
MacOS: Chrome / Safari
web.mov
MacOS: Desktop
desktop.mov