Skip to content

Conversation

@sid597
Copy link
Collaborator

@sid597 sid597 commented Sep 6, 2025

https://www.loom.com/share/e0ddc797613f49bfa78cd1e7611157fa?sid=ffc11a76-683a-40d1-8d29-767b70181a5f

Summary by CodeRabbit

  • Bug Fixes
    • Corrects tag popover alignment to center relative to the tag’s width for improved readability.
    • Ensures the overlay precisely matches the tag’s bounds, preventing visual overflow or clipping.
    • Improves popover positioning to appear consistently above/below the tag with appropriate offsets.
    • Resolves issues where the popover could appear shifted due to inaccurate size/position calculations.
    • Enhances overall stability of tag interaction by using reliable element measurements for layout.

@linear
Copy link

linear bot commented Sep 6, 2025

@supabase
Copy link

supabase bot commented Sep 6, 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 ↗︎.

Copy link
Collaborator Author

sid597 commented Sep 6, 2025

@sid597 sid597 changed the title Fix roam studio and node tag popup ENG-826 inconsistent 'create node' popover for candidate nodes Sep 6, 2025
@sid597 sid597 marked this pull request as ready for review September 6, 2025 16:05
@sid597
Copy link
Collaborator Author

sid597 commented Sep 6, 2025

@coderabbitai full review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 6, 2025

✅ Actions performed

Full review triggered.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 6, 2025

📝 Walkthrough

Walkthrough

Updates the tag popover positioning logic in renderNodeTagPopup: measures the parent’s bounding rect, sets container and target overlay dimensions to rect.width/height with absolute positioning, removes a redundant rect computation, and adjusts the popover offset to center relative to the tag’s width.

Changes

Cohort / File(s) Summary
Tag popover positioning
apps/roam/src/utils/renderNodeTagPopup.tsx
Compute parent bounding rect once; size reactRoot to rect width/height; make target span absolute at top/left 0 with rect-based width/height; remove duplicate rect calc; change Popover offset from "0, 10" to ${rect.width / 2}px, 10 to center popover.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs


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: 2

🧹 Nitpick comments (1)
apps/roam/src/utils/renderNodeTagPopup.tsx (1)

29-30: Prefer responsive sizing over fixed px to handle reflow/zoom.

Hard-coding px width/height can drift on font/zoom changes and wrapping. Using 100% keeps the overlay in sync with its wrapper.

-  reactRoot.style.width = `${rect.width}px`;
-  reactRoot.style.height = `${rect.height}px`;
+  reactRoot.style.width = "100%";
+  reactRoot.style.height = "100%";
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3109273 and 66ba630.

📒 Files selected for processing (1)
  • apps/roam/src/utils/renderNodeTagPopup.tsx (4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: Graphite / mergeability_check
  • GitHub Check: Graphite / mergeability_check
  • GitHub Check: Graphite / mergeability_check
  • GitHub Check: Graphite / mergeability_check
  • GitHub Check: Graphite / mergeability_check
  • GitHub Check: Graphite / mergeability_check
  • GitHub Check: Graphite / mergeability_check
🔇 Additional comments (1)
apps/roam/src/utils/renderNodeTagPopup.tsx (1)

75-77: Anchoring the target overlay is correct.

Adding top: 0, left: 0, and position: absolute on the target improves hitbox alignment with the tag.

Also applies to: 79-79

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.

Added via Giphy

@sid597 sid597 merged commit cdbda8a into eng-752-node-formalization-menu-doesnt-pop-up-in-query-results-forr Sep 7, 2025
5 of 6 checks passed
@github-project-automation github-project-automation bot moved this to Done in General Sep 7, 2025
@sid597 sid597 deleted the eng-826-inconsistent-create-node-popover-for-candidate-nodes branch September 7, 2025 04:37
sid597 added a commit that referenced this pull request Sep 7, 2025
…forr (#417)

* Fix on hover show popup

* Fix roam studio and node tag popup (#421)
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