ENG-994 - move canvas drawer into tldraw component #568
Conversation
…mproved UI. Remove CanvasDrawerButton and update related imports. Simplify state management for grouped shapes in Tldraw component.
…tion. Move grouped shapes logic from Tldraw to CanvasDrawerPanel, simplifying component structure and improving maintainability.
…tate management. Remove unnecessary props and simplify integration in Tldraw component.
…ent functionality. Refactor moveCameraToShape logic and update CanvasDrawerPanel to pass editor instance, streamlining component interactions.
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughThe Canvas Drawer component architecture is refactored from a provider/custom-event-based pattern to a self-contained CanvasDrawerPanel component. The CanvasDrawerButton component is removed, command palette registration and context menu options for opening the drawer are eliminated, and integration points are updated to use the new panel component directly. Changes
Sequence DiagramsequenceDiagram
actor User
participant UI as Tldraw UI
participant Panel as CanvasDrawerPanel
participant Content as CanvasDrawerContent
participant Editor as Editor API
rect rgb(220, 240, 250)
Note over Panel,Content: New Self-Contained Architecture
User->>UI: Interact with Tldraw
UI->>Panel: Render CanvasDrawerPanel
Panel->>Panel: Manage isOpen state
Panel->>Editor: Get editor instance (useEditor)
Panel->>Content: Pass editor + groupedShapes
end
rect rgb(245, 230, 255)
Note over Content,Editor: Camera Movement (via Editor API)
User->>Content: Click shape in drawer
Content->>Editor: moveCameraToShape(editor, shapeId)
Editor->>Editor: Center on shape & select
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Pre-merge checks✅ Passed checks (3 passed)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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
🧹 Nitpick comments (1)
apps/roam/src/components/canvas/CanvasDrawer.tsx (1)
436-441: Consider extracting duplicate boxShadow to a constant.The same complex
boxShadowvalue appears twice (lines 439-441 and 458-459). Extracting it to a constant would reduce duplication and make maintenance easier.const TLDRAW_SHADOW = "0px 0px 2px hsl(0, 0%, 0%, 16%), 0px 2px 3px hsl(0, 0%, 0%, 24%), 0px 2px 6px hsl(0, 0%, 0%, 0.1), inset 0px 0px 0px 1px hsl(0, 0%, 100%)"; // Then use: style={{ zIndex: 250, boxShadow: TLDRAW_SHADOW, ... }}Also applies to: 454-460
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/roam/src/components/canvas/CanvasDrawer.tsx(6 hunks)apps/roam/src/components/canvas/CanvasDrawerButton.tsx(0 hunks)apps/roam/src/components/canvas/Tldraw.tsx(4 hunks)apps/roam/src/components/canvas/uiOverrides.tsx(0 hunks)apps/roam/src/utils/registerCommandPaletteCommands.ts(0 hunks)
💤 Files with no reviewable changes (3)
- apps/roam/src/components/canvas/CanvasDrawerButton.tsx
- apps/roam/src/utils/registerCommandPaletteCommands.ts
- apps/roam/src/components/canvas/uiOverrides.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/main.mdc)
**/*.{ts,tsx}: Use Tailwind CSS for styling where possible
When refactoring inline styles, use tailwind classes
Prefertypeoverinterfacein TypeScript
Use explicit return types for functions
Avoidanytypes when possible
Prefer arrow functions over regular function declarations
Use named parameters (object destructuring) when a function has more than 2 parameters
Use PascalCase for components and types
Use camelCase for variables and functions
Use UPPERCASE for constants
Function names should describe their purpose clearly
Prefer early returns over nested conditionals for better readability
Files:
apps/roam/src/components/canvas/Tldraw.tsxapps/roam/src/components/canvas/CanvasDrawer.tsx
apps/roam/**/*.{js,ts,tsx,jsx,json}
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
Prefer existing dependencies from package.json when working on the Roam Research extension
Files:
apps/roam/src/components/canvas/Tldraw.tsxapps/roam/src/components/canvas/CanvasDrawer.tsx
apps/roam/**/*.{ts,tsx,jsx,js,css,scss}
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
Use BlueprintJS 3 components and Tailwind CSS for platform-native UI in the Roam Research extension
Files:
apps/roam/src/components/canvas/Tldraw.tsxapps/roam/src/components/canvas/CanvasDrawer.tsx
apps/roam/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
apps/roam/**/*.{ts,tsx,js,jsx}: Use the roamAlphaApi docs from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj when implementing Roam functionality
Use Roam Depot/Extension API docs from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU when implementing extension functionality
Files:
apps/roam/src/components/canvas/Tldraw.tsxapps/roam/src/components/canvas/CanvasDrawer.tsx
apps/roam/**
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
Implement the Discourse Graph protocol in the Roam Research extension
Files:
apps/roam/src/components/canvas/Tldraw.tsxapps/roam/src/components/canvas/CanvasDrawer.tsx
🧠 Learnings (3)
📚 Learning: 2025-11-25T00:52:41.909Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.909Z
Learning: Applies to apps/roam/**/*.{ts,tsx,js,jsx} : Use the roamAlphaApi docs from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj when implementing Roam functionality
Applied to files:
apps/roam/src/components/canvas/Tldraw.tsxapps/roam/src/components/canvas/CanvasDrawer.tsx
📚 Learning: 2025-11-25T00:52:41.909Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.909Z
Learning: Applies to apps/roam/**/*.{ts,tsx,jsx,js,css,scss} : Use BlueprintJS 3 components and Tailwind CSS for platform-native UI in the Roam Research extension
Applied to files:
apps/roam/src/components/canvas/Tldraw.tsxapps/roam/src/components/canvas/CanvasDrawer.tsx
📚 Learning: 2025-08-11T19:09:58.252Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 337
File: apps/roam/src/components/DiscourseFloatingMenu.tsx:43-43
Timestamp: 2025-08-11T19:09:58.252Z
Learning: The roam subdirectory (apps/roam) is constrained to React 17 and cannot use React 18+ features like createRoot API. ReactDOM.render should be used instead of createRoot in this subdirectory.
Applied to files:
apps/roam/src/components/canvas/CanvasDrawer.tsx
🧬 Code graph analysis (2)
apps/roam/src/components/canvas/Tldraw.tsx (1)
apps/roam/src/components/canvas/CanvasDrawer.tsx (1)
CanvasDrawerPanel(393-487)
apps/roam/src/components/canvas/CanvasDrawer.tsx (3)
apps/roam/src/components/LivePreview.tsx (1)
Props(86-93)packages/ui/src/components/ui/card.tsx (1)
Card(82-82)apps/roam/src/components/canvas/DiscourseNodeUtil.tsx (1)
DiscourseNodeShape(158-170)
🪛 Biome (2.1.2)
apps/roam/src/components/canvas/Tldraw.tsx
[error] 934-934: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
🔇 Additional comments (5)
apps/roam/src/components/canvas/CanvasDrawer.tsx (3)
36-40: LGTM!Props type definition correctly uses
typeoverinterfaceand includes the neweditorprop needed for direct camera control.
147-159: LGTM!Clean refactor of camera control logic using the Editor API directly instead of custom events. The implementation properly handles the shape lookup and provides smooth animation.
393-400: Critical: Hook ordering bug -setIsOpenis referenced before it's defined.
toggleDrawercallback (lines 395-397) referencessetIsOpen, butuseStateis declared on line 398. WhenuseCallbackexecutes,setIsOpenisundefinedin the closure, causing the toggle button to fail.Apply this diff to fix the hook ordering:
export const CanvasDrawerPanel = () => { const editor = useEditor(); + const [isOpen, setIsOpen] = useState(false); + const pageUid = getCurrentPageUid(); + const [groupedShapes, setGroupedShapes] = useState<GroupedShapes>({}); + const toggleDrawer = useCallback(() => { setIsOpen((prev) => !prev); }, []); - const [isOpen, setIsOpen] = useState(false); - const pageUid = getCurrentPageUid(); - const [groupedShapes, setGroupedShapes] = useState<GroupedShapes>({});Likely an incorrect or invalid review comment.
apps/roam/src/components/canvas/Tldraw.tsx (2)
87-87: LGTM!Import correctly updated to use the new
CanvasDrawerPanelcomponent.
685-685: LGTM!Correct placement inside
TldrawUiensures the panel has access to the editor context viauseEditor().
…divs for improved layout consistency. Adjust styles and structure in CanvasDrawerPanel for better responsiveness and visual clarity.
jsmorabito
left a comment
jsmorabito
left a comment
There was a problem hiding this comment.
Fantastic! Let's merge!!
mdroidian
deleted the
eng-994-move-canvas-drawer-into-tldraw-component
branch
URL: https://discoursegraphs.com/releases/roam/eng-994-move-canvas-drawer-into-tldraw-component
https://www.loom.com/share/75f329a5c39943ce81d45124880624ea
Summary by CodeRabbit
Refactor
Bug Fixes
Removals
✏️ Tip: You can customize this high-level summary in your review settings.