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

[$250] [Search v1] Design updates to Search rows on mobile #44563

Open shawnborton opened 4 days ago

shawnborton commented 4 days ago

We have a few simplifications we'd like to make for the mobile view of Search rows to help make the rows a bit more digestible and legible: CleanShot 2024-06-27 at 20 14 47@2x

  1. Let's remove the tag from the bottom line under the merchant, and only show a Category if one is available
  2. Let's remove the icon in front of Category
  3. If no category exists, let's vertically center the Merchant line in the available space
  4. If no Merchant exists, let's use the Description if it exists. If neither exists, do nothing and leave it blank

cc @JmillsExpensify @luacmartins @Expensify/design

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01b532850f6d400dbe
  • Upwork Job ID: 1806412035454885668
  • Last Price Increase: 2024-06-27
  • Automatic offers:
    • dukenv0307 | Reviewer | 102915283
    • dominictb | Contributor | 102915284
Issue OwnerCurrent Issue Owner: @dukenv0307
melvin-bot[bot] commented 3 days ago

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

melvin-bot[bot] commented 3 days ago

Triggered auto assignment to @garrettmknight (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 3 days ago

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

neonbhai commented 3 days ago

Proposal

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

Design updates to Search rows on mobile

What is the root cause of that problem?

This is a feature request

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

Let's remove the tag from the bottom line under the merchant, and only show a Category if one is available

We will remove the Tag Cell from here: https://github.com/Expensify/App/blob/3519795a4c2f41e9549131bd6c3b89bdbc2c172b/src/components/SelectionList/Search/TransactionListItemRow.tsx#L249-L253

Let's remove the icon in front of Category

We will edit this function to only use TextWithTooltip. We will remove this part. We will update the styles to use only one component if it can be done simply.

If no category exists, let's vertically center the Merchant line in the available space

We will add alignItemsCenter styling to this view if transactionItem?.category doesn't exist. We will use a variable to facilitate the check. The line seems near centre but not on, so we apply this.

If no Merchant exists, let's use the Description if it exists. If neither exists, do nothing and leave it blank

We will add a check here to see if transactionItem.formattedMerchant exists. We will render the description if it doesn't. https://github.com/Expensify/App/blob/3519795a4c2f41e9549131bd6c3b89bdbc2c172b/src/components/SelectionList/Search/TransactionListItemRow.tsx#L112

Assuming we want this behaviour on web also. If not we will add a check for isLargeScreenWidth when defaulting to description

dominictb commented 3 days 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:

    Let's remove the icon in front of Category

  1. With the feature:

    If no category exists, let's vertically center the Merchant line in the available space

  1. With the feature:

If no Merchant exists, let's use the Description if it exists. If neither exists, do nothing and leave it blank

dominictb commented 3 days ago

Proposal Updated

dukenv0307 commented 3 days ago

Thanks for all your proposals.

@neonbhai Your proposal is first but using alignItemsCenter is not correct to me. It should be justifyContentCenter like @dominictb's proposal.

I also agree we should set isLargeScreenWidth={true} in

https://github.com/Expensify/App/blob/27a8d0a6d88698e7d2cc6d42692abfbe7a8f6b76/src/components/SelectionList/Search/TransactionListItemRow.tsx#L302

At this place, isLargeScreenWidth is already true so setting isLargeScreenWidth: false can cause confusion.

Let's go with @dominictb's proposal

BTW, should we apply the description fallback on web/desktop? If yes, do we need to change the merchant column name? @shawnborton @luacmartins

🎀👀🎀 C+ reviewed

melvin-bot[bot] commented 3 days ago

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

melvin-bot[bot] commented 3 days ago

📣 @dukenv0307 🎉 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 3 days 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 📖

luacmartins commented 3 days ago

BTW, should we apply the description fallback on web/desktop? If yes, do we need to change the merchant column name? @shawnborton @luacmartins

I'll let @shawnborton confirm this one since he's more on top of those discussions.

shawnborton commented 3 days ago

BTW, should we apply the description fallback on web/desktop? If yes, do we need to change the merchant column name?

I think we decided no, since we plan to implement the dynamic columns on web/desktop - right @luacmartins ?

luacmartins commented 3 days ago

Yea, that makes sense!

dominictb commented 3 hours ago

@dukenv0307 PR https://github.com/Expensify/App/pull/44677 is ready