Disable letters in Rate under Track Distance #7931

kakajann · 2022-02-28T05:56:33Z

Details

Keyboard type changed to prevent to type letters in Rate under Track Distance

Fixed Issues

$ #7712

Tests

Verify that no errors appear in the JS console

PR Review Checklist

Contributor (PR Author) Checklist

PR Reviewer Checklist

QA Steps

Launch the app and log in with any account
Go to Setting
Tap on any Workspace you have in your account
Go to Reimburse expenses
Verify you can not type letters

Verify that no errors appear in the JS console

Tested On

Screenshots

Web

Screen.Recording.2022-02-28.at.11.01.37.AM.mov

Mobile Web

Screen.Recording.2022-03-02.at.2.02.08.PM.mov

Desktop

Screen.Recording.2022-03-02.at.1.58.41.PM.mov

iOS

Screen.Recording.2022-02-28.at.10.57.32.AM.mov

Android

Screen.Recording.2022-02-28.at.11.10.19.AM.mov

parasharrajat

There are a couple of things on the checklist.

Please complete those which are required. (not marked with if applicable).

parasharrajat

I will continue my review when all the required checklist items are done and STYLING.md is followed. Thanks.

src/pages/workspace/reimburse/WorkspaceReimburseNoVBAView.js

kakajann · 2022-03-01T06:28:17Z

There are a couple of things on the checklist.

Please complete those which are required. (not marked with if applicable).

There are things that I didn't modify, Styles for example, should I still mark it?

parasharrajat · 2022-03-01T17:10:08Z

Please add screenshots /videos of al platforms and tick the box

parasharrajat · 2022-03-01T17:21:39Z

I am not able to reproduce the issue on staging. Could you please test it @kakajann on prod or staging?

src/pages/workspace/reimburse/WorkspaceReimburseNoVBAView.js

parasharrajat

Thanks for the changes.

src/pages/workspace/reimburse/WorkspaceReimburseNoVBAView.js

Co-authored-by: Rajat Parashar <parasharrajat@users.noreply.github.com>

parasharrajat

It looks good. The only thing that I noticed is that user can enter 10.000.00.000 which include multiple decimals.

pecanoro · 2022-03-03T20:33:07Z

So this should pick up invalid extra decimals [0-9]+\.[0-9]+\., but I am not sure how we could incorporate it, if that regex is detected should we return and not update the value and leave it as it was?

parasharrajat · 2022-03-03T20:38:50Z

Yup, I think so Just don't update the state. That should work. Right?

kakajann · 2022-03-03T20:41:01Z

That's good idea. I'll update the code tomorrow

src/CONST.js

src/pages/workspace/reimburse/WorkspaceReimburseNoVBAView.js

parasharrajat · 2022-03-11T07:24:24Z

src/pages/workspace/reimburse/WorkspaceReimburseNoVBAView.js

@@ -116,6 +102,32 @@ class WorkspaceReimburseNoVBAView extends React.Component {
        }, null);
    }

+    handleKeyPress = (event) => {


No inline functions. Function names should reflect what they do instead of where they are used.

Suggested change

handleKeyPress = (event) => {

debounceUpdateOnCursorMove(event) {

Sorry, I don't see this change. Did you push it?

Whoops, sorry, just a sec

parasharrajat · 2022-03-14T14:48:32Z

@kakajann Please merge main. Thanks.

parasharrajat

Thanks for the changes looks good.

cc: @pecanoro

PR Reviewer Checklist

🎀 👀 🎀 C+ reviewed

OSBotify · 2022-03-14T15:58:22Z

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

OSBotify · 2022-03-15T21:36:36Z

🚀 Deployed to staging by @pecanoro in version: 1.1.43-0 🚀

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

OSBotify · 2022-03-17T17:56:33Z

🚀 Deployed to production by @roryabraham in version: 1.1.43-2 🚀

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

trjExpensify · 2022-03-18T20:09:40Z

👋 @kakajann @parasharrajat @pecanoro - I think this PR has caused a regression that prevents users from entering three decimal places. Can we look into fixing that, please? The default IRS rate is 0.585, so it's pretty important!

sonialiap · 2022-03-18T20:10:19Z

Issue for the decimal place rounding down to two instead of three https://github.com/Expensify/Expensify/issues/202283

kakajann · 2022-03-18T20:19:34Z

I thought we should round the value down to 2 decimal places. I can make it 3 if it's what has to be done

sonialiap · 2022-03-18T20:43:10Z

Yes please! The IRS rate is 3 decimal places and since we must follow their rules, we need 3 :)

disable letters in rate under track distance

bb3fa43

kakajann requested a review from a team as a code owner February 28, 2022 05:56

MelvinBot requested review from parasharrajat and pecanoro and removed request for a team February 28, 2022 05:56

parasharrajat suggested changes Feb 28, 2022

View reviewed changes

kakajann added 2 commits March 1, 2022 11:19

use CONST for keyboard type

ddcf92b

change numValue with rateValue

503f74c

kakajann requested a review from parasharrajat March 1, 2022 06:29

parasharrajat suggested changes Mar 1, 2022

View reviewed changes

cleanup and refactor

27174c1

kakajann requested a review from parasharrajat March 2, 2022 09:03

parasharrajat suggested changes Mar 2, 2022

View reviewed changes

src/pages/workspace/reimburse/WorkspaceReimburseNoVBAView.js Outdated Show resolved Hide resolved

Update src/pages/workspace/reimburse/WorkspaceReimburseNoVBAView.js

6d85ddc

Co-authored-by: Rajat Parashar <parasharrajat@users.noreply.github.com>

parasharrajat suggested changes Mar 2, 2022

View reviewed changes

kakajann added 2 commits March 7, 2022 07:31

prevent invalid rate values

3c5126f

resolve conflicts

5079494

kakajann requested a review from parasharrajat March 7, 2022 02:34

parasharrajat suggested changes Mar 7, 2022

View reviewed changes

kakajann added 4 commits March 9, 2022 18:42

allow infinity number of digits after decimal point and minor refactor

a5ddb35

return numValue.toFixed(2)

9962cf4

call debounced function on arrow left and right

5105c89

minor refactor

65b8a5a

refactor as guideline

2ca9a40

parasharrajat suggested changes Mar 11, 2022

View reviewed changes

kakajann requested a review from parasharrajat March 14, 2022 01:55

no inline functions

4ea4ad1

parasharrajat approved these changes Mar 14, 2022

View reviewed changes

pecanoro approved these changes Mar 14, 2022

View reviewed changes

pecanoro merged commit 39e1ad2 into Expensify:main Mar 14, 2022

OSBotify mentioned this pull request Mar 15, 2022

Deploy Checklist: New Expensify 2022-03-15 #8170

Closed

52 tasks

botify mentioned this pull request Mar 17, 2022

[HOLD for payment on 2022-04-06] Workspace - Able to type any numbers and letters in Rate under Track Distance #7712

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disable letters in Rate under Track Distance #7931

Disable letters in Rate under Track Distance #7931

kakajann commented Feb 28, 2022 •

edited

Loading

parasharrajat left a comment

parasharrajat left a comment

kakajann commented Mar 1, 2022

parasharrajat commented Mar 1, 2022

parasharrajat commented Mar 1, 2022

parasharrajat left a comment •

edited

Loading

parasharrajat left a comment

pecanoro commented Mar 3, 2022

parasharrajat commented Mar 3, 2022 •

edited

Loading

kakajann commented Mar 3, 2022

parasharrajat Mar 11, 2022

kakajann Mar 14, 2022

parasharrajat Mar 14, 2022 •

edited

Loading

kakajann Mar 14, 2022

kakajann Mar 14, 2022

parasharrajat commented Mar 14, 2022

parasharrajat left a comment

OSBotify commented Mar 14, 2022

OSBotify commented Mar 15, 2022

OSBotify commented Mar 17, 2022

trjExpensify commented Mar 18, 2022

sonialiap commented Mar 18, 2022

kakajann commented Mar 18, 2022

sonialiap commented Mar 18, 2022

	handleKeyPress = (event) => {
	debounceUpdateOnCursorMove(event) {

Disable letters in Rate under Track Distance #7931

Disable letters in Rate under Track Distance #7931

Conversation

kakajann commented Feb 28, 2022 • edited Loading

Details

Fixed Issues

Tests

PR Review Checklist

Contributor (PR Author) Checklist

PR Reviewer Checklist

QA Steps

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

parasharrajat left a comment

Choose a reason for hiding this comment

parasharrajat left a comment

Choose a reason for hiding this comment

kakajann commented Mar 1, 2022

parasharrajat commented Mar 1, 2022

parasharrajat commented Mar 1, 2022

parasharrajat left a comment • edited Loading

Choose a reason for hiding this comment

parasharrajat left a comment

Choose a reason for hiding this comment

pecanoro commented Mar 3, 2022

parasharrajat commented Mar 3, 2022 • edited Loading

kakajann commented Mar 3, 2022

parasharrajat Mar 11, 2022

Choose a reason for hiding this comment

kakajann Mar 14, 2022

Choose a reason for hiding this comment

parasharrajat Mar 14, 2022 • edited Loading

Choose a reason for hiding this comment

kakajann Mar 14, 2022

Choose a reason for hiding this comment

kakajann Mar 14, 2022

Choose a reason for hiding this comment

parasharrajat commented Mar 14, 2022

parasharrajat left a comment

Choose a reason for hiding this comment

PR Reviewer Checklist

OSBotify commented Mar 14, 2022

OSBotify commented Mar 15, 2022

OSBotify commented Mar 17, 2022

trjExpensify commented Mar 18, 2022

sonialiap commented Mar 18, 2022

kakajann commented Mar 18, 2022

sonialiap commented Mar 18, 2022

kakajann commented Feb 28, 2022 •

edited

Loading

parasharrajat left a comment •

edited

Loading

parasharrajat commented Mar 3, 2022 •

edited

Loading

parasharrajat Mar 14, 2022 •

edited

Loading