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: remove unnatural Ada rounding #1442

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

rhyslbw
Copy link
Member

@rhyslbw rhyslbw commented Sep 22, 2024

Ada naturally supports 6 decimal points, and we should be supporting this precision

@rhyslbw rhyslbw requested a review from a team as a code owner September 22, 2024 10:31
@pczeglik-iohk
Copy link
Contributor

pczeglik-iohk commented Sep 22, 2024

Allure Report

allure-report-publisher generated test report!

processReports: ❌ test report for 6eecf095

passed failed skipped flaky total result
Total 29 3 4 0 36

@rhyslbw rhyslbw marked this pull request as draft September 22, 2024 11:56
@mirceahasegan
Copy link
Contributor

At the moment it seems that UI components will start displaying all 6 decimals if present.
I think the UI components should also be updated to display max 2 decimal points, while showing the rest in another way (i.e. in a tooltip)

@rhyslbw rhyslbw force-pushed the fix/ada-decimal-precisiom branch 2 times, most recently from 876b6a3 to 1c0fd20 Compare September 23, 2024 08:46
- Ada has a 6 decimal point precision, but currently we're rounding to 2
- Decimals on the CoinId type is dropped to simplify the implementation,
as Ada has a constant value and it's not used for any other currencies
- Fixes some wonky logic that was working due to the common rounding to
2 decimal points
Copy link

sonarcloud bot commented Sep 23, 2024

@rhyslbw rhyslbw changed the title fix: round Ada naturally fix: remove unnatural Ada rounding Sep 23, 2024
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.

4 participants