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.53k stars 2.88k forks source link

[HOLD for payment 2024-02-20] [$500] Feature request: UI - Enhance currency selection in Number Pad Page #35170

Closed isagoico closed 8 months ago

isagoico commented 9 months ago

UI - Enhance currency selection in Number Pad Page

Problem

Changing the currency in the big number pad page is not that intuitive and it's currently not very responsive in the Native app (specifically Android). This leads to users often needing multiple tries to successfully open the currency list, impacting the overall user experience.

Solution

Improve the user interface by adding a down-facing caret so users can easily identify where they need to tap to alter the currency. This should guide users on reaching the currency list on the first attempt.

Thread: https://expensify.slack.com/archives/C049HHMV9SM/p1705889957656699 Reported by: @jliexpensify

Votes were taken in the thread and πŸ₯• is the winner πŸ†

image

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~013f662f491f2cca37
  • Upwork Job ID: 1752153788071690240
  • Last Price Increase: 2024-01-30
  • Automatic offers:
    • jjcoffee | Reviewer | 28134965
melvin-bot[bot] commented 9 months ago

Triggered auto assignment to @anmurali (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

dannymcclain commented 9 months ago

Figma file is here in case that's helpful.

image

neonbhai commented 9 months ago

Proposal

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

UI - Enhance currency selection in Number Pad Page

What is the root cause of that problem?

New Feature


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


We will render a caret in CurrencySymbolButton here: https://github.com/Expensify/App/blob/889051cf6c155c225a9c602c17d95ec40d740839/src/components/CurrencySymbolButton.tsx#L27

We'll increase the area covered by PressableWithFeedback here: https://github.com/Expensify/App/blob/889051cf6c155c225a9c602c17d95ec40d740839/src/components/CurrencySymbolButton.tsx#L22 to make the hitbox bigger and easier to select.

melvin-bot[bot] commented 9 months ago

@anmurali Whoops! This issue is 2 days overdue. Let's get this updated quick!

melvin-bot[bot] commented 9 months ago

Current assignee @anmurali is eligible for the NewFeature assigner, not assigning anyone new.

melvin-bot[bot] commented 9 months ago

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

melvin-bot[bot] commented 9 months ago

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

BryceAltman commented 9 months ago

Proposal

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

The currency selection in the number pad page is not intuitive, users do not realize at first that the area is clickable.

What is the root cause of that problem?

Unclear UI, nothing currently exists to indicate that the user can click to change the currency.

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

Using the existing Icon component and the "DownArrow" Icon, we can render the caret in a similar way to the ButtonWithDropdownMenu.tsx component. The icon can be placed here in the CurrencySymbolButton.tsx file, above the text component.

https://github.com/Expensify/App/blob/889051cf6c155c225a9c602c17d95ec40d740839/src/components/CurrencySymbolButton.tsx#L27

Also necessary to add flex styling for proper alignment. No need to update the hitbox as long as the icon is contained within the existing pressable component.

Img showing changes on local dev environment (exact width can be updated once access to figma is given):

Screenshot 2024-01-30 at 5 34 50β€―pm

What alternative solutions did you explore? (Optional)

Considered removing the pressable behaviour from the text and only having the caret be pressable. But decided it would be better to keep that existing functionality and just combine it with the new caret.

melvin-bot[bot] commented 9 months ago

πŸ“£ @BryceAltman! πŸ“£ Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Screen Shot 2022-11-16 at 4 42 54 PM Format:
    Contributor details
    Your Expensify account email: <REPLACE EMAIL HERE>
    Upwork Profile Link: <REPLACE LINK HERE>
BryceAltman commented 9 months ago

Contributor details Your Expensify account email: bryce.altman@gmail.com Upwork Profile Link: https://www.upwork.com/freelancers/~0111c8051f543f4fd4

melvin-bot[bot] commented 9 months ago

βœ… Contributor details stored successfully. Thank you for contributing to Expensify!

Pujan92 commented 9 months ago

Proposal

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

Add caret icon for currency selection

What is the root cause of that problem?

New feature

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

1) Add DownArrow icon as the first child of the PressableWithoutFeedback to show icon.

<Icon
    small
    src={Expensicons.DownArrow}
    fill={theme.icon}
/>

https://github.com/Expensify/App/blob/1b1eeb81cf1dd3b439b53b94abf4c476c792aa74/src/components/CurrencySymbolButton.tsx#L22-L28

2) To display content in the center of the row add those styles to the PressableWithoutFeedback.

style={[styles.dFlex, styles.flexRow, styles.alignItemsCenter, styles.gap1]}
Screenshot 2024-01-30 at 11 51 11 Screenshot 2024-01-30 at 11 51 22
noumantahir commented 9 months ago

Proposal

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

Make sure currency selection button is visually prominent and easily tappable

What is the root cause of that problem?

  1. Currency selection button do not follow appropriate guidelines for button size. For example, According to Material Design a button should at least have width and height to minimum 9x9 mm (48x48 RN view units roughly). ref: https://m2.material.io/components/buttons#specs
  2. User experience wise, currency symbol alone do not reflect as button to user, unless user tap on it.

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

  1. Add a view component only child to PressableWithoutFeedback, since ideally pressable should have only one direct child

  2. Set minWidth of 48 units to make sure the pressable is always roughly the size of a finger tip

  3. Add a DownArrow icon component before Text

  4. Update View styling to be row, centered vertically and horizontal justifyContentshould be flex-end, flex-end would make sure button content do not add un-necessary gap towards right if ever inner content width is below minWidthlimit.

