-
Notifications
You must be signed in to change notification settings - Fork 80
chore: Verifies if localization is enabled in settings before showing LocalizationButton #3181
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
chore: Verifies if localization is enabled in settings before showing LocalizationButton #3181
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThe changes introduce consistent feature flag-based gating for localization functionality by creating a computed Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/core/src/sdk/localization/useLocalizationConfig.tsx`:
- Around line 15-17: The early return in useLocalizationConfig causes a Rules of
Hooks violation by conditionally skipping hooks; remove the top-level return and
instead evaluate config.localization?.enabled inside the useState initializer
(the call to useState at line where state is created) so hooks (useState and
useEffect) execute unconditionally, then after hooks run return undefined or the
Settings value as before; update any conditional logic in the useEffect (the
effect that depends on localization) to no-op when enabled is false. Ensure
references to useState and useEffect in useLocalizationConfig remain in the same
order and only the initializer and effect body check
config.localization?.enabled.
What's the purpose of this pull request?
Don't display
LocalizationButtonif localization flag is not enabled in discovery.config.How to test it?
localization.enabledtofalseSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.