Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Show scan status bar when one of the receipt is scanning in expense report #42240

Merged
merged 5 commits into from
May 28, 2024
Merged

Show scan status bar when one of the receipt is scanning in expense report #42240

Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5c03f26
show scan status bar when one of the receipt is scanning
bernhardoj May 16, 2024
a62d0e8
fix show border bottom condition
bernhardoj May 16, 2024
7d2cc3e
prettier
bernhardoj May 16, 2024
df9bdfc
null-safety
bernhardoj May 20, 2024
ef0f367
Merge branch 'main' into fix/40828-show-scan-in-progress-indication-o…
bernhardoj May 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions src/components/MoneyReportHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ function MoneyReportHeader({
const isDraft = ReportUtils.isOpenExpenseReport(moneyRequestReport);
const [isConfirmModalVisible, setIsConfirmModalVisible] = useState(false);

const hasScanningReceipt = ReportUtils.getTransactionsWithReceipts(moneyRequestReport.reportID).some((transaction) => TransactionUtils.isReceiptBeingScanned(transaction));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const hasScanningReceipt = ReportUtils.getTransactionsWithReceipts(moneyRequestReport.reportID).some((transaction) => TransactionUtils.isReceiptBeingScanned(transaction));
const hasScanningReceipt = ReportUtils.getTransactionsWithReceipts(moneyRequestReport?.reportID).some((transaction) => TransactionUtils.isReceiptBeingScanned(transaction));

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

const transactionIDs = TransactionUtils.getAllReportTransactions(moneyRequestReport?.reportID).map((transaction) => transaction.transactionID);
const allHavePendingRTERViolation = TransactionUtils.allHavePendingRTERViolation(transactionIDs);

Expand All @@ -126,13 +127,14 @@ function MoneyReportHeader({
const shouldShowSubmitButton = isDraft && reimbursableSpend !== 0 && !allHavePendingRTERViolation;
const shouldDisableSubmitButton = shouldShowSubmitButton && !ReportUtils.isAllowedToSubmitDraftExpenseReport(moneyRequestReport);
const isFromPaidPolicy = policyType === CONST.POLICY.TYPE.TEAM || policyType === CONST.POLICY.TYPE.CORPORATE;
const shouldShowNextStep = !ReportUtils.isClosedExpenseReportWithNoExpenses(moneyRequestReport) && isFromPaidPolicy && !!nextStep?.message?.length && !allHavePendingRTERViolation;
const shouldShowNextStep =
!ReportUtils.isClosedExpenseReportWithNoExpenses(moneyRequestReport) && isFromPaidPolicy && !!nextStep?.message?.length && !allHavePendingRTERViolation && !hasScanningReceipt;
const shouldShowAnyButton = shouldShowSettlementButton || shouldShowApproveButton || shouldShowSubmitButton || shouldShowNextStep;
const bankAccountRoute = ReportUtils.getBankAccountRoute(chatReport);
const formattedAmount = CurrencyUtils.convertToDisplayString(reimbursableSpend, moneyRequestReport.currency);
const [nonHeldAmount, fullAmount] = ReportUtils.getNonHeldAndFullAmount(moneyRequestReport, policy);
const displayedAmount = ReportUtils.hasHeldExpenses(moneyRequestReport.reportID) && canAllowSettlement ? nonHeldAmount : formattedAmount;
const isMoreContentShown = shouldShowNextStep || (shouldShowAnyButton && shouldUseNarrowLayout);
const isMoreContentShown = shouldShowNextStep || hasScanningReceipt || (shouldShowAnyButton && shouldUseNarrowLayout);

const confirmPayment = (type?: PaymentMethodType | undefined) => {
if (!type) {
Expand Down Expand Up @@ -212,7 +214,7 @@ function MoneyReportHeader({
shouldShowBackButton={shouldUseNarrowLayout}
onBackButtonPress={onBackButtonPress}
// Shows border if no buttons or next steps are showing below the header
shouldShowBorderBottom={!(shouldShowAnyButton && shouldUseNarrowLayout) && !(shouldShowNextStep && !shouldUseNarrowLayout) && !allHavePendingRTERViolation}
shouldShowBorderBottom={!isMoreContentShown && !allHavePendingRTERViolation}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bernhardoj hmm.. Looks like I am missing something here. Can you please help me understand why we need to replace the existing conditions with isMoreContentShown? Both seem to be different.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to simplify it. We need to set shouldShowBorderBottom to false if hasScanningReceipt is true, but instead of appending it to the existing condition (&& !hasScanningReceipt), I just replaced it with isMoreContentShown.

Without simplifying:

!(shouldShowAnyButton && shouldUseNarrowLayout) && !(shouldShowNextStep && !shouldUseNarrowLayout) && !hasScanningReceipt && !allHavePendingRTERViolation

isMoreContentShown condition

shouldShowNextStep || hasScanningReceipt || (shouldShowAnyButton && shouldUseNarrowLayout)

If we reverse this (!isMoreContentShown), it becomes,

!shouldShowNextStep && !hasScanningReceipt && !(shouldShowAnyButton && shouldUseNarrowLayout)

repositioned:

!(shouldShowAnyButton && shouldUseNarrowLayout) && !shouldShowNextStep && !hasScanningReceipt

You can see that hasScanningReceipt and shouldShowAnyButton condition is the same. The only difference is shouldShowNextStep.

Next step component is put outside of the HeaderWithBackButton, no matter whether it's shouldUseNarrowLayout or not which has its own border bottom.

App/src/components/MoneyReportHeader.tsx

Lines 335 to 339 in 8375abe

{shouldShowNextStep && (
<View style={[styles.ph5, styles.pb3]}>
<MoneyReportHeaderStatusBar nextStep={nextStep} />
</View>
)}

App/src/components/MoneyReportHeader.tsx

Line 303 in 8375abe

<View style={isMoreContentShown ? [styles.dFlex, styles.flexColumn, styles.borderBottom] : []}>

So, we don't want the extra border bottom from the HeaderWithBackButton anymore.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. That helps.

shouldShowThreeDotsButton
threeDotsMenuItems={threeDotsMenuItems}
threeDotsAnchorPosition={styles.threeDotsPopoverOffsetNoCloseButton(windowWidth)}
Expand Down Expand Up @@ -301,6 +303,20 @@ function MoneyReportHeader({
<MoneyReportHeaderStatusBar nextStep={nextStep} />
</View>
)}
{hasScanningReceipt && (
<MoneyRequestHeaderStatusBar
title={
<Icon
src={Expensicons.ReceiptScan}
height={variables.iconSizeSmall}
width={variables.iconSizeSmall}
fill={theme.icon}
/>
}
description={translate('iou.receiptScanInProgressDescription')}
shouldShowBorderBottom={false}
/>
)}
</View>
{isHoldMenuVisible && requestType !== undefined && (
<ProcessMoneyReportHoldMenu
Expand Down
Loading