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.98k stars 2.98k forks source link

[$250] Improve eReceipt visually in New Dot #55083

Open anmurali opened 2 weeks ago

anmurali commented 2 weeks ago

When I go to approve expenses that @cole submits from his Expensify Card, I keep finding myself asking him to upload a receipt. That's because the receipt in the app looks like this image Looks like there's no receipt but that's actually an eReceipt and is sufficient from a compliance standpoint.

Let's update the design for an eReceipt to look more like a receipt.

CleanShot 2025-01-13 at 09 26 35@2x

Let's also make sure the eReceipt takes up full width in the various thumbnail areas where its used, such as:

Image

Image

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021878720719253020177
  • Upwork Job ID: 1878720719253020177
  • Last Price Increase: 2025-01-13
  • Automatic offers:
    • ishpaul777 | Reviewer | 105839173
    • mkzie2 | Contributor | 105839174
Issue OwnerCurrent Issue Owner: @ishpaul777
melvin-bot[bot] commented 2 weeks ago

Triggered auto assignment to @dannymcclain (Design), see these Stack Overflow questions for more details.

shawnborton commented 2 weeks ago

I like all of them except the plug that isn't plugged in one. That somehow suggests to me that something needs doing.

The icons are added depending on the MCC category. So that might be what we use for utility bills or something.

dannymcclain commented 2 weeks ago

Giving this one to you since you've done all the work for it!

shawnborton commented 2 weeks ago

Putting the receipt edges here for posterity so I don't lose them: receiptedge

shawnborton commented 2 weeks ago

Very generous with your GH coinage today, thank you again for filling my cup.

shawnborton commented 2 weeks ago

I think we can probably make this external @anmurali - thoughts? Also cc @grgia since you were the original engineer behind our NewDot eReceipts...

anmurali commented 2 weeks ago

If its clear from my issue description what the contributor needs to do, let's slap an External on it. I wasn't sure I had described it sufficiently clearly

shawnborton commented 1 week ago

Okay, I updated the original post with a more clear screenshot. I would still love @grgia 's thoughts on how to implement this best though, but I think we can at least start getting proposals for this one.

melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

Triggered auto assignment to @strepanier03 (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.

dannymcclain commented 1 week ago

@shawnborton you may want to add some mocks of the new eReceipt "in situ" in the previews cards and stuff, just to make sure that gets implemented properly!

shawnborton commented 1 week ago

Good shout Danny. After taking them for a spin, I think it makes sense to do something like this where we just make the eReceipt go full width inside its container. We have various container sizes depending on the situation, but here is generally what might happen:

Image

Image

Thoughts? cc @dubielzyk-expensify too.

dannymcclain commented 1 week ago

Honestly I think that's lovely! Even the 3-receipt version looks fantastic IMO.

shawnborton commented 1 week ago

Awesome, let's see what Jon thinks but in the meantime, I've added those mocks to the OP for extra clarity. Thanks for the nudge on this one!

dubielzyk-expensify commented 1 week ago

I love them ❤️ I think they're stunning. SHIP IT

ishpaul777 commented 1 week ago

We are looking for proposals here

grgia commented 1 week ago

Question, should we be creating these as PNGs from the BE or something instead of live from data?

grgia commented 1 week ago

Image

This screenshot implies that the eReceipt will be an image instead of a thumbnail we can create from the FE

@shawnborton

shawnborton commented 1 week ago

This is why I tagged you @grgia!

How do you think it's best to accomplish the screenshot above? And how are we doing it now? It seems like now we are doing something to inject the eReceipt image/styles into the thumbnail areas. Curious for your thoughts.

grgia commented 1 week ago

Ah gothca, It sounded like this design had moved to implementation.

@shawnborton

We currently use the data to build the receipt in the FE. There's no actual image or preview- the thumbnails use the MCC data from the transaction to show the icon. Same with the EReceipt, the transaction data is injected.

Thumbnail: https://github.com/Expensify/App/blob/7ba9f8a36c6cfb5323ffe971686fec1baf86d842/src/components/EReceiptThumbnail.tsx

Receipt: https://github.com/Expensify/App/blob/f9fe23973c2c8203ac82d7419552fcca9cf2f786/src/components/EReceipt.tsx

shawnborton commented 1 week ago

Hmm interesting, how do you think we would pull that off for the mockups above? The idea on them is to basically display eReceipt thumbnails the same way we display image-based receipt thumbnails...

mkzie2 commented 1 week ago

Proposal

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

Improve eReceipt visually in New Dot

What is the root cause of that problem?

This is an improvement

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

  1. We need to remove EReceiptThumbnail here and we need to reuse the logic in EReceiptThumbnail to display the background image and get the MCC icon

https://github.com/Expensify/App/blob/25cd9e580d3235d7fdb21787ed04ceaf9073a6fb/src/components/EReceipt.tsx#L54-L59

  1. The main UI will have two svg here and inside these svg we will display the content with the white background.

  2. About the transaction data, we need to change the data text like currency, amount, merchant, ... to primary color of eReceipt here

https://github.com/Expensify/App/blob/25cd9e580d3235d7fdb21787ed04ceaf9073a6fb/src/components/EReceipt.tsx#L36

  1. Then when we display the thumbnail, we will use EReceipt component if it's isEReceipt otherwise if it's a thumbnail we will use EReceiptThumbnail component. To scale the EReceipt we can use the transform style and scale based on the width of the parent component
const [scale, setScale] = useState<number>(1);

const onParentLayout = (event: LayoutChangeEvent) => {
    const parentWidth = event.nativeEvent.layout.width;
    const desiredWidth = 335; // Replace with the original width of your component
    const scaleFactor = Math.min(parentWidth / desiredWidth, 1);
    setScale(scaleFactor);
};
<View
  onLayout={onParentLayout}
  style={style ?? [styles.mw100, styles.h100, {transform: `scale(${scale})`, transformOrigin: 'top left'}]}
>
  <EReceipt
      transactionID={transactionID ?? '-1'}
  />
</View>

https://github.com/Expensify/App/blob/25cd9e580d3235d7fdb21787ed04ceaf9073a6fb/src/components/ReceiptImage.tsx#L134

Other styles can discuss in the PR.

Result

Image Image Image

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

ishpaul777 commented 1 week ago

~will review proposal today~

will look here on Monday

ishpaul777 commented 5 days ago

@mkzie2 proposal looks good, we can make any design/code polish at PR stage

🎀 👀 🎀

melvin-bot[bot] commented 5 days ago

Triggered auto assignment to @danieldoglas, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

mkzie2 commented 1 day ago

@danieldoglas Please help to take a look when you have a chance.

melvin-bot[bot] commented 1 day ago

@shawnborton @danieldoglas @strepanier03 @grgia @ishpaul777 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

melvin-bot[bot] commented 1 day ago

@shawnborton, @danieldoglas, @strepanier03, @grgia, @ishpaul777 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

melvin-bot[bot] commented 1 day ago

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

Offer link Upwork job

melvin-bot[bot] commented 1 day ago

📣 @mkzie2 🎉 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 📖

grgia commented 1 day ago

LGTM @mkzie2 @ishpaul777

Let's make sure we catch the spacing between the top part of the receipt and the middle part. I can see a faint line. Would it be better to use a single SVG here?

Image
shawnborton commented 1 day ago

We can also supply an entire receipt SVG to use as the BG image in case that is helpful?

Attaching here in case it's worth trying:

receipt-body.svg.zip