Skip to content

Conversation

@trangdoan982
Copy link
Collaborator

@trangdoan982 trangdoan982 commented Nov 28, 2025

https://www.loom.com/share/55f0fa6f11e047a6a638b06ad358ec10

Summary by CodeRabbit

  • Refactor
    • Improved canvas view switching behavior with optimized timing and staged activation.
    • Enhanced reliability of canvas preview functionality through refined state management.

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

@linear
Copy link

linear bot commented Nov 28, 2025

@supabase
Copy link

supabase bot commented Nov 28, 2025

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 ↗︎.

@trangdoan982
Copy link
Collaborator Author

@coderabbitai full review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 28, 2025

✅ Actions performed

Full review triggered.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 28, 2025

📝 Walkthrough

Walkthrough

The changes implement a deferred canvas view-switching mechanism for the Obsidian plugin. Instead of immediately switching canvas views on file-open, the system now marks files in a tracking set and defers the switch until the active leaf changes, improving coordination with canvas readiness.

Changes

Cohort / File(s) Summary
Canvas view switching refactoring
apps/obsidian/src/index.ts
Introduces pendingCanvasSwitches set for tracking deferred canvas view switches. Replaces eager per-file switch logic on file-open with deferred marking; switches views during active-leaf-change events only if the switch is pending and canvas-indicating frontmatter is detected. Registers "View as canvas" action after confirming canvas-enable status and tracks it in currentViewActions.

Sequence Diagram

sequenceDiagram
    participant User
    participant FileOpen as File Open Event
    participant PendingState as Pending Switches
    participant LeafChange as Active Leaf Change Event
    participant ViewMgr as View Manager
    participant Canvas as Canvas Preview

    User->>FileOpen: Opens Markdown file with canvas frontmatter
    FileOpen->>PendingState: Mark file path in pendingCanvasSwitches
    PendingState-->>FileOpen: Marked (deferred)
    
    User->>LeafChange: Navigate/focus changes
    LeafChange->>PendingState: Check if file has pending switch
    
    alt Pending Switch Exists
        PendingState->>ViewMgr: Confirm not already in canvas view
        ViewMgr->>Canvas: Switch to VIEW_TYPE_TLDRAW_DG_PREVIEW
        Canvas-->>ViewMgr: View switched
        ViewMgr->>PendingState: Clear pending state
        PendingState-->>LeafChange: Cleared
    else No Pending Switch
        PendingState-->>LeafChange: No action needed
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Logic complexity: The deferred switching mechanism introduces state tracking across multiple event handlers (file-open, active-leaf-change) requiring careful trace-through to ensure no race conditions.
  • Conditional branching: Multiple conditionals around canvas-enable status, pending state checks, and view type verification add decision density.
  • State management: Introduction of pendingCanvasSwitches set requires verification of proper initialization, mutation, and cleanup patterns.

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 accurately reflects the main change: implementing immediate canvas view opening for files, which aligns with the PR's shift to deferred view switching on file-open events.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
apps/obsidian/src/index.ts (1)

81-90: file-open handler correctly scopes pending switches; consider clearing rare stale entries

The file-open handler’s frontmatter check ensures only canvas‑enabled files are marked in pendingCanvasSwitches, which keeps the deferred logic focused and cheap. If you want to be extra defensive, you could also clear entries whose cache no longer has FRONTMATTER_KEY (e.g., in the "active-leaf-change" path when isCanvasFile is false) to avoid very rare cases where frontmatter is edited between file-open and the next leaf change, leaving an entry that will never be consumed.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 20199dc and aeb9285.

📒 Files selected for processing (1)
  • apps/obsidian/src/index.ts (3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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/obsidian/src/index.ts
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/index.ts
🧬 Code graph analysis (1)
apps/obsidian/src/index.ts (1)
apps/obsidian/src/constants.ts (2)
  • FRONTMATTER_KEY (72-72)
  • VIEW_TYPE_TLDRAW_DG_PREVIEW (79-79)
🔇 Additional comments (1)
apps/obsidian/src/index.ts (1)

28-76: Deferred canvas switching via pendingCanvasSwitches looks solid; confirm event ordering

Keying pendingCanvasSwitches by file.path and consuming it in the "active-leaf-change" handler (with the VIEW_TYPE_TLDRAW_DG_PREVIEW guard) is a clean way to avoid switching before the canvas view is ready and to prevent redundant switches on an already‑canvas leaf. One thing to verify manually is that in your target Obsidian version the "file-open" event fires before (or at least not strictly after) the corresponding "active-leaf-change" for a newly opened file, so that the pending entry is visible on that first leaf activation and the auto‑switch really happens “right when open” without requiring another leaf change.

@trangdoan982 trangdoan982 force-pushed the eng-606-open-the-canvas-view-immediately-when-new-file-is-opened branch from aeb9285 to 6d2d1c4 Compare December 4, 2025 22:01
@trangdoan982 trangdoan982 merged commit 373aeaf into main Dec 15, 2025
4 checks passed
@trangdoan982 trangdoan982 deleted the eng-606-open-the-canvas-view-immediately-when-new-file-is-opened branch December 15, 2025 15: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.

3 participants