ENG-772 Implement Discourse Graph Tool and UI Enhancements #365
Conversation
- Introduced DiscourseGraphPanel component to manage node and relation tools. - Added onEnter cursor behavior for DiscourseGraphTool and relation tools. - Updated Tldraw component to include the new DiscourseGraphTool. - Enhanced UI with new styles for the Discourse Graph tool button. - Refactored UI components to integrate the DiscourseGraphPanel for better tool management.
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
@CodeRabbit Full Review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughIntroduces a new “discourse-tool” StateNode and integrates it into Tldraw and the toolbar; adds a DiscourseGraphPanel for selecting nodes/relations via a SharePanel; updates tool entry behavior to set crosshair cursors; and adds styling for the new tool’s icon. No exported signatures changed except new panel props/component. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant TB as Toolbar
participant UI as SharePanel / DiscourseGraphPanel
participant ED as Editor (tldraw)
participant DT as DiscourseGraphTool
participant NT as Node/Relation Tools
U->>TB: Click "Discourse Graph" tool
TB->>ED: setCurrentTool("discourse-tool")
ED->>DT: enter()
note right of DT: Tool state = idle
U->>UI: Open panel / select Node or Relation
UI->>ED: setCurrentTool("<node|relation>")
ED->>NT: enter()
NT->>ED: setCursor({ type: "cross"[ , rotation: 45 ]})
note over NT,ED: Cursor switches to crosshair on enter
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 (
|
![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
🧹 Nitpick comments (14)
apps/roam/src/components/canvas/Tldraw.tsx (1)
321-325: Add basic onEnter cleanup for DiscourseGraphToolAll occurrences of
"discourse-tool"across the codebase refer to the same tool ID (no drift detected in tldrawStyles.ts, Tldraw.tsx, uiOverrides.tsx, DiscourseToolPanel.tsx).Apply this diff in apps/roam/src/components/canvas/Tldraw.tsx:
const discourseGraphTool = class DiscourseGraphTool extends StateNode { static override id = "discourse-tool"; static override initial = "idle"; + override onEnter = () => { + // Neutral cursor; this is a palette tool, not a drawing tool. + this.editor.setCursor({ type: "default" }); + this.editor.setSelectedShapes([]); + this.editor.setEditingShape(null); + }; };apps/roam/src/components/canvas/DiscourseNodeUtil.tsx (1)
106-112: Reset cursor on exit to avoid crosshair persisting after switching toolsNice touch setting a crosshair with 45° rotation on enter. Add an onExit to return to default when leaving the tool, so the cursor doesn’t persist when switching back to select/discourse-tool.
Add alongside onEnter (outside the shown range):
override onExit = () => { this.editor.setCursor({ type: "default" }); };apps/roam/src/components/canvas/tldrawStyles.ts (1)
69-78: Make the icon theme-aware and better scoped (use mask + currentColor in the toolbar)Using a background-image hardcodes black and may clash with themes. Prefer mask-image with currentColor so the icon picks up the button color, and scope to the TL toolbar to avoid accidental collisions.
Apply this diff:
-/* Discourse Graph ToolButton */ - button[data-value="discourse-tool"] div::before { - content: ""; - display: inline-block; - width: 18px; - height: 18px; - background-image: url("data:image/svg+xml,%3Csvg width='256' height='264' viewBox='0 0 256 264' fill='none' xmlns='http://www.w3.org/2000/svg'%3E%3Cpath fill-rule='evenodd' clip-rule='evenodd' d='M156.705 252.012C140.72 267.995 114.803 267.995 98.8183 252.012L11.9887 165.182C-3.99622 149.197 -3.99622 123.28 11.9886 107.296L55.4035 63.8807C63.3959 55.8881 76.3541 55.8881 84.3467 63.8807C92.3391 71.8731 92.3391 84.8313 84.3467 92.8239L69.8751 107.296C53.8901 123.28 53.8901 149.197 69.8751 165.182L113.29 208.596C121.282 216.589 134.241 216.589 142.233 208.596C150.225 200.604 150.225 187.646 142.233 179.653L127.761 165.182C111.777 149.197 111.777 123.28 127.761 107.296C143.746 91.3105 143.746 65.3939 127.761 49.4091L113.29 34.9375C105.297 26.9452 105.297 13.9868 113.29 5.99432C121.282 -1.99811 134.241 -1.99811 142.233 5.99434L243.533 107.296C259.519 123.28 259.519 149.197 243.533 165.182L156.705 252.012ZM200.119 121.767C192.127 113.775 179.168 113.775 171.176 121.767C163.184 129.76 163.184 142.718 171.176 150.71C179.168 158.703 192.127 158.703 200.119 150.71C208.112 142.718 208.112 129.76 200.119 121.767Z' fill='%23000000'/%3E%3C/svg%3E"); - background-size: contain; - background-repeat: no-repeat; -} +/* Discourse Graph ToolButton (scoped to TL toolbar) */ +.tlui-toolbar button[data-value="discourse-tool"] .tlui-button__icon::before { + content: ""; + display: inline-block; + width: 18px; + height: 18px; + /* theme-aware monochrome via mask + currentColor */ + -webkit-mask-image: url("data:image/svg+xml,%3Csvg width='256' height='264' viewBox='0 0 256 264' fill='none' xmlns='http://www.w3.org/2000/svg'%3E%3Cpath fill-rule='evenodd' clip-rule='evenodd' d='M156.705 252.012C140.72 267.995 114.803 267.995 98.8183 252.012L11.9887 165.182C-3.99622 149.197 -3.99622 123.28 11.9886 107.296L55.4035 63.8807C63.3959 55.8881 76.3541 55.8881 84.3467 63.8807C92.3391 71.8731 92.3391 84.8313 84.3467 92.8239L69.8751 107.296C53.8901 123.28 53.8901 149.197 69.8751 165.182L113.29 208.596C121.282 216.589 134.241 216.589 142.233 208.596C150.225 200.604 150.225 187.646 142.233 179.653L127.761 165.182C111.777 149.197 111.777 123.28 127.761 107.296C143.746 91.3105 143.746 65.3939 127.761 49.4091L113.29 34.9375C105.297 26.9452 105.297 13.9868 113.29 5.99432C121.282 -1.99811 134.241 -1.99811 142.233 5.99434L243.533 107.296C259.519 123.28 259.519 149.197 243.533 165.182L156.705 252.012ZM200.119 121.767C192.127 113.775 179.168 113.775 171.176 121.767C163.184 129.76 163.184 142.718 171.176 150.71C179.168 158.703 192.127 158.703 200.119 150.71C208.112 142.718 208.112 129.76 200.119 121.767Z' fill='%23000'/%3E%3C/svg%3E"); + mask-image: url("data:image/svg+xml,%3Csvg width='256' height='264' viewBox='0 0 256 264' fill='none' xmlns='http://www.w3.org/2000/svg'%3E%3Cpath fill-rule='evenodd' clip-rule='evenodd' d='M156.705 252.012C140.72 267.995 114.803 267.995 98.8183 252.012L11.9887 165.182C-3.99622 149.197 -3.99622 123.28 11.9886 107.296L55.4035 63.8807C63.3959 55.8881 76.3541 55.8881 84.3467 63.8807C92.3391 71.8731 92.3391 84.8313 84.3467 92.8239L69.8751 107.296C53.8901 123.28 53.8901 149.197 69.8751 165.182L113.29 208.596C121.282 216.589 134.241 216.589 142.233 208.596C150.225 200.604 150.225 187.646 142.233 179.653L127.761 165.182C111.777 149.197 111.777 123.28 127.761 107.296C143.746 91.3105 143.746 65.3939 127.761 49.4091L113.29 34.9375C105.297 26.9452 105.297 13.9868 113.29 5.99432C121.282 -1.99811 134.241 -1.99811 142.233 5.99434L243.533 107.296C259.519 123.28 259.519 149.197 243.533 165.182L156.705 252.012ZM200.119 121.767C192.127 113.775 179.168 113.775 171.176 121.767C163.184 129.76 163.184 142.718 171.176 150.71C179.168 158.703 192.127 158.703 200.119 150.71C208.112 142.718 208.112 129.76 200.119 121.767Z' fill='%23000'/%3E%3C/svg%3E"); + -webkit-mask-repeat: no-repeat; + mask-repeat: no-repeat; + -webkit-mask-size: contain; + mask-size: contain; + background-color: currentColor; +}apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationTool.tsx (1)
36-41: Reset cursor on exit for ReferencedNodeToolGood to set crosshair on enter. Add an onExit to return to default when leaving the tool to avoid the cursor persisting after switching.
Add alongside onEnter (outside the shown range):
override onExit = () => { this.editor.setCursor({ type: "default" }); };apps/roam/src/components/canvas/DiscourseToolPanel.tsx (4)
3-3: Remove unused importDiscourseRelation is imported but not used. This will trip the linter.
-import { DiscourseRelation } from "~/utils/getDiscourseRelations";
27-28: Memoize uniqueRelations to avoid recomputing on each renderTiny optimization; prevents recreating a new Set/string array on any unrelated state change.
- const uniqueRelations = [...new Set(relations)]; + const uniqueRelations = React.useMemo( + () => [...new Set(relations)], + [relations], + );If relations are not passed in a stable order, colors derived from getRelationColor(relation, index) may shift across renders. Confirm the prop maintains a stable order. If not, consider a deterministic sort or a persistent mapping keyed by relation id.
40-56: Use semantic buttons for clickable rows (accessibility + keyboard support)These rows are interactive but use divs. Switch to button with type="button", role, and keyboard semantics for a11y (and to match TL UI styles).
Example diff for the first occurrence; apply the same pattern to the other three ranges:
- <div + <button className="tlui-style-panel__row tlui-button tlui-button__icon flex h-5 cursor-pointer items-center gap-2 px-3" style={{ justifyContent: "flex-start", }} onClick={() => editor.setCurrentTool("discourse-tool")} - > + type="button" + aria-label="Back to Discourse Graph tools" + > <span className="tlui-icon tlui-button__icon mr-2" style={{ mask: `url("https://cdn.tldraw.com/2.3.0/icons/icon/color.svg") center 100% / 100% no-repeat`, backgroundColor: formatHexColor(currentNodeTool.canvasSettings.color) || "black", }} /> <span>{currentNodeTool.text}</span> - </div> + </button>Optionally, consider extracting a small component to remove duplication.
Also applies to: 72-86, 98-116, 126-142
78-84: Prefer theme-driven coloring for icons (mask + currentColor)For consistency with TL UI, prefer driving icon color via the color (currentColor) rather than mixing backgroundColor + mask. Set className to include tlui-button__icon and pass style={{ color }}; rely on TL’s CSS to color the masked icon.
Example for relations:
- <div + <div className="tlui-icon tlui-button__icon mr-2" style={{ - color, - mask: `url("https://cdn.tldraw.com/2.3.0/icons/icon/tool-arrow.svg") center 100% / 100% no-repeat`, + color, // TL CSS uses currentColor with masked icons + mask: `url("https://cdn.tldraw.com/2.3.0/icons/icon/tool-arrow.svg") center 100% / 100% no-repeat`, }} ></div>For node color swatches, consider the same approach or keep backgroundColor if you intend the glyph to be filled rather than stroked.
Also applies to: 106-113, 134-139
apps/roam/src/components/canvas/uiOverrides.tsx (6)
228-233: Toolbar wiring for discourse-tool looks good; tiny readability nitHook usage is correct and outside loops. For readability, consider extracting the selection hook to a named const.
- <TldrawUiMenuItem - key="discourse-tool" - {...tools["discourse-tool"]} - isSelected={useIsToolSelected(tools["discourse-tool"])} - /> + <TldrawUiMenuItem + key="discourse-tool" + {...tools["discourse-tool"]} + isSelected={isDiscourseToolSelected} + />And just above the return in this component:
const isDiscourseToolSelected = useIsToolSelected(tools["discourse-tool"]);
234-257: Remove or feature-flag the large commented toolbar blockLeaving a large commented section adds noise and risks drift. Either remove it or guard the old toolbar with a feature flag for easy rollback.
- {/* {allNodes.map((n) => ( - <TldrawUiMenuItem - key={n.type} - {...tools[n.type]} - // eslint-disable-next-line react-hooks/rules-of-hooks - isSelected={useIsToolSelected(tools[n.type])} - /> - ))} - {allRelationNames.map((name) => ( - <TldrawUiMenuItem - key={name} - {...tools[name]} - // eslint-disable-next-line react-hooks/rules-of-hooks - isSelected={useIsToolSelected(tools[name])} - /> - ))} - {allAddReferencedNodeActions.map((action) => ( - <TldrawUiMenuItem - key={action} - {...tools[action]} - // eslint-disable-next-line react-hooks/rules-of-hooks - isSelected={useIsToolSelected(tools[action])} - /> - ))} */}
302-308: De-duplicate relations before passing to DiscourseGraphPanelIf any names overlap between relation tools and add-referenced actions, the panel could show duplicates. De-duplicate when composing the list.
- SharePanel: () => { - const allRelations = [ - ...allRelationNames, - ...allAddReferencedNodeActions, - ]; - return <DiscourseGraphPanel nodes={allNodes} relations={allRelations} />; - }, + SharePanel: () => { + const allRelations = Array.from( + new Set([...allRelationNames, ...allAddReferencedNodeActions]) + ); + return <DiscourseGraphPanel nodes={allNodes} relations={allRelations} />; + },
459-459: i18n key added — ensure parity across locales if applicableEnglish entry looks right. If you maintain other locales, mirror this key there to avoid fallback English strings in non-en locales.
47-47: Default export and props verified; consider renaming fileThe import of
DiscourseGraphPanelfrom./DiscourseToolPanelis correct—DiscourseToolPanel.tsxdeclaresexport type DiscourseGraphPanelProps = { nodes: DiscourseNode[]; relations: string[]; }; … export default DiscourseGraphPanel;and the default export matches the imported component.
• Optional refactor: rename
– apps/roam/src/components/canvas/DiscourseToolPanel.tsx → DiscourseGraphPanel.tsx
for clearer discoverability and consistency with the component name.
329-338: Add a “D” shortcut for the discourse tool and include it in the shortcuts dialogBased on the existing keybindings scan, no tool is currently using “D” as its
kbdvalue, so it’s safe to assign it here.• In
apps/roam/src/components/canvas/uiOverrides.tsx(around line 333), update thediscourse-toolregistration:tools["discourse-tool"] = { id: "discourse-tool", icon: "none", label: "tool.discourse-tool" as TLUiTranslationKey, - kbd: "", + kbd: "D", readonlyOk: true, onSelect: () => { editor.setCurrentTool("discourse-tool"); }, };• In the same file, within the
<KeyboardShortcutsDialog>JSX (near line 266), add the new menu item so it appears in the shortcuts list:const tools = useTools(); … <TldrawUiMenuItem key="discourse-tool" {...tools["discourse-tool"]} />These optional UX enhancements will give users a quick “D” hotkey to switch to the custom discourse tool and surface it in the built-in keyboard shortcuts dialog for discoverability.
📜 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 (6)
apps/roam/src/components/canvas/DiscourseNodeUtil.tsx(1 hunks)apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationTool.tsx(3 hunks)apps/roam/src/components/canvas/DiscourseToolPanel.tsx(1 hunks)apps/roam/src/components/canvas/Tldraw.tsx(3 hunks)apps/roam/src/components/canvas/tldrawStyles.ts(1 hunks)apps/roam/src/components/canvas/uiOverrides.tsx(6 hunks)
🔇 Additional comments (3)
apps/roam/src/components/canvas/Tldraw.tsx (2)
43-44: LGTM: importing StateNode to enable a custom tool is correctImport looks right for defining a no-op palette tool (DiscourseGraphTool).
332-336: LGTM: tool registration order is fineRegistering DiscourseGraphTool ahead of shape tools is consistent with showing it as the primary toolbar entry.
apps/roam/src/components/canvas/DiscourseRelationShape/DiscourseRelationTool.tsx (1)
276-278: LGTM: Idle state cursor for referenced-node toolsIdle showing a crosshair matches drawing semantics and is consistent with node tools.
https://www.loom.com/share/c9170d0e2f9c4685aa719ee475711ebe
Summary by CodeRabbit
New Features
Style
Chores