Skip to content

Conversation

@sid597
Copy link
Collaborator

@sid597 sid597 commented Sep 6, 2025

@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

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@sid597 sid597 changed the title Move the new block as first child of the current block ENG-829 formalizing a block with a node tag that contains an image kills the image Sep 6, 2025
@sid597 sid597 changed the title ENG-829 formalizing a block with a node tag that contains an image kills the image ENG-829 Sep 6, 2025
@sid597 sid597 changed the title ENG-829 ENG-829 Move candidate node as child of new discourse node Sep 6, 2025
@sid597 sid597 marked this pull request as ready for review September 6, 2025 19:02
@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

Modifies CreateNodeDialog to change how a page reference is inserted when a sourceBlockUid is present: instead of updating the source block’s text, it queries the source block’s parent and order, creates a new sibling block with the page reference, and moves the original source block under the new block.

Changes

Cohort / File(s) Summary
CreateNode placement refactor
apps/roam/src/components/CreateNodeDialog.tsx
Import createBlock; when sourceBlockUid exists, query parentUid and order via roamAlphaAPI, create a new block with pageRef under the same parent, then move the original source block to be a child of the new block at order 0. No exported API changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant CreateNodeDialog
  participant roamAlphaAPI
  participant createBlock

  User->>CreateNodeDialog: Confirm create node (with sourceBlockUid)
  CreateNodeDialog->>roamAlphaAPI: Datalog query to get parentUid, order
  Note right of roamAlphaAPI: Returns parentUid and order of source block
  roamAlphaAPI-->>CreateNodeDialog: parentUid, order

  CreateNodeDialog->>createBlock: Create new block under parentUid with pageRef
  Note right of createBlock: Returns newBlockUid
  createBlock-->>CreateNodeDialog: newBlockUid

  CreateNodeDialog->>roamAlphaAPI: Move sourceBlockUid under newBlockUid at order 0
  Note over CreateNodeDialog,roamAlphaAPI: Source block becomes a child of the pageRef block
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 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: 3

🧹 Nitpick comments (1)
apps/roam/src/components/CreateNodeDialog.tsx (1)

14-14: Import looks good; drop now-unused updateBlock.

Since you switched to createBlock, updateBlock (Line 6) appears unused; consider removing to keep lint clean.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

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

📒 Files selected for processing (1)
  • apps/roam/src/components/CreateNodeDialog.tsx (3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-22T10:40:52.752Z
Learnt from: sid597
PR: DiscourseGraphs/discourse-graph#232
File: apps/roam/src/utils/getAllDiscourseNodesSince.ts:18-31
Timestamp: 2025-06-22T10:40:52.752Z
Learning: In apps/roam/src/utils/getAllDiscourseNodesSince.ts, the user confirmed that querying for `?title` with `:node/title` and mapping it to the `text` field in the DiscourseGraphContent type is the correct implementation for retrieving discourse node content from Roam Research, despite it appearing to query page titles rather than block text content.

Applied to files:

  • apps/roam/src/components/CreateNodeDialog.tsx
⏰ 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). (6)
  • 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

@sid597 sid597 requested a review from mdroidian September 6, 2025 19:26
Base automatically changed from eng-826-inconsistent-create-node-popover-for-candidate-nodes to eng-752-node-formalization-menu-doesnt-pop-up-in-query-results-forr September 7, 2025 04:37
Base automatically changed from eng-752-node-formalization-menu-doesnt-pop-up-in-query-results-forr to eng-693-handle-in-tag September 7, 2025 04:38
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.

Consider using roamjs-component functions instead rewriting queries.

@sid597 sid597 merged commit d7c0aca into eng-693-handle-in-tag Sep 8, 2025
7 of 8 checks passed
@github-project-automation github-project-automation bot moved this to Done in General Sep 8, 2025
sid597 added a commit that referenced this pull request Sep 8, 2025
… to use # (#420)

* use text not tag

* Move the new block as first child of the current block (#422)
sid597 added a commit that referenced this pull request Sep 8, 2025
* modify dom only for node tags

* add background color to a nodetag

* use it as color not background color

* remove unused refresh

* Roam: ENG-693 handle node tags with # in front and update placeholder to use # (#420)

* use text not tag

* Move the new block as first child of the current block (#422)
sid597 added a commit that referenced this pull request Sep 8, 2025
* use getDiscourseNodes

* Eng-737 use node color to style node tags (#424)

* modify dom only for node tags

* add background color to a nodetag

* use it as color not background color

* remove unused refresh

* Roam: ENG-693 handle node tags with # in front and update placeholder to use # (#420)

* use text not tag

* Move the new block as first child of the current block (#422)
trangdoan982 pushed a commit that referenced this pull request Oct 3, 2025
* use getDiscourseNodes

* Eng-737 use node color to style node tags (#424)

* modify dom only for node tags

* add background color to a nodetag

* use it as color not background color

* remove unused refresh

* Roam: ENG-693 handle node tags with # in front and update placeholder to use # (#420)

* use text not tag

* Move the new block as first child of the current block (#422)
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