[ENG-877] Initial Implementation node tags for Obsidian #455

trangdoan982 · 2025-09-24T23:52:36Z

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

Summary by CodeRabbit

New Features
- Discourse tags in Markdown are auto-styled with distinct background/text colors per node type for better readability.
- Hovering a discourse tag shows a tooltip with a “Create [NodeType]” button.
- Clicking “Create” opens a modal to create the node and replaces the tag with a link to the new item.
- Works dynamically as you edit and across view changes.
Style
- Added styling for the “Create Node” button with hover effects for clearer, more consistent UI.

linear · 2025-09-24T23:52:39Z

supabase · 2025-09-24T23:52:45Z

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 · 2025-09-25T17:30:17Z

@CodeRabbit full review

coderabbitai · 2025-09-25T17:30:24Z

✅ Actions performed

Full review triggered.

coderabbitai · 2025-09-25T17:38:13Z

📝 Walkthrough

Walkthrough

Adds TagNodeHandler lifecycle integration in the Obsidian plugin, introduces a TagNodeHandler utility for DOM observation and tag-to-node creation flow, adds color utilities for node theming, and updates stylesheet with a button class used by the new tooltip UI.

Changes

Cohort / File(s)	Summary
Plugin lifecycle integration `apps/obsidian/src/index.ts`	Adds private `tagNodeHandler` member; initializes it in `onload()` with try/catch and `initialize()`; cleans up in `onunload()` via `cleanup()` and null reset.
Tag handling and UI flow `apps/obsidian/src/utils/tagNodeHandler.ts`	New `TagNodeHandler` class: observes Markdown view for tag elements, applies styles, shows hover tooltip with “Create [NodeType]” button, opens modal, creates node file, and replaces tag with link; includes init/refresh/cleanup and error handling.
Color utilities `apps/obsidian/src/utils/colorUtils.ts`	New color utils: defines palette/order; exports `formatHexColor`, `getContrastColor`, `getDiscourseNodeColors`, and `getAllDiscourseNodeColors`; uses `DiscourseNode` type.
Styles for create button `apps/obsidian/styles.css`	Adds `.dg-create-node-button` and hover state with variables-driven colors, padding, radius, and transition.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant Obsidian as Obsidian Plugin
  participant TagHandler as TagNodeHandler
  participant View as Markdown View (DOM)
  participant Modal as CreateNodeModal
  participant FS as createDiscourseNodeFile

  Obsidian->>TagHandler: initialize() on load
  TagHandler->>View: Attach MutationObserver
  Note over TagHandler,View: Detect and style tag elements

  User-->>View: Hover tag
  View->>TagHandler: Tag hover event
  TagHandler->>User: Show tooltip with "Create [Type]" button

  User-->>TagHandler: Click "Create"
  TagHandler->>Modal: Open with prefilled content
  Modal-->>TagHandler: Confirm (node metadata)

  TagHandler->>FS: Create node file
  FS-->>TagHandler: Path/Success or Error

  alt Success
    TagHandler->>View: Replace tag with link
  else Error
    TagHandler->>User: Notify error
  end

sequenceDiagram
  autonumber
  actor Obsidian
  participant Plugin as DiscourseGraphPlugin
  participant TagHandler as TagNodeHandler

  Obsidian->>Plugin: onload()
  Plugin->>TagHandler: new + initialize()
  Note over TagHandler: Observe DOM, register events

  Obsidian->>Plugin: onunload()
  Plugin->>TagHandler: cleanup()
  Note over TagHandler: Disconnect observers, remove tooltips

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

[ENG-410] Node creation dialog #194 — Introduces CreateNodeModal and createDiscourseNode APIs used by TagNodeHandler for node creation and tag replacement.
[ENG-301] Create node in right-click menu #152 — Refactors/expands discourse-node creation utilities that TagNodeHandler invokes during the convert-tag-to-node workflow.
[ENG-40] Identify node obsidian #105 — Modifies plugin load/unload flow in index.ts, overlapping with TagNodeHandler initialization and cleanup wiring.

Pre-merge checks

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.
Title Check	✅ Passed	The title clearly references the core change—initially implementing node tags support for Obsidian—concise and directly aligned with the pull request’s main functionality.

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.}

coderabbitai

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

apps/obsidian/src/index.ts (1)
204-207: Refresh tag colors after settings save.

When nodeTypes or their colors change, tags in the editor won’t update until a reload. Call refreshColors() after saving.
 async saveSettings() {
   await this.saveData(this.settings);
   this.updateFrontmatterStyles();
+  this.tagNodeHandler?.refreshColors();
 }

🧹 Nitpick comments (6)

apps/obsidian/styles.css (1)
21-36: Add focus-visible state for keyboard accessibility.

The button has hover styling but no visible focus indicator, which is a11y-hostile. Add a clear focus-visible style.
 .dg-create-node-button {
   background: var(--interactive-accent);
   color: var(--text-on-accent);
   border: none;
   border-radius: 4px;
   padding: 6px 12px;
   font-size: 11px;
   cursor: pointer;
   font-weight: 500;
   white-space: nowrap;
-  transition: background-color 0.2s ease;
+  transition: background-color 0.2s ease, box-shadow 0.2s ease;
 }
 
 .dg-create-node-button:hover {
   background: var(--interactive-accent-hover);
 }
+
+.dg-create-node-button:focus-visible {
+  outline: none;
+  box-shadow: 0 0 0 2px var(--background-primary), 0 0 0 4px var(--interactive-accent);
+}
apps/obsidian/src/utils/colorUtils.ts (1)

39-49: Support short hex and invalid inputs more gracefully.

formatHexColor rejects 3/8-digit hex and returns empty string on invalid input (falling back later). Consider supporting #rgb/#rgba/#rrggbbaa or log/normalize to reduce surprises.

apps/obsidian/src/utils/tagNodeHandler.ts (4)

170-192: Avoid duplicate handler attachment; ensure keyboard support.

Marking processed via data attribute prevents duplicates, but keyboard users can’t access the action since it’s hover-driven. Consider adding a click handler on the tag itself and key handlers (Enter/Space) to trigger the tooltip/action.

240-256: Initial title cleaning strips all hashtags; confirm intent.

cleanText removes any #word sequences from the pre-tag content. If users intentionally included leading ##, we may over-strip. See learning from PR #372 about preserving multiple leading hashtags.

Consider limiting removal to a single leading # on a single trailing tag marker rather than all hashtags in the prefix.

340-441: Make tooltip accessible and robust.

Tooltip is hover-only and not constrained to viewport; add keyboard trigger and simple viewport clamping.

Store and remove event listeners in cleanup to avoid potential leaks if elements persist across reprocessing.

517-545: Cleanup should also clear event listeners.

You remove attributes/styles but keep mouseenter/leave listeners attached to surviving elements. Store handler refs and remove them during cleanup.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a1fb479 and f5cbb89.

📒 Files selected for processing (4)

