-
Notifications
You must be signed in to change notification settings - Fork 3.6k
fix: Tw chart theme not applied when language changes #12464
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: develop
Are you sure you want to change the base?
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
8 Skipped Deployments
|
fee2fae to
eba5b21
Compare
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.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on November 28
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| } | ||
| changeTheme() | ||
| }, [isDark, theme]) | ||
| }, [isDark, theme, lastUpdated]) |
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.
Bug: Theme Change Effect Triggered Incorrectly
The changeTheme effect dependency array includes lastUpdated (line 383), but refresh() is called during widget initialization (lines 335 and 345). This causes the theme change effect to execute whenever the widget becomes ready, not just when the theme actually changes. The lastUpdated dependency should be removed from the effect dependency array, as lastUpdated is not related to theme changes and its inclusion creates unintended effect triggers during widget initialization.
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.
lastUpdated needed to trigger to force to change the theme, when rerender
eba5b21 to
45edafd
Compare
PR-Codex overview
This PR focuses on enhancing the
TradingViewChartcomponent by introducing a new function for widget overrides based on the theme, improving the handling of theme changes, and debouncing updates to prevent excessive widget recreation.Detailed summary
getWidgetOverridesfunction to manage theme-based widget properties.optionswith a call togetWidgetOverrides.useLastUpdatedhook to manage refresh state.lastUpdated.Note
Centralizes TradingView widget overrides, adds lastUpdated-driven refresh, and ensures theme overrides are reapplied when the widget becomes ready or the theme changes.
apps/web/src/components/Chart/TradingViewChart.tsx)getWidgetOverrides(isDark)and uses it for both widget initialization (overrides) and runtime updates (applyOverrides).useLastUpdated; triggersrefresh()when the widget becomes ready and includeslastUpdatedin the theme effect deps to force reapplication after readiness changes.changeThemewithchangeTheme(...), applyOverrides(getWidgetOverrides(isDark)).Written by Cursor Bugbot for commit 45edafd. This will update automatically on new commits. Configure here.