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

[$100] Expensify Card - improvements to the Set a limit screen #50788

Open
2 of 6 tasks
IuliiaHerets opened this issue Oct 15, 2024 · 26 comments
Open
2 of 6 tasks

[$100] Expensify Card - improvements to the Set a limit screen #50788

IuliiaHerets opened this issue Oct 15, 2024 · 26 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@IuliiaHerets
Copy link

IuliiaHerets commented Oct 15, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Version Number: v9.0.49-0
Reproducible in staging?: Y
Reproducible in production?: Y
Issue reported by: Applause Internal Team

Action Performed:

Pre-condition: Enable expensify card and add a VBA

  1. Go to https://staging.new.expensify.com/home
  2. Tap profile icon-- workspaces- workspace
  3. Tap expensify cards
  4. Tap +issue card - select a user -- physical card
  5. Note smart limit checked
  6. Tap next
  7. Enter a decimal number and tap next

Expected Result:

In expensify card, issue a card section if we set limit in decimals, must not show error enter valid amount.

Actual Result:

In expensify card, issue a card section if we set limit in decimals, shows error enter valid amount.

Workaround:

Unknown

Platforms:

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6635041_1728983975900.Screenrecorder-2024-10-15-14-37-25-413_compress_1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021846213635698721920
  • Upwork Job ID: 1846213635698721920
  • Last Price Increase: 2024-10-16
  • Automatic offers:
    • rayane-djouah | Reviewer | 104449939
Issue OwnerCurrent Issue Owner: @rayane-djouah
@IuliiaHerets IuliiaHerets added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 15, 2024
Copy link

melvin-bot bot commented Oct 15, 2024

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

@IuliiaHerets
Copy link
Author

@slafortune FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

@twilight2294
Copy link
Contributor

twilight2294 commented Oct 15, 2024

Edited by proposal-police: This proposal was edited at 2024-10-15 09:59:48 UTC.

Proposal

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

In expensify card, issue a card section if we set limit in decimals, shows error

What is the root cause of that problem?

We have check to give error on decimals:

