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

[HOLD for payment 2021-09-27] IOU - Modal is shifted to the left when entering a long amount number #5115

Closed
isagoico opened this issue Sep 7, 2021 · 35 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@isagoico
Copy link

isagoico commented Sep 7, 2021

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Navigate to NewDot
  2. Click on request money
  3. Enter an amount of over 15 numbers

Expected Result:

Modal should stay on the visible area of the screen

Actual Result:

Modal is shifted to the left and button is not completely visible.

Workaround:

N/A

Platform:

Where is this issue confirmed?

  • Web

Version Number: 1.0.94-0

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

image

Expensify/Expensify Issue URL:

View all open jobs on GitHub


From @akshayasalvi https://expensify.slack.com/archives/C01GTK53T8Q/p1630779418448200

When I enter the 15 to 20 digits long amount in Request Money it goes out of the screen (amount is cut). Also, the button aligned is misplaced. Screenshot attached in the thread.

@MelvinBot
Copy link

Triggered auto assignment to @bondydaa (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@akshayasalvi
Copy link
Contributor

Proposed Solution

If we know that we're going to have a max value for the amount then we should add a maxLength property to the amount field. I added 14 and it worked fine. Then basically it didn't messed up the alignment of the the button as well.

<TextInputAutoWidth
   inputStyle={styles.iouAmountTextInput}
   ...
   ...
   ...
   value={this.state.amount}
   maxLength={14}
/>

@bondydaa
Copy link
Contributor

bondydaa commented Sep 7, 2021

15 digits would be 900 billion max, currently the richest person in the world is Jeff Bezos with an estimated 177 billion and while I do hope we eventually have Mr Bezos using expensify to settle up his IOUs I don't think 15-20 digit numbers are going to be super common use case here so I think I agree we should just add a maxLength attribute here and be done with it.

@bondydaa bondydaa removed their assignment Sep 7, 2021
@bondydaa bondydaa added the External Added to denote the issue can be worked on by a contributor label Sep 7, 2021
@MelvinBot
Copy link

Triggered auto assignment to @stephanieelliott (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@bondydaa
Copy link
Contributor

bondydaa commented Sep 7, 2021

i don't think this needs to be a daily so once the job is created we could move this to weekly or monthly.

@isagoico
Copy link
Author

isagoico commented Sep 7, 2021

Maybe countries with high inflations might be affected by this, e.g Venezuela, currently 1$ is equal to 4.200.000 Bs. So having an IOU of 1000$ would be 4.200.000.000 Bs. It still doesn't reach the 14 digit mark so I agree with Bondy that this will not be a common issue.

@bondydaa
Copy link
Contributor

bondydaa commented Sep 7, 2021 via email

@isagoico
Copy link
Author

isagoico commented Sep 7, 2021

Yeah that crossed my mind too but for the time being I believe IOUs can only be in USD

That's not correct. At the moment IOUs can be in any currency available in the currency list.

image

@akshayasalvi
Copy link
Contributor

Considering that the issue is uncommon, I suggested the maxLength fix rather than the style change in the IOUModal and related child components.

@MelvinBot MelvinBot added Weekly KSv2 Help Wanted Apply this label when an issue is open to proposals by contributors and removed Daily KSv2 labels Sep 9, 2021
@MelvinBot
Copy link

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

@stephanieelliott
Copy link
Contributor

Posted to Upwork here: https://www.upwork.com/jobs/~013d0ec60b16212278

@deetergp looks like we have a proposal in this comment.

@deetergp
Copy link
Contributor

deetergp commented Sep 9, 2021

I agree that we can kick the can down the street on 15+ digit numbers for the time being. Let's go with @akshayasalvi proposal of just adding a max length of 14 digits.

@botify
Copy link

botify commented Sep 13, 2021

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@stephanieelliott
Copy link
Contributor

stephanieelliott commented Sep 13, 2021

Hey @akshayasalvi can you please propose to the job post in Upwork so I can hire and pay you for this: https://www.upwork.com/jobs/~013d0ec60b16212278

@MelvinBot MelvinBot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 13, 2021
@akshayasalvi
Copy link
Contributor

@stephanieelliott Done. Thanks for the help.

@botify
Copy link

botify commented Sep 13, 2021

🚀 Deployed to staging by @marcaaron in version: 1.0.97-1 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@botify botify added Weekly KSv2 and removed Weekly KSv2 labels Sep 15, 2021
@botify botify changed the title [HOLD for payment 2021-09-22] IOU - Modal is shifted to the left when entering a long amount number [HOLD for payment 2021-09-22] [HOLD for payment 2021-09-22] IOU - Modal is shifted to the left when entering a long amount number Sep 15, 2021
@botify
Copy link

botify commented Sep 15, 2021

🚀 Deployed to production by @francoisl in version: 1.0.98-1 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

1 similar comment
@botify
Copy link

botify commented Sep 15, 2021

🚀 Deployed to production by @francoisl in version: 1.0.98-1 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@botify
Copy link

botify commented Sep 15, 2021

🚀 Deployed to staging by @francoisl in version: 1.0.98-2 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@botify botify added Weekly KSv2 and removed Weekly KSv2 labels Sep 20, 2021
@botify botify changed the title [HOLD for payment 2021-09-22] [HOLD for payment 2021-09-22] IOU - Modal is shifted to the left when entering a long amount number [HOLD for payment 2021-09-27] [HOLD for payment 2021-09-22] [HOLD for payment 2021-09-22] IOU - Modal is shifted to the left when entering a long amount number Sep 20, 2021
@botify
Copy link

botify commented Sep 20, 2021

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

If no regressions arise, payment will be issued on 2021-09-27. 🎊

@mountiny mountiny changed the title [HOLD for payment 2021-09-27] [HOLD for payment 2021-09-22] [HOLD for payment 2021-09-22] IOU - Modal is shifted to the left when entering a long amount number [HOLD for payment 2021-09-22] IOU - Modal is shifted to the left when entering a long amount number Sep 20, 2021
@mountiny
Copy link
Contributor

This message is a bug, caused by today's deploy and some weird GH API behaviour. Updated the title back to what it was 🙇

@stephanieelliott stephanieelliott changed the title [HOLD for payment 2021-09-22] IOU - Modal is shifted to the left when entering a long amount number [HOLD for payment 2021-09-27] IOU - Modal is shifted to the left when entering a long amount number Sep 24, 2021
@akshayasalvi
Copy link
Contributor

@stephanieelliott Is this payment on hold?

@mountiny
Copy link
Contributor

Was there a regression from this PR? If not, it should have been paid on 22nd September. Sorry for confusion because of the bot message 😬 @stephanieelliott

@akshayasalvi
Copy link
Contributor

akshayasalvi commented Oct 4, 2021

@stephanieelliott @mountiny Still waiting for this one to get paid out.

@stephanieelliott @deetergp Still waiting for this one to get paid out.

@mountiny Sorry for tagging you. I thought you were the CME.

@mountiny
Copy link
Contributor

mountiny commented Oct 4, 2021

Yikes, apologies @akshayasalvi, we are having a bit busy times so some tasks simply slip from our radar. I will ping someone!

@akshayasalvi
Copy link
Contributor

Thanks for the quick response @mountiny. Any expected date, so that I don't have to ping you :P

@mountiny
Copy link
Contributor

mountiny commented Oct 4, 2021

@akshayasalvi I think someone will do it today for sure, I have pinged our team and there will be at least one one person with time to handle this. Sorry for the delay and we appreciate you patience 🙇

@dylanexpensify
Copy link
Contributor

Scheduling payment now - sorry for the delay @akshayasalvi !

@dylanexpensify
Copy link
Contributor

Payment sent, job posting closed!

@akshayasalvi
Copy link
Contributor

@dylanexpensify I received half the payment I guess. I reported + resolved the issue.

@dylanexpensify
Copy link
Contributor

Ahh my apologies @akshayasalvi! One second

@dylanexpensify dylanexpensify reopened this Oct 4, 2021
@akshayasalvi
Copy link
Contributor

@akshayasalvi I think someone will do it today for sure, I have pinged our team and there will be at least one one person with time to handle this. Sorry for the delay and we appreciate you patience 🙇

Thanks @mountiny. Received help on this. :)

@akshayasalvi
Copy link
Contributor

Ahh my apologies @akshayasalvi! One second

No problem @dylanexpensify. Sorry if my follow up got you rushing into this

@dylanexpensify
Copy link
Contributor

@akshayasalvi no no, you're all good!! I just sent over the bonus 😄

@akshayasalvi
Copy link
Contributor

@akshayasalvi no no, you're all good!! I just sent over the bonus 😄

Thank you @dylanexpensify and @mountiny 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

9 participants