EmerisHQ / demeris

Emeris web app
https://app.emeris.com/
Apache License 2.0
12 stars 2 forks source link

fix: resolve price and currency big number #1664

Closed Dawntraoz closed 2 years ago

Dawntraoz commented 2 years ago

Description

Add BigNumber to Price and accept it in CurrencyDisplay. Resolve warning for optional props making them optional.

Fixes: #1662

Testing

cloudflare-pages[bot] commented 2 years ago

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 01dedf1
Status: ✅  Deploy successful!
Preview URL: https://10585c78.emeris-app.pages.dev

View logs

Dawntraoz commented 2 years ago

As this PR is introducing so many errors on the web app, I setted up this PR as draft until we have a proper solution to deal with BigNumber.

Maybe will be a nice idea to make the BigNumber case for big precision (outside JavaScript decimal range: 15) for now to fix #1620 & #1662, and then refactor the whole calculation in another PR.

UPDATE: Bugs fixed, now is working properly in the whole app.

github-actions[bot] commented 2 years ago

Visit the preview URL for this PR (updated for commit fa5168d):

https://emeris-app--pr1664-fix-price-amount-not-39ivbvt8.web.app

(expires Mon, 30 May 2022 08:25:52 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

eitjuh commented 2 years ago

Sorting of Rowan on the main portfolio page + staking table is not done by price (as all other assets):

image
josietyleung commented 2 years ago

An account with a tiny amount of ROWEN is showing the full 18 decimals, is this expected? image

Another account with ROWEN balance that has 3 digits to the left is showing 2 decimals 😊

+1 on asset in Staking table's not sorted by balance anymore

Dawntraoz commented 2 years ago

An account with a tiny amount of ROWEN is showing the full 18 decimals, is this expected? image

Another account with ROWEN balance that has 3 digits to the left is showing 2 decimals 😊

@josietyleung exactly, we are not cropping decimal values if your quantity on the left is less than 3 digits. For that, you see 18 precision on decimals when lower quantities and 2 decimals when more than 999.xx.

Dawntraoz commented 2 years ago

+1 on asset in Staking table's not sorted by balance anymore

@josietyleung will try to get some ROWANs to be able to test the sorting, because for normal assets is working fine :(

josietyleung commented 2 years ago

An account with a tiny amount of ROWEN is showing the full 18 decimals, is this expected? image Another account with ROWEN balance that has 3 digits to the left is showing 2 decimals 😊

@josietyleung exactly, we are not cropping decimal values if your quantity on the left is less than 3 digits. For that, you see 18 precision on decimals when lower quantities and 2 decimals when more than 999.xx.

Ah perfect then! Apart from the sorting (not sure if it's related to this PR 😅 ) everything looks great!

faboweb commented 2 years ago

There is so much parsing and converting happening everywhere in this PR. Like so much logic in the view layer. Can we simplify this somehow by converting numbers closer to the data getters?

Dawntraoz commented 2 years ago

There is so much parsing and converting happening everywhere in this PR. Like so much logic in the view layer. Can we simplify this somehow by converting numbers closer to the data getters?

In the first place, this was just a change in 2 components Price.vue and AmountDisplay.vue, but then the Spaghetti code came into our life 🤯 This should be refactored for sure, but as part of this PR @eitjuh?

PS: Simply to know where to place my efforts.

faboweb commented 2 years ago

In the first place, this was just a change in 2 components Price.vue and AmountDisplay.vue, but then the Spaghetti code came into our life 🤯 This should be refactored for sure, but as part of this PR @eitjuh?

sure let's create a ticket and do later