Show last chart data as non-hover price on token info page #4016
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
4 Skipped Deployments
|
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
packages/web/components/pages/asset-info-page/chart.tsxOops! Something went wrong! :( ESLint: 8.50.0 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/packages/web/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. WalkthroughThe pull request modifies the Changes
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (2)packages/web/components/pages/asset-info-page/chart.tsx (2)
The change correctly implements the PR objective by consistently showing the last chart data price instead of falling back to market price. The code is more maintainable with simplified logic.
Since we're now always using chart data instead of potentially fresher market prices, please verify that this doesn't create noticeable price discrepancies in the UI, especially during high volatility periods. ✅ Verification successfulPrice consistency is maintained through synchronized data sources Based on the code analysis, the change is safe because:
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Search for any price comparison or validation logic in the codebase
# that might be affected by this change
# Look for files comparing chart prices with market prices
rg -l "currentPrice|lastChartData.*close" --type ts --type tsx
# Look for price-related test files
fd "price.*test\.(ts|tsx)$"
Length of output: 122 Script: #!/bin/bash
# Search for any price comparison or validation logic in the codebase
# Let's try with ripgrep without file type restrictions and focus on the patterns
# Look for files comparing chart prices with market prices
rg -l "currentPrice|lastChartData\.close"
# Let's also check for any price validation or comparison logic
rg -l "validatePrice|comparePrice|priceComparison"
# And check for any test files related to asset info or price
find . -type f -name "*price*test*" -o -name "*asset*test*"
Length of output: 2475 Script: #!/bin/bash
# Let's examine the specific usage of lastChartData and price comparisons in relevant files
# Check the context of lastChartData usage in the chart component
rg -B 3 -A 3 "lastChartData.*close" "packages/web/components/pages/asset-info-page/chart.tsx"
# Check how prices are used in asset info components
rg -B 3 -A 3 "currentPrice|lastChartData" "packages/web/components/table/asset-info.tsx"
# Check price-related logic in historical data hook
rg -B 3 -A 3 "currentPrice|lastChartData" "packages/web/hooks/ui-config/use-historical-and-depth-data.ts"
Length of output: 2687 Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
| data = | ||
| marketAsset?.currentPrice?.toDec() ?? | ||
| new Dec(assetInfoConfig.lastChartData?.close ?? 0); | ||
| data = new Dec(assetInfoConfig.lastChartData?.close ?? 0); |
There was a problem hiding this comment.
Have we verified whether this price reflects the most current value? I’m concerned that discrepancies might appear throughout the app
There was a problem hiding this comment.
The difference between the two is noticeable but small.
There has been a discussion going on with Numia talking about that. As of now data from Numia and SQS differ so the last point on the chart and the price displayed when no hover is performed are different and inconsistent. They tried to make the SQS value the same as Numia but they ultimately decided to just use the Numia value in this page
There was a problem hiding this comment.
We should investigate this further because the currentPrice is sourced from Numia instead of SQS. Changing this might introduce risk since more features in the DEX rely on the currentPrice rather than the last chart value. I’d like to avoid potential discrepancies, especially since we recently resolved a related bug in #4015
No description provided.