Skip to content

Conversation

@mdroidian
Copy link
Contributor

@mdroidian mdroidian commented Oct 16, 2025

Re-add document.body.click() to remove focus after node tag creation in Roam.

This restores the expected UI behavior where the app exits block editing mode after a node tag (candidate node) is created, addressing a regression introduced by the removal of document.click in ENG-931.


Linear Issue: ENG-954

Open in Cursor Open in Web

Summary by CodeRabbit

  • Bug Fixes
    • Tags now display with correct styling immediately after creation in the Node Menu, improving visual feedback during tag operations.

Co-authored-by: mclicks <mclicks@gmail.com>
@cursor
Copy link

cursor bot commented Oct 16, 2025

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
Learn more about Cursor Agents

@linear
Copy link

linear bot commented Oct 16, 2025

@supabase
Copy link

supabase bot commented Oct 16, 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 ↗︎.

…seNodeMenu

Co-authored-by: mclicks <mclicks@gmail.com>
@mdroidian mdroidian marked this pull request as ready for review October 16, 2025 05:49
@mdroidian
Copy link
Contributor Author

@coderabbitai full review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 16, 2025

✅ Actions performed

Full review triggered.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 16, 2025

📝 Walkthrough

Walkthrough

A document body click is added to the tag insertion logic in DiscourseNodeMenu.tsx to remove focus from the block immediately after tag creation via the Node Menu, ensuring CSS styling is applied without delay.

Changes

Cohort / File(s) Change Summary
Tag Creation Focus Fix
apps/roam/src/components/DiscourseNodeMenu.tsx
Added document.body.click() call after setTimeout during tag insertion to blur the active block and trigger immediate CSS application

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • #477: Modifies tag-insertion and focus timing logic in DiscourseNodeMenu.tsx using similar setTimeout-based approaches.
  • #154: Adds document.body.click() blur behavior to DiscourseNodeMenu.tsx after node selection to address post-selection focus issues.

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 pull request title "Re-add focus removal after node tag creation" is directly aligned with the main change described in the raw summary. The change adds a document.body.click() call to remove focus from the block after creating a tag via the Node Menu, which is exactly what the title conveys through the terms "focus removal" and "node tag creation." The title is concise, clear, and specific without vague terminology or unnecessary noise, and it accurately reflects the restoration of previously removed functionality as indicated by the "Re-add" prefix.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ffc313a and 6d93247.

📒 Files selected for processing (1)
  • apps/roam/src/components/DiscourseNodeMenu.tsx (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/roam/src/components/DiscourseNodeMenu.tsx
🔇 Additional comments (1)
apps/roam/src/components/DiscourseNodeMenu.tsx (1)

133-135: Verify the timing is correct relative to the node creation path.

This change successfully re-introduces focus removal after tag insertion, addressing the regression from ENG-931. However, note the timing difference compared to the node creation path:

  • Node creation (line 83): document.body.click() is called before setTimeout
  • Tag insertion (line 135): document.body.click() is called after setTimeout (but before the callback executes)

Since addTagToBlock captures textarea.value and cursorPos in its closure, this timing should be safe. However, the inconsistency between the two paths could lead to subtle behavioral differences or future maintenance confusion.

Please verify that calling document.body.click() after scheduling (rather than before, as in the node creation path) produces the expected behavior and doesn't introduce any edge cases where the block update might fail or the UI state becomes inconsistent.

If both timings work correctly, consider adding a brief comment explaining why the timing differs between the two paths, or unifying them if possible.


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 changed the title Re-add focus removal after node tag creation ENG-954 Re-add focus removal after node tag creation Oct 16, 2025
@mdroidian mdroidian merged commit d61d393 into main Oct 16, 2025
5 checks passed
@mdroidian mdroidian deleted the cursor/ENG-954-re-add-focus-removal-after-node-tag-creation-d2b9 branch October 16, 2025 05:59
@github-project-automation github-project-automation bot moved this to Done in General Oct 16, 2025
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