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.11k stars 2.6k forks source link

[Search v1] Improvements to grouped expenses on the Search page #44131

Open shawnborton opened 1 week ago

shawnborton commented 1 week ago

We're finding that the bold report title style and "X grouped expenses" feels a bit jarring when we show grouped expenses on the Search page. To help make this page feel more consistent, we'd like to make the following changes on Desktop Search for grouped expenses:

That gives us something like this for desktop: image

For mobile, we'll take a few similar approaches:

Also note: we'll want to make sure this PR gets merged first since it has some font changes we'll want here

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0116b9010289ce564f
  • Upwork Job ID: 1804172877990281426
  • Last Price Increase: 2024-06-21
  • Automatic offers:
    • dominictb | Contributor | 102873381
Issue OwnerCurrent Issue Owner: @sobitneupane
melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

Triggered auto assignment to @adelekennedy (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.

melvin-bot[bot] commented 1 week ago

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

dominictb commented 1 week ago

@shawnborton

Maybe it is not implemented in staging, right? Now, all the button have "View" title.

dominictb commented 1 week ago

Proposal

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

What is the root cause of that problem?

  1. With the feature:

    remove the HR line that is above the expenses and below the title remove X grouped expenses text

  1. With the feature:

    move the expense type icon to the left of the row

  1. With the feature:

    use a more subtle style for view button

  1. With the feature:

use combined RBR + button pattern when needed. Our buttons already support having an icon in them, we'd just pass the RBR icon plus a color

Mobile

  1. With the feature:

    remove the X grouped expenses text

  1. With the feature:

    don't use bold for the title or amount

  1. With the feature:

use combined RBR button

shawnborton commented 1 week ago

Maybe it is not implemented in staging, right? Now, all the button have "View" title.

Yeah, none of this is on staging. But what I meant by that comment was that our buttons have a prop where they can take an icon, so we get buttons that look like this: CleanShot 2024-06-24 at 12 43 11@2x

So I'm saying we can pass in the "dot indicator" icon we have, but also pass in a color (danger red) for it, too.

adelekennedy commented 1 week ago

@sobitneupane one proposal to review above

melvin-bot[bot] commented 1 week ago

@luacmartins, @sobitneupane, @adelekennedy Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

JmillsExpensify commented 1 week ago

@sobitneupane Do you mind prioritizing this one when you're online. These are important polish items we'd like to get in product and stress test as soon as we reasonably can.

sobitneupane commented 1 week ago

Thanks for the proposal @dominictb

Overall your proposal looks good to me. I think we should prioritize moving on to the PR, where we can address the minor details.

πŸŽ€ πŸ‘€ πŸŽ€ C+ reviewed

melvin-bot[bot] commented 1 week ago

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

melvin-bot[bot] commented 1 week ago

πŸ“£ @dominictb πŸŽ‰ 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 πŸ“–

dominictb commented 1 week ago

ETA: PR will be up in a day or two.

JmillsExpensify commented 6 days ago

Quick comment since we just started working on the PR. @shawnborton didn't we agree to remove tags on mobile, and in addition the folder icon for categories since only a category will appear on that line moving forward?

shawnborton commented 6 days ago

Yup, but I think we should do that as a separate GH? Maybe even consider doing the Merchant/Description idea too where we show the Description in place of Merchant if no Merchant is available on mobile?

melvin-bot[bot] commented 5 days ago

@luacmartins, @sobitneupane, @adelekennedy, @dominictb Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

JmillsExpensify commented 4 days ago

Cool, we can go that route too.

sobitneupane commented 2 days ago

Any update on PR @dominictb?

dominictb commented 2 days ago

@sobitneupane I am working on it and PR will be opened in a few hours.

dominictb commented 2 days ago

About the feature:

use combined RBR + button pattern when needed. Our buttons already support having an icon in them, we'd just pass the RBR icon plus a color

image

  1. Else, we display:

image

@shawnborton @luacmartins When does the button display as below?

image

shawnborton commented 2 days ago

Ah, I guess we haven't implemented the red dots yet for violations so I think we can skip that part for now. @luacmartins does that sound right? When do we plan to add violations here?

dominictb commented 1 day ago

@sobitneupane PR https://github.com/Expensify/App/pull/44674 is ready, and based on the comment above, we can skip:

use combined RBR + button pattern when needed. Our buttons already support having an icon in them, we'd just pass the RBR icon plus a color

luacmartins commented 1 day ago

Violations are blocked on the migration to Auth, so whenever that project is ready.

luacmartins commented 1 day ago

I agree that we don't need to worry about them in the meantime though