Skip to content

ENG-1373: Drag pages onto the canvas to create node shapes#1083

Open
sid597 wants to merge 18 commits into
mainfrom
eng-1373-drag-and-drop-pages-to-canvas
Open

ENG-1373: Drag pages onto the canvas to create node shapes#1083
sid597 wants to merge 18 commits into
mainfrom
eng-1373-drag-and-drop-pages-to-canvas

Conversation

@sid597

@sid597 sid597 commented May 25, 2026

Copy link
Copy Markdown
Collaborator
eng-1373.mp4

Page refs aren't draggable by default, so make them draggable via the
existing rm-page-ref observer and route the drag payload through the canvas
text-content handler that already turns [[name]] into a discourse-node shape.

- Always-on: only the draggable attribute is global; the drag payload and
  drop handling live in the canvas component, so drags are inert unless a
  canvas is open.
- Custom MIME (application/x-roam-page), not text/plain, so a drag can't leak
  [[title]] into other blocks or apps.
- Guard the existing block-dragstart so dragging a ref doesn't also attach its
  containing block's UID.
@linear-code

linear-code Bot commented May 25, 2026

Copy link
Copy Markdown

ENG-1373

@supabase

supabase Bot commented May 25, 2026

Copy link
Copy Markdown

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 ↗︎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 014d57e942

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/roam/src/utils/initializeObserversAndListeners.ts Outdated
sid597 added 3 commits May 25, 2026 15:08
The always-on registration had no matching teardown, so the singleton
rm-page-ref observer and the makePageRefDraggable callback survived extension
unload/reload. Add a cleanup that unregisters the handler; for users without
the overlay/preview/suggestive handlers this empties the set and disconnects
the observer.
The first commit only captured spans whose data-link-title lives on the
parent element. Query-builder result rows and the discourse-context overlay
render page links as <a class="rm-page-ref" data-link-title="..."> with the
title on the element itself, which the existing parent-only read missed.

Also resolve a dragged block whose text is exactly [[X]] to that discourse
node's shape (instead of a gray blck-node) when X is a discourse node. The
page-ref branch already does this, so factor out tryCreatePageNodeShape and
reuse it from the block-ref path; the heavy page-uid + discourse-node lookup
is gated behind a cheap PAGE_REF_REGEX match on the block text.

- Read data-link-title on the page-ref element itself, falling back to the
  parent for span page-refs.
- Extract tryCreatePageNodeShape; the boolean return lets callers decide
  the fallback (default text handler vs. blck-node).
- Hoist PAGE_REF_REGEX to module scope next to BLOCK_REF_REGEX usage.
Comment thread apps/roam/src/components/canvas/Tldraw.tsx Outdated
Comment thread apps/roam/src/components/canvas/Tldraw.tsx Outdated
@sid597
sid597 requested a review from mdroidian May 29, 2026 05:53
@mdroidian

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep them coming!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@mdroidian mdroidian left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

I think this needs one more pass before merging.

The goal is to drag a page reference from inside an outliner block onto the canvas. Right now, mixed-content blocks still open the page instead of allowing drag.

It also looks like this changes the behavior when a page reference is the only content in a block: dragging the block now creates a page node instead of a block node. I’m not sure we want to change that existing block-drag behavior.

Expected behavior: dragging the specific page reference should add that page to the canvas, similar to the discourse context behavior, without changing how the containing block itself drags.

Could you also test/show:

  • link/render/embed query views
  • multiple canvases, including sidebar canvases, to check useEffect behavior

Comment thread apps/roam/src/components/canvas/Tldraw.tsx Outdated
Comment thread apps/roam/src/components/canvas/Tldraw.tsx
Comment thread apps/roam/src/components/canvas/Tldraw.tsx Outdated
@vercel

vercel Bot commented Jun 16, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
discourse-graph Skipped Skipped Jul 22, 2026 3:48pm

Request Review

@mattakamatsu

Copy link
Copy Markdown
Contributor

Ping! I wanted to use this feature recently, would love to put it back on @sid597's backlog, thankx

Query link and alias views render page refs as a.rm-page-ref anchors,
which the shared SPAN-only page-ref observer never sees, so those
results offered no drag handle. The drag-source system now observes
anchors with its own createHTMLObserver and runs them through the same
listener wiring (observePageRef generalized to HTMLElement); observed
locally so anchors are not fed to the shared observer's other handlers
(overlays, previews).

Verified by the eng-1373-query-views storyboard: its link-view drag
scene failed before this change and passes after. The query-page crash
seen on the dev graph while testing (matchDiscourseNode on relations
pointing at deleted node types) is pre-existing on main and will be
fixed separately.
@sid597
sid597 requested a review from mdroidian July 17, 2026 10:14

@mdroidian mdroidian left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just listing some discussion points from Friday

  • Use absolute positioning for the drag handle so it overlays the text instead of causing text reflow on hover.
  • Change the drag handle icon color from red to gray so it matches the existing icon palette.
  • Fix the post-drag bug where the handle disappears from other blocks until the next click, likely due to the observer not re-firing.

Comment thread apps/roam/src/components/canvas/Tldraw.tsx
@sid597
sid597 requested a review from mdroidian July 19, 2026 17:41
@sid597

sid597 commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator Author

@mdroidian Fixed all 3 points and updated the video to use latest code

@mdroidian

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 516d39eccb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +339 to +344
const pageRefAnchorObserver = createHTMLObserver({
useBody: true,
tag: "A",
className: "rm-page-ref",
callback: observePageRef,
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Attach drag listeners to existing page-link anchors

When a query link/alias view has already rendered before this canvas effect runs, its a.rm-page-ref elements are never passed to observePageRef: createHTMLObserver only covers subsequently added nodes, while the shared span path explicitly applies its handler to existing refs via onPageRefObserverChange. Consequently, opening or mounting a canvas alongside an already-rendered query leaves those page links without a drag handle until the query rerenders. Query the existing a.rm-page-ref elements here as well before relying on the observer.

Useful? React with 👍 / 👎.

@mdroidian

mdroidian commented Jul 19, 2026

Copy link
Copy Markdown
Member

@mdroidian Fixed all 3 points and updated the video to use latest code

Hey @sid597 , there is no mouse in the video so it's really hard to see that all three issues are solved. I think I made out the second (absolute positioning) but it only displays for a very brief moment.

Did you test the post-drag bug fix yourself?

Copy link
Copy Markdown
Member

@sid597 have you tried dragging an [[EVD]] or [[RES]]? How do you handle the multiple referenced pages?

@mdroidian mdroidian left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

https://www.loom.com/share/4c2add16b45b49b1b2f3e464f56b8394

Change requests:

  1. Match the background of the icon to the background of the window (white if main window, gray if sidebar)

  2. Move the handle farther right so it sits outside the page reference’s closing ]] brackets instead of overlapping them.

  3. Stabilize handle placement for multiline references.

  4. Do not render drag handles for nested references inside another reference, such as “EVD” or “Source” references embedded within a larger “Evidence” reference.

  5. Change the mouse pointer to grabbing when over the drag icon

A few more issues found
https://www.loom.com/share/00f51beb434349b99d4b1a8dcf661c0d

  • change: remove drag handles from titles
  • bug: drag handle moves and is persistent when mouse is removed from the left

After making these changes, please ensure the video records a comprehensive manual regression pass. Eg: demonstrating the drag behavior in the main window, sidebar, Kanban, and query link, embed, and render views. Include single-line, multiline, wrapping, and nested references in each applicable surface.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants