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

Fix: bigNumberToString fn also return <0.001 #2215

Closed
wants to merge 12 commits into from
Closed

Conversation

Frankaus
Copy link
Contributor

@Frankaus Frankaus commented Aug 17, 2021

closes #2214

Testing
After merging #2169 you can test this feature in the Set Outcome stage of the market bond component in the Potential Fee entry row having <0.001 while other entries in market buy still show <0.01

@Frankaus Frankaus self-assigned this Aug 17, 2021
Copy link
Contributor

@Mi-Lan Mi-Lan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left feedback on how I think this should be done

Comment on lines 36 to 38
if (Number(number) < 0.001 && Number(number) > 0 && decimals > 2) {
return '<0.001'
}
Copy link
Contributor

@Mi-Lan Mi-Lan Aug 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Frankaus This works but I don't think its the best approach. So we wanna make it so that according to number of decimal places (precision) we define it checks for that amount of less then decimal places.
For example :
If we have one decimal place it will evaluate to <0.1
if we have 2 decimals it will evaluate to <0.01
if we have 3 decimals it will evaluate to <0.001
if we have 4 decimal it will evaluate to <0.0001
etc....
And we should make it in one place and to do this programatically not hardcoding it...

Copy link
Contributor Author

@Frankaus Frankaus Aug 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely better approach yes. I thought of a for loop and it works on my machine. That should do the job, hope you like it. Thank you!

@Frankaus Frankaus requested review from Mi-Lan, hexyls, kadenzipfel and pimato and removed request for kadenzipfel and hexyls August 18, 2021 08:40
@Frankaus Frankaus changed the title bigNumberToString fn also return <0.001 Fix: bigNumberToString fn also return <0.001 Aug 18, 2021
@pimato pimato changed the base branch from master to feature/2073-v2 August 18, 2021 14:57
Comment on lines 37 to 42
if (Number(number) > 0) {
for (let i = 1; i < 10; i++) {
if (Number(number) < Number('0.' + '0'.repeat(i) + '1') && decimals === i + 1) {
return '<0.' + '0'.repeat(i) + '1'
}
}
Copy link
Contributor

@Mi-Lan Mi-Lan Aug 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Frankaus Why are we making this loop which loops to 10? I think because we have decimals we can just determine which evaluation we need to do and use the value for evaluation to be based on number of decimals....

Base automatically changed from feature/2073-v2 to master August 24, 2021 12:40
@Frankaus Frankaus requested a review from Mi-Lan August 26, 2021 14:24
@Mi-Lan
Copy link
Contributor

Mi-Lan commented Aug 26, 2021

Screenshot 2021-08-26 at 17 52 22

@Frankaus Im getting this error. Please test pr-s first its like this on main page or did I do something wrong?

@Mi-Lan Mi-Lan requested a review from hexyls August 31, 2021 11:29
@Mi-Lan
Copy link
Contributor

Mi-Lan commented Aug 31, 2021

@hexyls Hey men would be good if you have time to take a look I added tests also and it should act as expected...But haven't dwelled that much how it looks on components themselves or if it has some issues. But should work in theory...

Copy link
Contributor

@hexyls hexyls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@Frankaus Frankaus closed this Dec 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bigNumberToString fn should also format < x.xx1
4 participants