ENG-1373: Drag pages onto the canvas to create node shapes#1083
Conversation
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.
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 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".
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.
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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
left a comment
mdroidian
left a comment
There was a problem hiding this comment.
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
useEffectbehavior
|
|
||
| // Handle Roam block drag and drop | ||
| useEffect(() => { | ||
| const temporaryDraggableAttribute = "data-roamjs-canvas-page-ref-draggable"; |
| const pageMatch = text.match(PAGE_REF_REGEX); | ||
| if (pageMatch?.[1]) { | ||
| if (await tryCreatePageNodeShape(pageMatch[1])) { | ||
| posthog.capture("Canvas: Node Added from External Content", { |
There was a problem hiding this comment.
"External Content" here can be misleading. Does this mean outside of Roam?
|
|
||
| const refMatch = blockText.match(PAGE_REF_REGEX); | ||
| if (refMatch?.[1] && (await tryCreatePageNodeShape(refMatch[1]))) { | ||
| posthog.capture("Canvas: Node Added from External Content", { |
There was a problem hiding this comment.
"External Content" here can be misleading. Does this mean outside of Roam?
2 participants
https://www.loom.com/share/ab6520fa77ed4b10b045d5c8e39bf444