Target code snippet that will be changed https://github.com/Expensify/App/blob/1b1eeb81cf1dd3b439b53b94abf4c476c792aa74/src/components/CurrencySymbolButton.tsx#L22-L28

Component to be added to snippet

  1. View: from react-native
  2. Icon: from @components/Icon

styling to be used from utils/flex.ts

  1. justifyContentEnd
  2. alignItemsCenter
  3. flexRow

style creation in utils/sizing.ts

    mnw48: {
        minWidth: 48,
    }

Final proposed structure of code snippet

<PressableWithoutFeedback>
   <View>
      <Icon />
      <Text />
   </View>
</PressableWithoutFeedback>
Kunj3093 commented 9 months ago

please re-state the problem that we are trying to solve in this issue- The issue is to enhance the currency selection in number pad page What is the root cause of that problem?- It's the UI issue which is not letting users to find currency easily on single tap and due to unresponsive nature of the UI What changes do you think we should make in order to solve the problem?- We should change the UI for the currency so that users can easily tap and find the currency to change. We should be more precise on this. What alternative solutions did you explore? (Optional)- We can give a box with currency change so that users can easily get this currency selection. We can also get more alternatives from our UI team to suggest you more better.

melvin-bot[bot] commented 9 months ago

πŸ“£ @Kunj3093! πŸ“£ Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Screen Shot 2022-11-16 at 4 42 54 PM Format:
    Contributor details
    Your Expensify account email: <REPLACE EMAIL HERE>
    Upwork Profile Link: <REPLACE LINK HERE>
Kunj3093 commented 9 months ago

Contributor details: Owebest Technologies, Kuldeep Sharma. Your Expensify account email: kuldeep.owebesttech@gmail.com Upwork Profile Link: https://www.upwork.com/agencies/owebest/

jjcoffee commented 9 months ago

Reviewing tomorrow!

jjcoffee commented 9 months ago

I think it's fair to say that @BryceAltman has the first properly fleshed out proposal, so I'd go with them! Whilst @neonbhai was first, their proposal is a bit too scant on detail to accept.

:ribbon::eyes::ribbon: C+ reviewed

melvin-bot[bot] commented 9 months ago

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

dangrous commented 9 months ago

Yep, I agree with that - I appreciate the additional detail in @BryceAltman's proposal so that it's crystal clear what the plan is (for example, specifying using a DownArrow Icon element as the caret).

melvin-bot[bot] commented 9 months ago

πŸ“£ @jjcoffee πŸŽ‰ 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 9 months ago

πŸ“£ @BryceAltman You have been assigned to this job! Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review πŸ§‘β€πŸ’» Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs! Keep in mind: Code of Conduct | Contributing πŸ“–

BryceAltman commented 9 months ago

Thanks! PR will be up today for this one

BryceAltman commented 9 months ago

PR is up here - link

The caret is currently also visible on desktop as well as mobile. Just want to confirm if this is the expected behaviour? I believe it adds value as well on larger screens as well as smaller screens.

However if you would like it hidden on desktop just let me know and I can easily hide and make it mobile only.

Thanks! :)

jjcoffee commented 9 months ago

@BryceAltman I don't think there was any discussion around making this mobile-only, so what you've implemented sounds good. Thanks for checking though!

melvin-bot[bot] commented 8 months ago

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot[bot] commented 8 months ago

The solution for this issue has been :rocket: deployed to production :rocket: in version 1.4.40-5 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-02-20. :confetti_ball:

For reference, here are some details about the assignees on this issue:

melvin-bot[bot] commented 8 months ago

BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

BryceAltman commented 8 months ago

I still haven't been assigned the job on upwork, who can I raise this with?

jliexpensify commented 8 months ago

Sent you the offer @BryceAltman , thanks for your patience here!

BryceAltman commented 8 months ago

Sent you the offer @BryceAltman , thanks for your patience here!

Sorry I just realised the regression period is still active, thank you for assigning so quickly though!

melvin-bot[bot] commented 8 months ago

Triggered auto assignment to @sonialiap (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

anmurali commented 8 months ago

Payment not due for another 4 days but I will be OOO, so I rotated the label to assign another BZ member to pay.

sonialiap commented 8 months ago

No regressions πŸŽ‰

@BryceAltman $500 - paid βœ”οΈ @jjcoffee $500 - paid βœ”οΈ

sonialiap commented 8 months ago

https://github.com/Expensify/App/issues/35170#issuecomment-1942142149 [@jjcoffee] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.

jjcoffee commented 8 months ago

Regression Test Proposal

  1. Tap FAB -> Request money
  2. Select manual
  3. Confirm caret icon is shown in the correct location next to currency select
  4. Change theme and repeat steps
  5. Repeat with split bill action

Do we agree πŸ‘ or πŸ‘Ž

dangrous commented 8 months ago

I'd add a step to tap on it and make sure it works as expecte