if (!Number(values.limit) || !Number.isInteger(Number(values.limit))) {

We need to make thus error more specific

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

We should add a new valudation check will will throw an error if !Number.isInteger(Number(values.limit)), the error would be similar to Please enter a whole dollar amount before continuing and the spanish translation too

In en.ts and both es.ts we need to create a new copy as :

    const validate = useCallback(
        (values: FormOnyxValues<typeof ONYXKEYS.FORMS.ISSUE_NEW_EXPENSIFY_CARD_FORM>): FormInputErrors<typeof ONYXKEYS.FORMS.ISSUE_NEW_EXPENSIFY_CARD_FORM> => {
            const errors = ValidationUtils.getFieldRequiredErrors(values, [INPUT_IDS.LIMIT]);

            // We only want integers to be sent as the limit
            if (!Number(values.limit)) {
                errors.limit = translate('iou.error.invalidAmount');
            }
            
            if(!Number.isInteger(Number(values.limit))){
            errors.limit = translate('iou.error.invalidDecimalAmount');
            }
            return errors;
        },
        [translate],
    );

Same should be done to the edit limit screen as well

@slafortune
Copy link
Contributor

slafortune commented Oct 15, 2024

Yeah - I don't think that we would/should expect a decimal to be entered. I wonder if simply updating the error message to indicate that they need to enter a whole number would be good.

asking in design - https://expensify.slack.com/archives/C03TME82F/p1729001172189579

@slafortune slafortune changed the title Card - In expensify card, issue a card section if we set limit in decimals, shows error Expensify Card - improvements to the Set a limit screen Oct 15, 2024
@slafortune
Copy link
Contributor

Let's use this to make two updates to th Expensify Card Set Limit Screen

  • Align the $ with the amount box
  • Update the error message to the exact error
    • Current error message is Please enter a valid amount before continuing.

@Expensify/design can you chime in on what would be preferred language?

  • Please enter a whole dollar amount before continuing.
  • Enter only a whole dollar amount without a decimal point.
  • ?

@slafortune slafortune added the External Added to denote the issue can be worked on by a contributor label Oct 15, 2024
@melvin-bot melvin-bot bot changed the title Expensify Card - improvements to the Set a limit screen [$250] Expensify Card - improvements to the Set a limit screen Oct 15, 2024
Copy link

melvin-bot bot commented Oct 15, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 15, 2024
Copy link

melvin-bot bot commented Oct 15, 2024

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

@shawnborton
Copy link
Contributor

Let's see if the @Expensify/marketing team has any ideas, but I think I like the first bullet of your suggestions.

@NickTooker
Copy link

Please enter a whole dollar amount before continuing.

I think this is fine.

@jamesdeanexpensify
Copy link
Contributor

Double checking - the currency can only be dollars for Expensify Card limits right now, is that correct? Because otherwise "dollar amount" would be incorrect. I just want to make sure, thanks!

@slafortune
Copy link
Contributor

@twilight294 would you like to update your proposal?

@rayane-djouah
Copy link
Contributor

  • Align the $ with the amount box

The alignment of the dollar sign with the amount box is being addressed in #50778

@nyomanjyotisa
Copy link
Contributor

Proposal

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

Expensify Card - improvements to the Set a limit screen

What is the root cause of that problem?

We currently use invalidAmount error message here

errors.limit = translate('iou.error.invalidAmount');

errors.limit = translate('iou.error.invalidAmount');

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

Add new translations, need to confirm the right copy

invalidWholeDollarAmount: 'Please enter a whole dollar amount before continuing.',
invalidWholeDollarAmount: 'Por favor, introduce una cantidad en dólares enteros antes de continuar.',

And change the following code to use the new translation

errors.limit = translate('iou.error.invalidAmount');

errors.limit = translate('iou.error.invalidAmount');

errors.limit = translate('iou.error.invalidWholeDollarAmount');

For the alignment, is being addressed on another issue

What alternative solutions did you explore? (Optional)

@twilight2294
Copy link
Contributor

twilight2294 commented Oct 15, 2024

@twilight294 would you like to update your proposal?

Yes i did update my proposal @slafortune @rayane-djouah here

Note: In my previous proposal I already proposed the steps we have to take where to make the changes and both english and spanish copies as well, even mentioned the edit limit page as well

@shawnborton
Copy link
Contributor

Since we're addressing the currency symbol elsewhere and this is just a copy update, we should probably bump this down to something small like $62.50

@twilight2294
Copy link
Contributor

$125 is fair i guess 😞

@shawnborton
Copy link
Contributor

$125 is too much for a simple copy change.

@rayane-djouah
Copy link
Contributor

The proposal by @twilight294 looks good to me

🎀👀🎀 C+ reviewed

Copy link

melvin-bot bot commented Oct 15, 2024

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

@rayane-djouah
Copy link
Contributor

The proposal involves adding a new copy and adjusting the validation logic on both the "Issue New Card Limit" and "Edit Card Limit" screens. Let's see what @thienlnam thinks of a fair payment amount.

@thienlnam
Copy link
Contributor

Proposal looks good, it's a pretty straightforward change. Perhaps we can just meet in between with $100. @slafortune is handling payment for this, so feel free to make a final decision here

@slafortune slafortune changed the title [$250] Expensify Card - improvements to the Set a limit screen [$100] Expensify Card - improvements to the Set a limit screen Oct 16, 2024
Copy link

melvin-bot bot commented Oct 16, 2024

Upwork job price has been updated to $100

@twilight2294
Copy link
Contributor

@thienlnam can you hire me here :)

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

melvin-bot bot commented Oct 16, 2024

📣 @rayane-djouah 🎉 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

Copy link

melvin-bot bot commented Oct 16, 2024

📣 @twilight294 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 📖

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Oct 17, 2024
@twilight2294
Copy link
Contributor

PR ready for review @rayane-djouah

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
Status: Polish
Development

No branches or pull requests

9 participants