-
Notifications
You must be signed in to change notification settings - Fork 3
ENG-636 Add auto canvas relations setting to user preferences #363
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
ENG-636 Add auto canvas relations setting to user preferences #363
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughAdds a user-configurable setting (AUTO_CANVAS_RELATIONS_KEY) to control automatic creation of canvas discourse relations. Integrates the setting in canvas node creation/edit flows to conditionally call createExistingRelations. Exposes a new toggle in personal settings and defines the setting key in userSettings. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Settings as Settings UI
participant Ext as extensionAPI.settings
participant Canvas as Canvas (Tldraw/DiscourseNodeUtil)
participant Util as getSetting
participant Rel as createExistingRelations
User->>Settings: Toggle "Auto Canvas Relations"
Settings->>Ext: set(AUTO_CANVAS_RELATIONS_KEY, true/false)
rect rgb(240,245,255)
note right of Canvas: On shape create or label edit
Canvas->>Util: getSetting(AUTO_CANVAS_RELATIONS_KEY, false)
alt Auto enabled
Util-->>Canvas: true
Canvas->>Rel: createExistingRelations(...)
Rel-->>Canvas: done
else Auto disabled
Util-->>Canvas: false
note right of Canvas: Skip relation creation
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@CodeRabbit full review |
✅ Actions performedFull review 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: 2
🧹 Nitpick comments (2)
apps/roam/src/data/userSettings.ts (1)
6-6: Add brief JSDoc and confirm intended default/back-compat behaviorNice addition. Two quick items:
- Add a short comment to describe the key’s purpose.
- Today’s call sites use
getSetting(AUTO_CANVAS_RELATIONS_KEY, false), which flips a previously-always-on behavior to OFF by default. If that’s unintended, switch those defaults totruefor backward compatibility.Apply this doc tweak:
+// Controls whether discourse relations are auto-created on the canvas when nodes are added/edited. export const AUTO_CANVAS_RELATIONS_KEY = "auto-canvas-relations";If we want to preserve the prior default behavior (auto-creation ON), I’ve proposed diffs in the other files to default to
true.apps/roam/src/components/settings/HomePersonalSettings.tsx (1)
147-168: Clarify label/scope, align default for back-compat, and keep style consistent
- Label/description: The toggle also affects relation creation on edit flows, not just “when a node is added.” Clarify the copy.
- Default: If we intend to preserve the previous behavior (auto-create ON by default), prefer
!== falseas used elsewhere for “on by default” switches.- Style: Other checkboxes don’t use
voidbeforeextensionAPI.settings.set; drop it for consistency.<Checkbox - defaultChecked={ - extensionAPI.settings.get(AUTO_CANVAS_RELATIONS_KEY) === true - } + defaultChecked={ + extensionAPI.settings.get(AUTO_CANVAS_RELATIONS_KEY) !== false + } onChange={(e) => { const target = e.target as HTMLInputElement; - void extensionAPI.settings.set( + extensionAPI.settings.set( AUTO_CANVAS_RELATIONS_KEY, target.checked, ); }} labelElement={ <> - Auto Canvas Relations + Auto-create Canvas Relations <Description description={ - "Automatically add discourse relations to canvas when a node is added" + "Automatically create discourse relations on the canvas when a node is added or edited." } /> </> } />If the new behavior should be OFF by default, keep the current
=== truecheck, but please still update the label/description.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
apps/roam/src/components/canvas/DiscourseNodeUtil.tsx(2 hunks)apps/roam/src/components/canvas/Tldraw.tsx(2 hunks)apps/roam/src/components/settings/HomePersonalSettings.tsx(2 hunks)apps/roam/src/data/userSettings.ts(1 hunks)
🔇 Additional comments (3)
apps/roam/src/components/settings/HomePersonalSettings.tsx (1)
16-16: Import looks goodImporting the setting key from a single source of truth is the right call.
apps/roam/src/components/canvas/Tldraw.tsx (1)
85-87: LGTMCentralizing the key and using
getSettinghere keeps the canvas logic decoupled from the settings UI.apps/roam/src/components/canvas/DiscourseNodeUtil.tsx (1)
41-43: LGTMImporting the key and
getSettinghere aligns the node util with the canvas handler.
…elations are deleted before creating existing relations when auto canvas relations are enabled.
https://www.loom.com/share/954b7fce269d41aea8d6e1600236de23
Summary by CodeRabbit