Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.49k stars 2.85k forks source link

[$250] Wallet statement: Clicking on download button showing a snackbar style notification about generating PDF #50828

Open m-natarajan opened 1 week ago

m-natarajan commented 1 week ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: Reproducible in staging?: needs reproduction no statement with test account Reproducible in production?: needs reproduction If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: @coleaeason Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1728668916480749

Action Performed:

  1. Go to https://new.expensify.com/statements/202404 for any YYYYMM combination in which you have at least one wallet transaction
  2. Click the download button in the top right of the RHP
  3. Weird snackbar notification appears
  4. Nothing happens then
  5. Click download button again

Expected Result:

In step 3 itself statement should be downloaded

Actual Result:

Weird snackbar notification appears and gets downloaded after clicking download button second time

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021847035521933253796
  • Upwork Job ID: 1847035521933253796
  • Last Price Increase: 2024-10-17
Issue OwnerCurrent Issue Owner: @fedirjh
melvin-bot[bot] commented 1 week ago

Triggered auto assignment to @johncschuster (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

MelvinBot commented 1 week ago

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

bernhardoj commented 6 days ago

Edited by proposal-police: This proposal was edited at 2024-10-21 15:15:22 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

We need to press download twice to download the wallet statement.

What is the root cause of that problem?

In the wallet statement page, we first checks whether the file name of the specific year-month is available or not. If not, the app will request it first with generateStatementPDF request. https://github.com/Expensify/App/blob/ce23fb55b1f8412d609cbd2c1f25690f7c5845a9/src/pages/wallet/WalletStatementPage.tsx#L52-L68

The user then need to press the download button again when the data is available.

What changes do you think we should make in order to solve the problem?

We can make the process simpler by downloading the file automatically when the request is completed.

const isWalletStatementGenerating = walletStatement?.isGenerating ?? false;
const prevIsWalletStatementGenerating = usePrevious(isWalletStatementGenerating);
useEffect(() => {
    if (prevIsWalletStatementGenerating && !isWalletStatementGenerating && walletStatement?.[yearMonth]) {
        processDownload();
    }
}, [prevIsWalletStatementGenerating, isWalletStatementGenerating, processDownload, walletStatement, yearMonth]);

The useEffect will check if it was previously loading and the data is now available, then we trigger the download logic.

Based on this comment, we want to remove the Growl and instead show loading while generating. There is currently no way to show the loading in HeaderWithBackButton, so, we can add a new props called isDownloading.

isDownloading={isWalletStatementGenerating}

Then, in HeaderWithBackButton, if isDownloading is true, then show the loading, otherwise, show the download button.

{shouldShowDownloadButton && (
    !isDownloading ? (
        <Tooltip text={translate('common.download')}>
            <PressableWithoutFeedback ...
                accessibilityLabel={translate('common.download')}
            >
                <Icon
                    src={Expensicons.Download}
                    fill={iconFill ?? StyleUtils.getIconFillColor(getButtonState(false, false, !isDownloadButtonActive))}
                />
            </PressableWithoutFeedback>
        </Tooltip>
    ) : (
        <ActivityIndicator style={[styles.touchableButtonImage]} size='small' color={theme.spinner} />
    )
)}
melvin-bot[bot] commented 5 days ago

Job added to Upwork: https://www.upwork.com/jobs/~021847035521933253796

melvin-bot[bot] commented 5 days ago

Triggered auto assignment to Contributor-plus team member for initial proposal review - @fedirjh (External)

fedirjh commented 1 day ago

@bernhardoj I believe there are opportunities for a better UI/UX approach. Instead of downloading the statement by default, we can enhance the user experience by introducing a more intuitive flow:

cc @Expensify/design Would love to get your feedback as well.

shawnborton commented 1 day ago

I think we can keep this simple - I think we should just replace the download icon with a spinner until the download starts. No need for a growl or anything like that.

coleaeason commented 1 day ago

2024-10-21_09-40-03 (1)

shawnborton commented 1 day ago

Also TIL: I believe this is the last place left in the app where we have a growl?!

dannymcclain commented 1 day ago

Also TIL: I believe this is the last place left in the app where we have a growl?!

If I knew it was possible to show these in ND I would've been sprinkling them all around the product this whole time 😆

trjExpensify commented 1 day ago

Lmao 😂

bernhardoj commented 1 day ago

I believe this is the last place left in the app where we have a growl?!

Based on the code, we still have 4 Growl usages.

https://github.com/Expensify/App/blob/36a8b13b88a20a086b5c14e1f2129ac52024ebb7/src/pages/EnablePayments/OnfidoStep.tsx#L43-L45 https://github.com/Expensify/App/blob/36a8b13b88a20a086b5c14e1f2129ac52024ebb7/src/pages/EnablePayments/VerifyIdentity/VerifyIdentity.tsx#L51-L53 https://github.com/Expensify/App/blob/36a8b13b88a20a086b5c14e1f2129ac52024ebb7/src/pages/ReimbursementAccount/RequestorOnfidoStep.tsx#L49-L51 https://github.com/Expensify/App/blob/36a8b13b88a20a086b5c14e1f2129ac52024ebb7/src/pages/ReimbursementAccount/VerifyIdentity/VerifyIdentity.tsx#L51-L53

I think we can keep this simple - I think we should just replace the download icon with a spinner until the download starts. No need for a growl or anything like that.

Updated my proposal to include this.

fedirjh commented 1 day ago

@bernhardoj With your proposal, we will automatically download the file when the component is mounted, correct?

bernhardoj commented 19 hours ago

No, it will only download if it's previously generating and currently it's done.

image

It's possible though that the statement will be downloaded without the user press any button.

In a slow connection,

  1. Press download, a loading indicator will show
  2. Before the generating is complete, close and reopen the page. The loading indicator still show and after the generating is complete, the PDF will be downloaded.