ENG-797: Handle multiple canvases loaded at the same time#928
ENG-797: Handle multiple canvases loaded at the same time#928
Conversation
Scope active canvas focus so clipboard, query builder actions, and shared context don't leak across simultaneously open canvases.
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
@coderabbitai full review |
| export const discourseContext: DiscourseContextType = { | ||
| nodes: {}, | ||
| relations: {}, | ||
| lastAppEvent: "", |
| let activeCanvasPageUid: string | null = null; | ||
| let activeCanvasEditor: Editor | null = null; | ||
|
|
||
| const setActiveCanvas = ({ |
There was a problem hiding this comment.
Surface 3 handled, we track the active canvas and ensure only that canvas is focused, so only this canvas responds to the clipboard events
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
📝 WalkthroughWalkthroughThis PR refactors discourse context state management by removing Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
✅ Actions performedFull review triggered. |
| if (!isTLStore) { | ||
| const relations = getDiscourseRelations(); | ||
| discourseContext.relations = relations.reduce( | ||
| (acc, r) => { |
There was a problem hiding this comment.
Mutating this could effect the state for other open canvases??? we do set this up in tldraw.tsx line 455
This is for you Michael it seems correct to me but can give an authortative answer here
| <Button | ||
| {...buttonProps} | ||
| onClick={() => { | ||
| onClick={(event) => { |
There was a problem hiding this comment.
Pass this to the correct canvas
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Check activeCanvasEditor identity in cleanup to avoid clearing a canvas that was already replaced by another instance - Guard onPointerDownCapture against null editor ref - Narrow closest() selector to .roamjs-tldraw-canvas-container so nested data-page-uid attributes don't match incorrectly
- Introduced a new `docsRouteMap.ts` file to manage documentation sections and redirects for Roam and Obsidian platforms. - Updated `next.config.ts` to include dynamic redirects based on the new route map. - Refactored the layout components for documentation pages to utilize a consistent theme layout. - Removed outdated `page.tsx` files for Roam and Obsidian, replacing them with new landing pages that leverage Nextra for content rendering. - Added new metadata and content files for various documentation sections, enhancing the overall documentation structure and accessibility.
1 participant
Scope active canvas focus so clipboard, query builder actions, and shared context don't leak across simultaneously open canvases.
Summary by CodeRabbit