Fix issue with numbers added after decimal point appearing preceding the decimal point#11177
Merged
chiragsalian merged 1 commit intoExpensify:mainfrom Sep 26, 2022
Conversation
NikkiWines
approved these changes
Sep 23, 2022
Contributor
NikkiWines
left a comment
There was a problem hiding this comment.
Looks good + works well. Adding @chiragsalian as a reviewer since he has context from the original PR
|
@chiragsalian looks like this was merged without passing tests. Please add a note explaining why this was done and remove the |
Contributor
|
Not emergency, tests were passing fine. |
19 tasks
Contributor
|
🚀 Deployed to staging by @chiragsalian in version: 1.2.7-1 🚀
|
Contributor
|
🚀 Deployed to production by @roryabraham in version: 1.2.7-2 🚀
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Details
This is a fix for the regression caused by #10147.
This is a controlled input, but currently in updateAmount we update
amount, but don't update theselection. All we need to do isupdateAmount(text) { this.setState((prevState) => { const amount = this.addLeadingZero(this.replaceAllDigits(text, this.props.fromLocaleDigit)); + const selection = this.getNewSelection(prevState.selection, prevState.amount.length, amount.length); return this.validateAmount(amount) - ? {amount: this.stripCommaFromAmount(amount)} + ? {amount: this.stripCommaFromAmount(amount), selection} : prevState; }); }However I noticed that now this piece of code is repeated 3 times, so created a separate method for it
Fixed Issues
$ #10928
Tests
0.and numbers are being added after the decimal separatorPR Review Checklist
Contributor (PR Author) Checklist
### Fixed Issuessection aboveTestssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*filesSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */displayNamepropertythisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Avataris modified, I verified thatAvataris working as expected in all cases)PR Reviewer Checklist
The Contributor+ will copy/paste it into a new comment and complete it after the author checklist is completed
### Fixed Issuessection aboveTestssectionQA stepssectiontoggleReportand notonIconClick).src/languages/*filesSTYLE.md) were followedAvatar, I verified the components usingAvatarhave been tested & I retested again)/** comment above it */displayNamepropertythisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Avataris modified, I verified thatAvataris working as expected in all cases)QA Steps
0.and numbers are being added after the decimal separatorScreenshots
Web
Screen.Recording.2022-09-21.at.20.23.44.mov
Mobile Web - Chrome
22-09-21-21-31-46.mp4
Mobile Web - Safari
Screen.Recording.2022-09-21.at.20.35.08.mov
Desktop
Screen.Recording.2022-09-21.at.20.41.41.mov
iOS
Screen.Recording.2022-09-21.at.20.33.43.mov
Android
22-09-21-21-08-13.mp4