apps/obsidian/src/index.ts (4 hunks)
apps/obsidian/src/utils/colorUtils.ts (1 hunks)
apps/obsidian/src/utils/tagNodeHandler.ts (1 hunks)
apps/obsidian/styles.css (1 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-08-25T15:53:21.799Z

Learnt from: sid597
PR: DiscourseGraphs/discourse-graph#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

🔇 Additional comments (6)

apps/obsidian/src/index.ts (2)

37-44: LGTM: lifecycle wiring of TagNodeHandler.

Instantiation, guarded init, and error handling look solid.

214-217: LGTM: cleanup on unload.

Ensures observers and UI are torn down cleanly.

apps/obsidian/src/utils/colorUtils.ts (2)

80-93: Index bounds handling and palette fallback are fine.

Safe indexing and contrast computation look good.

1-1: Correct DiscourseNode shape; no changes needed. The DiscourseNode imported from apps/obsidian/src/types.ts includes id, name, and optional color, so getDiscourseNodeColors is correctly typed.

apps/obsidian/src/utils/tagNodeHandler.ts (2)

5-6: createDiscourseNodeFile and formatNodeName exports verified.

135-151: Custom cm-tag- classes confirmed*
Verified no default cm-hashtag references exist; this plugin’s CodeMirror extension defines and applies the cm-tag-* classes.

apps/obsidian/src/utils/tagNodeHandler.ts

mdroidian

A few requested changes/nits.

Let's updating this PR's title to include the Linear ticket number.

Additionally,

make sure to call CodeRabbit and review before submitting. I have called it on this PR now but have not reviewed it's comments/nits. Please go ahead look them over.
make sure to format files with prettier before submitting (eg: colorUtils.ts), this is normally done on save.

Finally, a few higher level issues:

Multiple DOM creations for each popup
https://www.loom.com/share/b51df53bda6941798968bad126eb3417

Observer not unloading
https://www.loom.com/share/df15e313b89e4caca5a3fa58f9209a0f

apps/obsidian/src/utils/colorUtils.ts

apps/obsidian/src/utils/tagNodeHandler.ts

mdroidian

Some comments and a bug:
https://www.loom.com/share/7d1ebf66eff24ea0bb83fa87b030b922

apps/obsidian/src/utils/colorUtils.ts

apps/obsidian/src/utils/tagNodeHandler.ts

mdroidian

👍

github-project-automation bot added this to General Sep 24, 2025

trangdoan982 requested a review from mdroidian September 25, 2025 00:01

coderabbitai bot reviewed Sep 25, 2025

View reviewed changes

apps/obsidian/src/utils/tagNodeHandler.ts Outdated Show resolved Hide resolved

apps/obsidian/src/utils/tagNodeHandler.ts Show resolved Hide resolved

trangdoan982 changed the title ~~Node tags for Obsidian~~ Initial Implementation node tags for Obsidian Sep 25, 2025

mdroidian requested changes Sep 25, 2025

View reviewed changes

trangdoan982 force-pushed the eng-877-node-tags-obsidian branch from dc58d6f to cd1feba Compare October 1, 2025 19:58

trangdoan982 changed the title ~~Initial Implementation node tags for Obsidian~~ [ENG-877] Initial Implementation node tags for Obsidian Oct 2, 2025

trangdoan982 added 6 commits October 2, 2025 14:30

feature complete

ef2a057

reorganize

ea9b9fc

rm box shadow style

3c6c359

address some PR comments

d4c7558

some problem sovled

c43ff6b

address all PR comments

b10257e

trangdoan982 force-pushed the eng-877-node-tags-obsidian branch from cd1feba to b10257e Compare October 2, 2025 19:36

trangdoan982 requested a review from mdroidian October 2, 2025 19:36

sm fix

f956cd8

mdroidian requested changes Oct 5, 2025

View reviewed changes

mdroidian mentioned this pull request Oct 6, 2025

[ENG-926] Add tag as a field in setting (Obsidian) #475

Merged

address PR comments

4596a88

trangdoan982 requested a review from mdroidian October 10, 2025 21:04

mdroidian approved these changes Oct 12, 2025

View reviewed changes

trangdoan982 merged commit ffc313a into main Oct 13, 2025
4 checks passed

github-project-automation bot moved this to Done in General Oct 13, 2025

trangdoan982 deleted the eng-877-node-tags-obsidian branch October 13, 2025 14:06

coderabbitai bot mentioned this pull request Nov 12, 2025

[ENG-1048] Fix the tooltip position in Obsidian #547

Merged

coderabbitai bot mentioned this pull request Nov 18, 2025

[ENG-1052] Enable node tags via command key #554

Merged

[ENG-877] Initial Implementation node tags for Obsidian #455

[ENG-877] Initial Implementation node tags for Obsidian #455

Uh oh!

Conversation

trangdoan982 commented Sep 24, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

linear bot commented Sep 24, 2025

Uh oh!

supabase bot commented Sep 24, 2025

Uh oh!

mdroidian commented Sep 25, 2025

Uh oh!

coderabbitai bot commented Sep 25, 2025

Uh oh!

coderabbitai bot commented Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Pre-merge checks

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mdroidian left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mdroidian left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mdroidian left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

trangdoan982 commented Sep 24, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Sep 25, 2025 •

edited

Loading