Skip to content

Conversation

@trangdoan982
Copy link
Collaborator

@trangdoan982 trangdoan982 commented Nov 12, 2025

https://www.loom.com/share/cacf63e155ac462885ea98fa0f31af49

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced tooltip positioning for multi-line tags with cursor-aware dynamic placement to ensure tooltips appear at the most relevant location when hovering over tag content.

@linear
Copy link

linear bot commented Nov 12, 2025

@supabase
Copy link

supabase bot commented Nov 12, 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 12, 2025

✅ Actions performed

Full review triggered.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 12, 2025

📝 Walkthrough

Walkthrough

Implements mouse-tracking-based tooltip positioning for multi-line tag hovers in the Obsidian plugin. Adds cursor position state tracking, a proximity-based getClosestRect function to select the optimal tag rectangle, and dynamic tooltip repositioning via mousemove event listeners.

Changes

Cohort / File(s) Summary
Mouse-tracking tooltip positioning
apps/obsidian/src/utils/tagNodeHandler.ts
Adds currentMouseX and currentMouseY state variables to track cursor position; introduces handleMouseMove handler and getClosestRect function to dynamically select the closest tag-rect based on cursor proximity for multi-line tags; replaces fixed tooltip positioning with dynamic repositioning; wires mousemove event listener and adds cleanup on unmount

Sequence Diagram

sequenceDiagram
    participant User
    participant TagNode
    participant Handler as Event Handler
    participant getClosestRect as getClosestRect()
    participant Tooltip

    User->>TagNode: hover over multi-line tag
    activate TagNode
    TagNode->>Handler: showTooltip triggered
    activate Handler
    Handler->>getClosestRect: evaluate proximity to tag rects
    activate getClosestRect
    getClosestRect-->>Handler: return closest rect
    deactivate getClosestRect
    Handler->>Tooltip: position based on closest rect
    deactivate Handler
    deactivate TagNode
    
    User->>TagNode: move mouse over tag
    activate TagNode
    TagNode->>Handler: handleMouseMove (update cursor state)
    activate Handler
    Handler->>getClosestRect: re-evaluate proximity
    activate getClosestRect
    getClosestRect-->>Handler: return closest rect
    deactivate getClosestRect
    Handler->>Tooltip: reposition tooltip dynamically
    deactivate Handler
    deactivate TagNode
    
    User->>TagNode: move away from tag
    activate TagNode
    TagNode->>Handler: cleanup (remove mousemove listener)
    deactivate TagNode
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Event listener management: Verify proper cleanup of mousemove listener to prevent memory leaks and duplicate handlers
  • Proximity calculation logic: Review getClosestRect algorithm for correctness in multi-line scenarios and edge cases (overlapping rects, cursor near boundaries)
  • State synchronization: Ensure currentMouseX/Y updates don't cause stale closures or race conditions with tooltip positioning
  • Integration with existing hover logic: Confirm compatibility with existing showTooltip and hover state management

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 accurately describes the main change: fixing tooltip positioning for multi-line tags in Obsidian by implementing mouse-tracking-based dynamic positioning.
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/utils/tagNodeHandler.ts (1)

382-446: Seed initial mouse coordinates on hover

When the DOM reprocesses a multi-line tag while the cursor is already sitting over it (e.g., after color refresh or note re-render), mouseenter fires but no mousemove follows before showTooltip runs. Because currentMouseX/currentMouseY stay at 0, getClosestRect falls back to whichever fragment is closest to the viewport origin, so the tooltip still snaps to the wrong line. Please initialize the coordinates from the mouseenter event so the first call to getClosestRect uses the actual hover position.

Apply this diff:

-    tagElement.addEventListener("mouseenter", () => {
+    tagElement.addEventListener("mouseenter", (event: MouseEvent) => {
+      currentMouseX = event.clientX;
+      currentMouseY = event.clientY;
       if (hoverTimeout) {
         clearTimeout(hoverTimeout);
       }
       hoverTimeout = window.setTimeout(showTooltip, HOVER_DELAY);
     });

Also applies to: 503-508

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 38eae87 and 089cdb8.

📒 Files selected for processing (1)
  • apps/obsidian/src/utils/tagNodeHandler.ts (3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}

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

**/*.{ts,tsx}: Prefer type over interface
Use explicit return types for functions
Avoid any types when possible

Files:

  • apps/obsidian/src/utils/tagNodeHandler.ts
**/*.{ts,tsx,js,jsx}

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

**/*.{ts,tsx,js,jsx}: Prefer arrow functions over regular function declarations
Use named parameters (object destructuring) when a function has more than 2 parameters
Use Prettier with the project's configuration
Maintain consistent naming conventions: PascalCase for components and types
Maintain consistent naming conventions: camelCase for variables and functions
Maintain consistent naming conventions: UPPERCASE for constants

Files:

  • apps/obsidian/src/utils/tagNodeHandler.ts
apps/obsidian/**

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

Use the Obsidian style guide from help.obsidian.md/style-guide and docs.obsidian.md/Developer+policies for all code in the Obsidian plugin

Files:

  • apps/obsidian/src/utils/tagNodeHandler.ts
🧠 Learnings (1)
📚 Learning: 2025-08-25T15:53:21.799Z
Learnt from: sid597
Repo: DiscourseGraphs/discourse-graph PR: 372
File: apps/roam/src/components/DiscourseNodeMenu.tsx:116-116
Timestamp: 2025-08-25T15:53:21.799Z
Learning: In apps/roam/src/components/DiscourseNodeMenu.tsx, when handling tag insertion, multiple leading hashtags (like ##foo) should be preserved as they represent user intent, not normalized to a single hashtag. The current regex /^#/ is correct as it only removes one leading # before adding one back, maintaining any additional hashtags the user intended.

Applied to files:

  • apps/obsidian/src/utils/tagNodeHandler.ts

Copy link
Contributor

@mdroidian mdroidian left a comment

Choose a reason for hiding this comment

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

👍

@trangdoan982 trangdoan982 merged commit 18e7685 into main Nov 18, 2025
4 checks passed
@github-project-automation github-project-automation bot moved this to Done in General Nov 18, 2025
@trangdoan982 trangdoan982 deleted the eng-1048-bug-node-tag-not-rendered-correctly-if-tag-is-at-the-end-of branch November 18, 2025 01:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants