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

Add Price formatter and Vis comp #465

Merged
merged 11 commits into from
Feb 10, 2025
Merged

Add Price formatter and Vis comp #465

merged 11 commits into from
Feb 10, 2025

Conversation

bthaile
Copy link
Collaborator

@bthaile bthaile commented Feb 5, 2025

Related Issue

Closes #454

Adds formatting price or value to create a subscript
Used on coin list page
Used on activities page
Added tests for formatting value helper

Summary of Changes

Addes helper to format prices,

Need Regression Testing

  • Yes
  • No

Risk Assessment

  • Low
  • Medium
  • High

Additional Notes

This only a UI component with helper method and tests. It's not been integrated into the existing UI.

Screenshots (if applicable)

using scientific subscript notation. 0.0(4)12 means four zeros after decimal

image

image

@bthaile bthaile mentioned this pull request Feb 5, 2025
5 tasks
@@ -1174,7 +1174,10 @@ export class WalletController extends BaseController {

private async evmtokenPrice(tokeninfo, data) {
const token = tokeninfo.symbol.toLowerCase();
const price = await openapiService.getPricesByEvmaddress(tokeninfo.address, data);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Was this a bug?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I noticed that tokenInfo address was the cadence address and evmAddress was a property on TokenInfo. In effect it was a bug.

Copy link
Collaborator

@tombeckenham tombeckenham left a comment

Choose a reason for hiding this comment

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

Please confirm if 0.0001 is 0.0 (sub 3) 1 or 0.0 (sub 4) 1. You've implemented it as 0.0 (sub 3) 1 but at the very least there are some communties that believe this should be sub 4 - indicating 4 zeros. Please confirm where you've seen this notation before this is merged.

@Kay-Zee
Copy link
Member

Kay-Zee commented Feb 7, 2025

@tombeckenham I think what you're trying to say is actually that:

0.00001 should be 0.0(subscript4)1, and not 0.0(subscript3)1 that the code currently does. (it's just your example used 0.0001)

@bthaile See this example:

Screenshot 2025-02-06 at 6 23 42 PM

Think of it as if we're talking chemistry. H(subscript2)0 means theres 2 H's, including the one that's written, so 0.0(subscript 4)1 means there are 4 zeros, including the one displayed.

The only thing that has to change is remove the minus 1 in the total zeros - 1.

@Kay-Zee Kay-Zee self-requested a review February 7, 2025 02:33
Copy link
Collaborator

@tombeckenham tombeckenham left a comment

Choose a reason for hiding this comment

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

Thanks Kan. Thank you for clarifying! That's what's needed

@bthaile
Copy link
Collaborator Author

bthaile commented Feb 7, 2025

Updated PR to use correct scientific notion subscripts

@bthaile bthaile merged commit cbc68e1 into dev Feb 10, 2025
3 checks passed
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.

3 participants