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.

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

[$250] Update how we show pending and scanning status in the expense preview #52921

Open
shawnborton opened this issue Nov 21, 2024 · 14 comments
Assignees
Labels
External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@shawnborton
Copy link
Contributor

shawnborton commented Nov 21, 2024

Problem:
We're updating the expense preview to show the category and tag of the expense if there is one. However, that created a visual clash with how we have implemented the pending or scanning status text on an expense preview:
image

Solution:
Let's update the way we show the pending or scanning status on expense previews:
image

For scanning receipts, since the big text already says "Receipt scanning", we probably don't need to add anything extra here - let's just update the text to say "Scanning..." and "Receipt" will be in smaller letters above it.

For pending card transactions, we can just reuse the small dot separator pattern in the eye brow line above the amount.

cc @Expensify/design

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021859994708414093242
  • Upwork Job ID: 1859994708414093242
  • Last Price Increase: 2024-11-22
  • Automatic offers:
    • nkdengineer | Contributor | 105062857
Issue OwnerCurrent Issue Owner: @ntdiary
@nkdengineer
Copy link
Contributor

nkdengineer commented Nov 21, 2024

Edited by proposal-police: This proposal was edited at 2024-11-21 17:07:21 UTC.

Proposal

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

Update how we show pending and scanning status in the expense preview

What is the root cause of that problem?

new style

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

  1. Update translation key receiptScanning to 'scanning...'

receiptScanning: 'Receipt scanning...',

  1. Add props small to this Icon here

<Icon
src={Expensicons.DotIndicator}
fill={theme.danger}
/>

  1. For pending card transactions, we can remove this condition and status to getPreviewHeaderText function
        if(TransactionUtils.isPending(transaction)){
            message += `${CONST.DOT_SEPARATOR} Pending}`
        }

if (TransactionUtils.isPending(transaction)) {
return {shouldShow: true, messageIcon: Expensicons.CreditCardHourglass, messageDescription: translate('iou.transactionPending')};
}

What alternative solutions did you explore? (Optional)

@neonbhai
Copy link
Contributor

Proposal

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

Update how we show pending and scanning status in the expense preview

What is the root cause of that problem?

Feature Request

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

For Pending Transaction:

  1. Remove the Transaction Pending status here:
    if (TransactionUtils.isPending(transaction)) {
    return {shouldShow: true, messageIcon: Expensicons.CreditCardHourglass, messageDescription: translate('iou.transactionPending')};
    }
  2. Add Transaction Pending status for transaction in here:

    We will have it before Review Required & Hold as Design Team prefers.

For Scanning:

  1. Change here to Scanning...
  2. Remove Receipt Scan in Progress from here. Use an empty return as the last condition (hasPendingUI is true for scanning receipts)

@JmillsExpensify
Copy link

@grgia is this internal or can I open it up to the community?

@dannymcclain
Copy link
Contributor

@shawnborton thank you for making the issue! 🙌

@grgia
Copy link
Contributor

grgia commented Nov 22, 2024

@JmillsExpensify let's open this one up to the community-- adding External label

@grgia grgia added the External Added to denote the issue can be worked on by a contributor label Nov 22, 2024
Copy link

melvin-bot bot commented Nov 22, 2024

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

@melvin-bot melvin-bot bot changed the title Update how we show pending and scanning status in the expense preview [$250] Update how we show pending and scanning status in the expense preview Nov 22, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 22, 2024
Copy link

melvin-bot bot commented Nov 22, 2024

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

@JmillsExpensify JmillsExpensify self-assigned this Nov 22, 2024
@JmillsExpensify
Copy link

Awesome added myself for payment when we get there.

@ntdiary
Copy link
Contributor

ntdiary commented Nov 25, 2024

It seems like some simple changes. I'm going to approve @nkdengineer's proposal, but there are still a few minor details to take care of.

  1. I didn't notice any specific requirement to use a small Icon.
  2. I don't recommend changing the translation for receiptScanning, since it's used in other places, such as report names. Perhaps we can use receiptStatusTitle in the specific places where it's needed instead.

    receiptStatusTitle: 'Scanning…',
  3. We still have a hasPendingUI, maybe also need to remove isPending in this condition (or remove entire condition) ?
    if (TransactionUtils.hasPendingUI(transaction, TransactionUtils.getTransactionViolations(transaction?.transactionID ?? '-1', transactionViolations))) {
    return {shouldShow: true, messageIcon: Expensicons.Hourglass, messageDescription: translate('iou.pendingMatchWithCreditCard')};
    }

    🎀 👀 🎀 C+ reviewed

Copy link

melvin-bot bot commented Nov 25, 2024

Current assignee @grgia is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

@nkdengineer
Copy link
Contributor

Screenshot 2024-11-25 at 23 04 43

Thanks @ntdiary, for the small icon, you can see design here

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 25, 2024
Copy link

melvin-bot bot commented Nov 25, 2024

📣 @nkdengineer 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@nkdengineer
Copy link
Contributor

@ntdiary we have a draft PR here but I don't know how to create expense with type Card like design in OP. Please help me

@grgia
Copy link
Contributor

grgia commented Nov 26, 2024

@nkdengineer one way is to test UI is to onyx merge an expensify card and then update an existing manual request transaction to have transaction.bank = Expensify Card. I could send you example data from my dev environment.

I can also help QA

@garrettmknight garrettmknight moved this to Bugs and Follow Up Issues in [#whatsnext] #expense Nov 26, 2024
Copy link

melvin-bot bot commented Nov 29, 2024

@JmillsExpensify, @dannymcclain, @ntdiary, @grgia, @nkdengineer Whoops! This issue is 2 days overdue. Let's get this updated quick!

@melvin-bot melvin-bot bot added the Overdue label Nov 29, 2024
@grgia
Copy link
Contributor

grgia commented Nov 29, 2024

Sent test data in DM

@nkdengineer
Copy link
Contributor

I'll open this PR today

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 Overdue labels Dec 2, 2024
@ntdiary
Copy link
Contributor

ntdiary commented Dec 12, 2024

the related PR disappear? 🤔
a related slack conv

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
Status: Bugs and Follow Up Issues
Development

No branches or pull requests

7 participants