ENG-1553: Embed canvas in block#988
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Cast window.roamAlphaAPI.ui to access untyped slashCommand API with proper type annotation instead of relying on any. Fix void callback returning null.
- Move slash command out of registerCommandPaletteCommands into new registerSlashCommands.ts; wire teardown into index.ts unload - Replace hand-rolled InputGroup+Menu+keyboard logic in CanvasEmbedDialog with roamjs-components AutocompleteInput; add loading + empty states - Use data.async.fast.q (not the deprecated sync .q) for canvas page lookup - Extract canvas-page regex into shared helper in isCanvasPage.ts; escape regex special chars in user-provided canvasPageFormat - Convert inline styles to Tailwind in CanvasEmbed and CanvasEmbedDialog Pending follow-ups (not in this commit): - Upstream slashCommand type to roamjs-components so the `unknown` cast in registerSlashCommands.ts can be dropped - Dedupe getCanvasPageTargets (conditionToDatalog) and checkForCanvasPage (Export.tsx) onto the shared helper
PR size/scope checkThis PR is over our review-size guideline.
Please split this into smaller PRs unless there is a clear reason the changes need to land together. If keeping it as one PR, please add a brief justification covering:
|
Tailwind isn't compiled in the roam build (config has empty content, no @tailwind directives in styles/*.css, no PostCSS step in scripts/compile.ts), so the utility classes added in 08161b0 produced no CSS and the canvas embed wrapper collapsed to 0x0. Move the styles to dedicated classes in styles.css (.dg-canvas-embed, .dg-canvas-embed-placeholder, .dg-canvas-embed-dialog*, .dg-canvas-embed-source-hidden) to match the existing pattern in that file. Uses descendant selectors and proper specificity to override Blueprint dialog defaults and to size the AutocompleteInput inside the dialog -- neither possible from inline styles.
…-in-block # Conflicts: # apps/roam/src/index.ts # apps/roam/src/utils/initializeObserversAndListeners.ts # apps/roam/src/utils/isCanvasPage.ts
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
Moves the stopPropagation handler from button.parentElement to the wrapper div. Attaching to the parent overwrote any existing onmousedown Roam had on the block element and was never cleaned up; attaching to the wrapper (which renderWithUnmount owns) scopes the handler to the canvas and removes it on unmount.
The check guarded against a {{dg-canvas: [[X]]}} block living on canvas
page X. But canvas pages hide their block outline (tldrawStyles.ts:4-6),
so users can't author the block via the normal flow on a canvas page.
Removing the dead guard and the getCurrentPageTitle helper that only
served it.
|
@codex review |
![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: 01fc0cf4a4
ℹ️ 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".
| .dg-canvas-embed-source-hidden { | ||
| display: none; | ||
| } | ||
|
|
||
| .dg-canvas-embed { | ||
| height: 400px; | ||
| width: 100%; | ||
| overflow: hidden; | ||
| border-radius: 6px; | ||
| margin: 8px 0; | ||
| } | ||
|
|
||
| .dg-canvas-embed > .roamjs-tldraw-canvas-container { | ||
| height: 100%; | ||
| width: 100%; | ||
| } | ||
|
|
||
| .dg-canvas-embed-placeholder { | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| height: 100px; | ||
| color: #8a9ba8; | ||
| font-size: 14px; | ||
| border: 1px dashed #d1d5db; | ||
| border-radius: 6px; | ||
| } | ||
|
|
||
| .dg-canvas-embed-dialog.bp3-dialog { | ||
| width: 400px; | ||
| padding-bottom: 0; | ||
| } | ||
|
|
||
| .dg-canvas-embed-dialog-body { | ||
| padding: 16px; | ||
| } | ||
|
|
||
| .dg-canvas-embed-dialog-message { | ||
| color: #5c7080; | ||
| font-size: 14px; | ||
| } | ||
|
|
||
| .dg-canvas-embed-dialog .roamjs-autocomplete-input-target, | ||
| .dg-canvas-embed-dialog .roamjs-autocomplete-input-target .bp3-input-group { | ||
| display: block; | ||
| width: 100%; | ||
| } |
There was a problem hiding this comment.
How many of these could we move to inline tailwind classes?
There was a problem hiding this comment.
can move many of these, we need width: 400px; and height: 400px; only in the css because tailwind does not have any equivalent that gets compiled. Here we have all the related styles in style.css but in the new implementation we have both the inline style and in css,
There was a problem hiding this comment.
In the new implementation, if we reuse the existing roamjs-canvas-dialogwhich is used elsewhere in the code, what would need to be in css?
2 participants
https://www.loom.com/share/39b8760fe4094661a6f16f0fc764b4f5