Moves chain manager to pinia #850
Conversation
Deploying open-block-explorer with
|
| Latest commit: |
025b26c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://96b60468.open-block-explorer.pages.dev |
| Branch Preview URL: | https://chainmanager-to-pinia.open-block-explorer.pages.dev |
✅ Deploy Preview for obe-testnet ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for obe-staging ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| try { | ||
| const data: PriceChartData = await networksStore.getCurrentNetwork.getPriceData(); | ||
| tokenPrice.value = formatCurrencyValue(data.tokenPrice); | ||
| dayChange.value = formatPercentage(data.dayChange); | ||
| dayVolume.value = formatCurrencyValue(data.dayVolume); | ||
| marketCap.value = formatCurrencyValue(data.marketCap); | ||
| chartOptions.value.series[0].data = data.prices; | ||
| } catch(e) { | ||
| console.error('No price data available', e); | ||
| } |
There was a problem hiding this comment.
I think this try-catch is super correct !!!
However, instead of simply showing the empty fields in the HTML, I would recommend showing a "Price not available" text or something similar.
There was a problem hiding this comment.
The 'getPriceData' is an abstract method on the BaseChain class and each chain config file will specify if should call the coingecko API or return an empty object (you can compare both config files for telos and telos-testnet to see that they calls a different function on the price.ts file). Adding this on the catch will not solve the empty charts and will not show the 'Price not available' to the users for chains that doens't implement the call to the API.
For sure I can address this comment and implement this new behavior, but I think it's something out of scope of the proposal on this PR. I would suggest to we create an issue and work on this sepparately.
What do you think?
|
Hey @Viterbo, addressed your comments! The one regarding the Price Chart, I put a comment there, I believe this should be addressed in a different PR since it requires a change of behavior on the UX perspective. |
Description
Test scenarios
Checklist: