-
Notifications
You must be signed in to change notification settings - Fork 4
ENG-747 - Enhance Tldraw component to track last actions in history #383
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-747 - Enhance Tldraw component to track last actions in history #383
Conversation
- Added lastActions state to DiscourseContextType for tracking recent changes. - Implemented a mechanism to push new actions to lastActions on app change events, maintaining a maximum of 10 entries. - Updated error handling to include lastActions context for better debugging.
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughAdds action history tracking to the Tldraw canvas by extending the discourse context with lastActions, initializing and persisting it via a ref, subscribing to Tldraw change events on mount to append capped history (10 items), and including lastActions in the error context. Public types and exports are updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant TldrawCanvas
participant TldrawApp as TLDraw App
participant History as lastActionsRef / discourseContext.lastActions
participant ErrorCtx as Error Context
User->>TldrawCanvas: Mount component
TldrawCanvas->>TLDraw App: Initialize & subscribe to change events
note right of TLDraw App: onChange(handler)
TLDraw App-->>TldrawCanvas: changeEvent(entry)
TldrawCanvas->>History: Append entry
TldrawCanvas->>History: Trim to last 10 items
rect rgba(230,245,255,0.5)
note over TldrawCanvas,ErrorCtx: Error reporting
TldrawCanvas->>ErrorCtx: Provide { lastActions, ... }
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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 (
|
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: 4
🧹 Nitpick comments (1)
apps/roam/src/components/canvas/Tldraw.tsx (1)
91-92: Use type-only imports to avoid bundling runtime code from tldraw packagesThese are types; importing them as values can bloat the bundle.
-import { HistoryEntry } from "@tldraw/store"; -import { TLRecord } from "@tldraw/tlschema"; +import type { HistoryEntry } from "@tldraw/store"; +import type { TLRecord } from "@tldraw/tlschema";
📜 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 (1)
apps/roam/src/components/canvas/Tldraw.tsx(5 hunks)
🔇 Additional comments (1)
apps/roam/src/components/canvas/Tldraw.tsx (1)
611-616: Extract, Sanitize, and Cap Per-Canvas History with Proper Cleanup
- Move the
app.on("change", …)subscription into auseEffectthat runs once after mount (and whenever theapportitlechanges) to avoid re-attaching handlers on every render.- Capture the unsubscribe function returned by
app.onand invoke it in the effect’s cleanup callback to prevent orphaned listeners on remount.- Sanitize each entry via
redactHistoryEntryForEmailbefore adding it to the buffer to strip PII.- Store the capped history (max 10 entries) in
discourseContext.lastActionsByCanvas[title], isolating histories per canvas.Suggested refactor in
apps/roam/src/components/canvas/Tldraw.tsxaround lines 611–616:- app.on("change", (entry) => { - lastActionsRef.current.push(entry); - if (lastActionsRef.current.length > 10) - lastActionsRef.current.shift(); - }); + useEffect(() => { + const offChange = app.on("change", (entry) => { + const safeEntry = redactHistoryEntryForEmail(entry); + const buf = lastActionsRef.current; + buf.push(safeEntry); + while (buf.length > 10) buf.shift(); + discourseContext.lastActionsByCanvas[title] = buf; + }); + + return () => { + offChange(); + }; + }, [app, title]);If your version of
tldraw’sapp.ondoes not return an unsubscribe function, you’ll need to bind the listener in this effect and callapp.off("change", handler)(or the equivalent) in the cleanup.
Summary by CodeRabbit
Bug Fixes
Chores