Skip to content

Conversation

@mdroidian
Copy link
Contributor

@mdroidian mdroidian commented Jan 5, 2026

https://www.loom.com/share/09b39df601db4a93b47359a38b4ad6ae

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Improved rendering accuracy for shape indicators to reflect actual geometry calculations with enhanced precision.

✏️ Tip: You can customize this high-level summary in your review settings.

@linear
Copy link

linear bot commented Jan 5, 2026

@supabase
Copy link

supabase bot commented Jan 5, 2026

This pull request has been ignored for the connected project zytfjzqyijgagqxrzbmz because there are no changes detected in packages/database/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@mdroidian
Copy link
Contributor Author

@coderabbitai full review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 5, 2026

✅ Actions performed

Full review triggered.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 5, 2026

📝 Walkthrough

Walkthrough

Changes modify how indicator dimensions are rendered in Discourse node shapes across Obsidian and Roam integrations. Instead of using stored property values, width and height are now computed from the shape's actual rendered geometry via getShapeGeometry() and adjusted with toDomPrecision for accurate DOM rendering.

Changes

Cohort / File(s) Summary
Indicator dimension calculation
apps/obsidian/src/components/canvas/shapes/DiscourseNodeShape.tsx, apps/roam/src/components/canvas/DiscourseNodeUtil.tsx
Indicator width/height now derived from shape geometry via this.editor.getShapeGeometry(shape) with toDomPrecision applied, replacing direct shape.props.w and shape.props.h references. Adds toDomPrecision import in Roam file.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly references the specific issue (ENG-995) and accurately describes the main change: fixing node bounding box alignment with shape borders in tldraw.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 903ae89 and db84789.

📒 Files selected for processing (2)
  • apps/obsidian/src/components/canvas/shapes/DiscourseNodeShape.tsx
  • apps/roam/src/components/canvas/DiscourseNodeUtil.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{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
Prefer type over interface in TypeScript
Use explicit return types for functions
Avoid any types 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/DiscourseNodeUtil.tsx
  • apps/obsidian/src/components/canvas/shapes/DiscourseNodeShape.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/DiscourseNodeUtil.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/DiscourseNodeUtil.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/DiscourseNodeUtil.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/DiscourseNodeUtil.tsx
apps/obsidian/**

📄 CodeRabbit inference engine (.cursor/rules/obsidian.mdc)

apps/obsidian/**: Prefer existing dependencies from apps/obsidian/package.json when adding dependencies to the Obsidian plugin
Follow the Obsidian style guide from help.obsidian.md/style-guide and docs.obsidian.md/Developer+policies for UI and code styling
Use Lucide and custom Obsidian icons alongside detailed elements to provide visual representation of features in platform-native UI

Files:

  • apps/obsidian/src/components/canvas/shapes/DiscourseNodeShape.tsx
🧠 Learnings (2)
📚 Learning: 2025-11-25T00:52:41.934Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
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/DiscourseNodeUtil.tsx
📚 Learning: 2025-11-25T00:52:41.934Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-11-25T00:52:41.934Z
Learning: Applies to apps/roam/**/*.{ts,tsx,js,jsx} : Use Roam Depot/Extension API docs from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU when implementing extension functionality

Applied to files:

  • apps/roam/src/components/canvas/DiscourseNodeUtil.tsx
🔇 Additional comments (4)
apps/obsidian/src/components/canvas/shapes/DiscourseNodeShape.tsx (2)

16-16: LGTM! Necessary import for DOM precision.

The toDomPrecision import is correctly added to support the updated indicator rendering logic.


175-183: Excellent fix for bounding box alignment!

Using getShapeGeometry() to derive width and height from the actual rendered bounds (instead of shape.props.w/h) correctly aligns the indicator with the shape's borders. The toDomPrecision wrapper ensures proper DOM measurement precision. Both APIs are consistently available across tldraw versions (2.3.0 and 3.14.2) and are successfully used throughout the codebase.

apps/roam/src/components/canvas/DiscourseNodeUtil.tsx (2)

24-24: LGTM! Necessary import for DOM precision.

The toDomPrecision import is correctly added to support the updated indicator rendering logic, maintaining consistency with the Obsidian implementation.


427-435: Excellent fix with cross-platform consistency!

The indicator implementation correctly mirrors the Obsidian change, using getShapeGeometry() to derive dimensions from actual rendered bounds instead of stored props. This ensures consistent bounding box alignment across both integrations.

Critical version compatibility check needed: Roam uses tldraw 2.3.0 while Obsidian uses 3.14.2. The web search requested in the Obsidian file review will verify API availability in 2.3.0. If these APIs don't exist in 2.3.0, this change would break the Roam integration at runtime.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@mdroidian mdroidian merged commit 66e151a into main Jan 5, 2026
5 checks passed
@mdroidian mdroidian deleted the eng-995-node-bounding-box-doesnt-align-with-borders-of-the-shape-in branch January 5, 2026 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants