Implemented home page with TradingView widgets#4
Merged
Conversation
Contributor
WalkthroughIntroduces a client-side TradingViewWidget and a supporting hook to inject TradingView scripts, updates the root page to render multiple market widgets, and adds extensive constants for widget configurations, symbols, and UI options. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Page as Root Page
participant Widget as TradingViewWidget
participant Hook as useTradingViewWidget
participant DOM as Document
participant TV as tradingview.com Script
User->>Page: Load /
Page->>Widget: Render with {scriptUrl, config, height}
Widget->>Hook: useTradingViewWidget(scriptUrl, config, height)
Hook->>DOM: Create container ref
alt first mount or not loaded
Hook->>DOM: Inject <script src=scriptUrl async>
DOM-->>TV: Request script
TV-->>DOM: Load and execute
DOM->>DOM: Initialize widget with config into container
else already loaded
Hook->>DOM: Skip reinjection
end
Widget-->>User: Displays TradingView widget
note over Hook,DOM: On unmount: clear container and reset loaded flag
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/(root)/page.tsx(1 hunks)components/TradingViewWidget.tsx(1 hunks)hooks/useTradingViewWidget.tsx(1 hunks)lib/constants.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
app/(root)/page.tsx (1)
lib/constants.ts (3)
MARKET_OVERVIEW_WIDGET_CONFIG(40-96)MARKET_DATA_WIDGET_CONFIG(128-171)TOP_STORIES_WIDGET_CONFIG(117-126)
components/TradingViewWidget.tsx (1)
lib/utils.ts (1)
cn(4-6)
Comment on lines
+21
to
+27
| <TradingViewWidget | ||
| title="Stock Heatmap" | ||
| scriptUrl={`${scriptUrl}stock-heatmap.js`} | ||
| config={MARKET_OVERVIEW_WIDGET_CONFIG} | ||
| className="custom-chart" | ||
| height={600} | ||
| /> |
Contributor
There was a problem hiding this comment.
Hook the Heatmap widget to the correct config.
The heatmap embed expects the heatmap configuration (with dataSource, blockSize, etc.). Feeding it the market overview config leaves those keys missing and the widget fails to initialize. Wire it to HEATMAP_WIDGET_CONFIG.
Apply this diff:
-import TradingViewWidget from "@/components/TradingViewWidget";
-import {MARKET_DATA_WIDGET_CONFIG, MARKET_OVERVIEW_WIDGET_CONFIG, TOP_STORIES_WIDGET_CONFIG} from "@/lib/constants";
+import TradingViewWidget from "@/components/TradingViewWidget";
+import {HEATMAP_WIDGET_CONFIG, MARKET_DATA_WIDGET_CONFIG, MARKET_OVERVIEW_WIDGET_CONFIG, TOP_STORIES_WIDGET_CONFIG} from "@/lib/constants";
@@
<TradingViewWidget
title="Stock Heatmap"
scriptUrl={`${scriptUrl}stock-heatmap.js`}
- config={MARKET_OVERVIEW_WIDGET_CONFIG}
+ config={HEATMAP_WIDGET_CONFIG}
className="custom-chart"
height={600}
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <TradingViewWidget | |
| title="Stock Heatmap" | |
| scriptUrl={`${scriptUrl}stock-heatmap.js`} | |
| config={MARKET_OVERVIEW_WIDGET_CONFIG} | |
| className="custom-chart" | |
| height={600} | |
| /> | |
| // app/(root)/page.tsx | |
| import TradingViewWidget from "@/components/TradingViewWidget"; | |
| import {HEATMAP_WIDGET_CONFIG, MARKET_DATA_WIDGET_CONFIG, MARKET_OVERVIEW_WIDGET_CONFIG, TOP_STORIES_WIDGET_CONFIG} from "@/lib/constants"; | |
| … | |
| <TradingViewWidget | |
| title="Stock Heatmap" | |
| scriptUrl={`${scriptUrl}stock-heatmap.js`} | |
| config={HEATMAP_WIDGET_CONFIG} | |
| className="custom-chart" | |
| height={600} | |
| /> |
🤖 Prompt for AI Agents
In app/(root)/page.tsx around lines 21 to 27 the TradingViewWidget for the Stock
Heatmap is using MARKET_OVERVIEW_WIDGET_CONFIG which lacks heatmap-specific keys
(dataSource, blockSize, etc.); replace the config prop with
HEATMAP_WIDGET_CONFIG so the widget receives the proper heatmap configuration
and initializes correctly.
Comment on lines
+7
to
+27
| interface TradingViewWidgetProps{ | ||
| title: string; | ||
| scriptUrl: string; | ||
| config: Record<string, unknown>; | ||
| height?: number; | ||
| className?: string; | ||
| } | ||
|
|
||
| const TradingViewWidget = ({title, scriptUrl, config, height = 600, className}: TradingViewWidgetProps) => { | ||
| const containerRef = useTradingViewWidget(scriptUrl, config, height); | ||
|
|
||
|
|
||
| return ( | ||
| <div className="w-full"> | ||
| {title && <h3 className="font-semibold text-2xl text-gray-100 mb-5">{title}</h3>} | ||
| <div className={cn('tradingview-widget-container', className)} ref={containerRef}> | ||
| <div className="tradingview-widget-container__widget" style={{ height, width: "100%" }}/> | ||
| </div> | ||
| </div> | ||
|
|
||
| ); |
Contributor
There was a problem hiding this comment.
Mark title as optional to satisfy current usage.
title is rendered conditionally, but the prop is typed as required. The imports on Line 32 in page.tsx omit title, which causes a TS compile-time error. Make title optional.
Apply this diff:
-interface TradingViewWidgetProps{
- title: string;
+interface TradingViewWidgetProps{
+ title?: string;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| interface TradingViewWidgetProps{ | |
| title: string; | |
| scriptUrl: string; | |
| config: Record<string, unknown>; | |
| height?: number; | |
| className?: string; | |
| } | |
| const TradingViewWidget = ({title, scriptUrl, config, height = 600, className}: TradingViewWidgetProps) => { | |
| const containerRef = useTradingViewWidget(scriptUrl, config, height); | |
| return ( | |
| <div className="w-full"> | |
| {title && <h3 className="font-semibold text-2xl text-gray-100 mb-5">{title}</h3>} | |
| <div className={cn('tradingview-widget-container', className)} ref={containerRef}> | |
| <div className="tradingview-widget-container__widget" style={{ height, width: "100%" }}/> | |
| </div> | |
| </div> | |
| ); | |
| interface TradingViewWidgetProps { | |
| title?: string; | |
| scriptUrl: string; | |
| config: Record<string, unknown>; | |
| height?: number; | |
| className?: string; | |
| } | |
| const TradingViewWidget = ({ title, scriptUrl, config, height = 600, className }: TradingViewWidgetProps) => { | |
| const containerRef = useTradingViewWidget(scriptUrl, config, height); | |
| return ( | |
| <div className="w-full"> | |
| {title && <h3 className="font-semibold text-2xl text-gray-100 mb-5">{title}</h3>} | |
| <div className={cn('tradingview-widget-container', className)} ref={containerRef}> | |
| <div | |
| className="tradingview-widget-container__widget" | |
| style={{ height, width: "100%" }} | |
| /> | |
| </div> | |
| </div> | |
| ); | |
| }; |
🤖 Prompt for AI Agents
In components/TradingViewWidget.tsx around lines 7 to 27, the title prop is
typed as required but rendered conditionally and omitted at call sites; change
the prop to optional by updating the TradingViewWidgetProps interface to title?:
string (and keep the component destructuring as-is) so TypeScript no longer
errors when title is not provided.
Comment on lines
+235
to
+263
| symbol: symbol.toUpperCase(), | ||
| colorTheme: 'dark', | ||
| isTransparent: 'true', | ||
| locale: 'en', | ||
| width: '100%', | ||
| height: 400, | ||
| interval: '1h', | ||
| largeChartUrl: '', | ||
| }); | ||
|
|
||
| export const COMPANY_PROFILE_WIDGET_CONFIG = (symbol: string) => ({ | ||
| symbol: symbol.toUpperCase(), | ||
| colorTheme: 'dark', | ||
| isTransparent: 'true', | ||
| locale: 'en', | ||
| width: '100%', | ||
| height: 440, | ||
| }); | ||
|
|
||
| export const COMPANY_FINANCIALS_WIDGET_CONFIG = (symbol: string) => ({ | ||
| symbol: symbol.toUpperCase(), | ||
| colorTheme: 'dark', | ||
| isTransparent: 'true', | ||
| locale: 'en', | ||
| width: '100%', | ||
| height: 464, | ||
| displayMode: 'regular', | ||
| largeChartUrl: '', | ||
| }); |
Contributor
There was a problem hiding this comment.
Use boolean values for isTransparent.
isTransparent must be a boolean for TradingView widgets. Passing the string 'true' will serialize as "isTransparent":"true" in the JSON payload, which the embed script won’t interpret as the intended truthy flag. Update all affected configs to send a real boolean.
Apply this diff:
export const TECHNICAL_ANALYSIS_WIDGET_CONFIG = (symbol: string) => ({
symbol: symbol.toUpperCase(),
colorTheme: 'dark',
- isTransparent: 'true',
+ isTransparent: true,
@@
export const COMPANY_PROFILE_WIDGET_CONFIG = (symbol: string) => ({
symbol: symbol.toUpperCase(),
colorTheme: 'dark',
- isTransparent: 'true',
+ isTransparent: true,
@@
export const COMPANY_FINANCIALS_WIDGET_CONFIG = (symbol: string) => ({
symbol: symbol.toUpperCase(),
colorTheme: 'dark',
- isTransparent: 'true',
+ isTransparent: true,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| symbol: symbol.toUpperCase(), | |
| colorTheme: 'dark', | |
| isTransparent: 'true', | |
| locale: 'en', | |
| width: '100%', | |
| height: 400, | |
| interval: '1h', | |
| largeChartUrl: '', | |
| }); | |
| export const COMPANY_PROFILE_WIDGET_CONFIG = (symbol: string) => ({ | |
| symbol: symbol.toUpperCase(), | |
| colorTheme: 'dark', | |
| isTransparent: 'true', | |
| locale: 'en', | |
| width: '100%', | |
| height: 440, | |
| }); | |
| export const COMPANY_FINANCIALS_WIDGET_CONFIG = (symbol: string) => ({ | |
| symbol: symbol.toUpperCase(), | |
| colorTheme: 'dark', | |
| isTransparent: 'true', | |
| locale: 'en', | |
| width: '100%', | |
| height: 464, | |
| displayMode: 'regular', | |
| largeChartUrl: '', | |
| }); | |
| export const TECHNICAL_ANALYSIS_WIDGET_CONFIG = (symbol: string) => ({ | |
| symbol: symbol.toUpperCase(), | |
| colorTheme: 'dark', | |
| isTransparent: true, | |
| locale: 'en', | |
| width: '100%', | |
| height: 400, | |
| interval: '1h', | |
| largeChartUrl: '', | |
| }); | |
| export const COMPANY_PROFILE_WIDGET_CONFIG = (symbol: string) => ({ | |
| symbol: symbol.toUpperCase(), | |
| colorTheme: 'dark', | |
| isTransparent: true, | |
| locale: 'en', | |
| width: '100%', | |
| height: 440, | |
| }); | |
| export const COMPANY_FINANCIALS_WIDGET_CONFIG = (symbol: string) => ({ | |
| symbol: symbol.toUpperCase(), | |
| colorTheme: 'dark', | |
| isTransparent: true, | |
| locale: 'en', | |
| width: '100%', | |
| height: 464, | |
| displayMode: 'regular', | |
| largeChartUrl: '', | |
| }); |
🤖 Prompt for AI Agents
In lib/constants.ts around lines 235 to 263, the TradingView widget configs set
isTransparent as the string 'true', which must be a boolean; update each config
(the chart, COMPANY_PROFILE_WIDGET_CONFIG, and COMPANY_FINANCIALS_WIDGET_CONFIG)
to use the boolean value true (no quotes) instead of the string, preserving all
other properties and return shapes.
This was referenced Oct 2, 2025
Merged
This was referenced Jan 30, 2026
afanty2021
added a commit
to afanty2021/OpenStock
that referenced
this pull request
Feb 24, 2026
修复 Critical Open-Dev-Society#1, Important Open-Dev-Society#4 和 建议 Open-Dev-Society#6 代码质量问题： 1. 修复价格轮询 useEffect 依赖项问题 (Critical Open-Dev-Society#1) - 使用 useRef 存储 stocks 引用，避免依赖 stocks 导致 interval 重新创建 - 改为依赖 stocks.length 而非整个 stocks 对象 - 防止内存泄漏和性能问题 2. 改进类型安全性 (Important Open-Dev-Society#4) - 将 data 属性从 any[] 改为 WatchlistStockInput[] - 导出 WatchlistStock 和 WatchlistStockInput 类型供其他模块使用 - 使用 ?? 替代 || 进行空值合并，更符合 TypeScript 最佳实践 3. 提取魔法数字为常量 (建议 Open-Dev-Society#6) - PRICE_POLLING_INTERVAL: 价格轮询间隔 (5000ms) - TURNOVER_RATE_HIGH_THRESHOLD: 换手率高阈值 (10%) - TURNOVER_RATE_MEDIUM_THRESHOLD: 换手率中阈值 (5%) - VOLUME_UNIT_WAN: 成交量单位-万 (10000) - VOLUME_UNIT_YI: 成交量单位-亿 (100000000) 4. 其他改进 - 使用 useCallback 优化辅助函数性能 - 添加更详细的类型注释 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
afanty2021
added a commit
to afanty2021/OpenStock
that referenced
this pull request
Feb 24, 2026
修复 Task 5 中的 5 个代码质量问题： **Critical Open-Dev-Society#1**: 修复 DynamicVirtualTable ResizeObserver 内存泄漏 - 在组件级别维护 ResizeObserver 实例的 Map - 在 useEffect 中正确创建和清理观察者 - 每次创建新观察者前断开旧的连接 **Important Open-Dev-Society#2**: 实现简单的缓存管理器 CacheManager - 添加基于内存的缓存实现类 - 支持 TTL 和 stale-while-revalidate 策略 - 提供完整的 CRUD 操作和统计方法 **Important Open-Dev-Society#3**: 修复 useDebounce 导出冲突 - 移除向后兼容的 useDebounceCallback 导出 - 避免与 useDebounce 函数重名 **Important Open-Dev-Society#4**: 增强 Web Worker 错误处理 - 返回错误堆栈信息 (stack) - 添加安全的 payload 快照用于调试 - 保留原始错误消息 **Important Open-Dev-Society#5**: 修复 useThrottle 首次调用延迟 - 添加 isFirstCallRef 标志位 - 确保首次调用立即执行 测试结果: - useThrottle: 7/7 tests passed - cache strategies: 18/18 tests passed - worker calculations: 9/9 tests passed - 总计: 766/770 tests passed Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit