-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix: cp-7.46.0 add percent change for asset list v2 #14892
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
base: main
Are you sure you want to change the base?
Conversation
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
|
app/component-library/components-temp/Price/AggregatedPercentage/utils.ts
Outdated
Show resolved
Hide resolved
pricePercentChange1d = | ||
allMultichainAssetsRates[asset?.address as CaipAssetType]?.marketData | ||
?.pricePercentChange?.P1D; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the above variable is codefenced, should this also be code-fenced?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would assume we'd at least need a !isEvmNetwork
check. Since Bitrise is passing, I guess this is actually fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm interesting!
I all for ditching codefences (they are so painful to debug lol)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am surprised this is working as is. Given that the allMultichainAssetsRates
is behind a code fence, this variable would not exist on a main build and should throw an error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactored here 08e6bd6
app/component-library/components-temp/Price/AggregatedPercentage/utils.ts
Show resolved
Hide resolved
app/component-library/components-temp/Price/AggregatedPercentage/NonEvmAggregatedPercentage.tsx
Show resolved
Hide resolved
02bd9fe
b499a35
to
984beec
Compare
|
ad05484
to
ac59af1
Compare
|
ac59af1
to
08e6bd6
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #14892 +/- ##
==========================================
+ Coverage 67.71% 67.84% +0.12%
==========================================
Files 2329 2343 +14
Lines 50144 50443 +299
Branches 7373 7444 +71
==========================================
+ Hits 33954 34221 +267
- Misses 14054 14060 +6
- Partials 2136 2162 +26 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Description
Adds asset list 24h price change
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist