Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughThis PR refactors feature flag management by replacing config-tree-based settings with feature-flag-driven controls. The "Suggestive mode enabled" setting is renamed to "Suggestive mode overlay enabled" in the feature flags schema, while being removed from personal settings. A new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/roam/src/components/settings/AdminPanel.tsx`:
- Around line 283-291: handleFeatureToggled currently only opens the
reload/instruction alert when enabling (checked === true); change it to open the
alert for both enable and disable by calling setIsInstructionAlertOpen(true)
after calling setter(checked) (i.e., remove the checked-only condition), and
update the instruction alert text/state (used where setIsInstructionAlertOpen is
read) to a generic message that applies to both enabling and disabling so users
are prompted to reload when the observer change requires it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d0aa385e-cb8f-43b7-9631-0872c82917f7
📒 Files selected for processing (10)
apps/roam/src/components/settings/AdminPanel.tsxapps/roam/src/components/settings/HomePersonalSettings.tsxapps/roam/src/components/settings/NodeConfig.tsxapps/roam/src/components/settings/components/BlockPropSettingPanels.tsxapps/roam/src/components/settings/utils/accessors.tsapps/roam/src/components/settings/utils/zodSchema.example.tsapps/roam/src/components/settings/utils/zodSchema.tsapps/roam/src/index.tsapps/roam/src/utils/discourseConfigRef.tsapps/roam/src/utils/initializeObserversAndListeners.ts
💤 Files with no reviewable changes (1)
- apps/roam/src/utils/discourseConfigRef.ts
| const handleFeatureToggled = ( | ||
| checked: boolean, | ||
| setter: (v: boolean) => void, | ||
| ) => { | ||
| setter(checked); | ||
| if (checked) { | ||
| setIsInstructionAlertOpen(true); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Consider prompting for reload when disabling features too.
The reload prompt is only shown when checked is true (enabling). However, based on the initialization logic in initializeObserversAndListeners.ts, disabling a feature flag also requires a reload to take effect—the observer registered at startup will remain active until the page reloads.
Users might disable the feature expecting an immediate effect, but the observer will persist.
💡 Suggested fix
const handleFeatureToggled = (
checked: boolean,
setter: (v: boolean) => void,
) => {
setter(checked);
- if (checked) {
- setIsInstructionAlertOpen(true);
- }
+ setIsInstructionAlertOpen(true);
};Then update the instruction alert message to be more generic (applicable for both enable/disable):
-<p>Please reload the graph for this change to take effect.</p>
+<p>Please reload the graph for this setting change to take effect.</p>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/roam/src/components/settings/AdminPanel.tsx` around lines 283 - 291,
handleFeatureToggled currently only opens the reload/instruction alert when
enabling (checked === true); change it to open the alert for both enable and
disable by calling setIsInstructionAlertOpen(true) after calling setter(checked)
(i.e., remove the checked-only condition), and update the instruction alert
text/state (used where setIsInstructionAlertOpen is read) to a generic message
that applies to both enabling and disabling so users are prompted to reload when
the observer change requires it.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
1 participant
Summary by CodeRabbit
Release Notes