Honor embed format and custom size in the compose editor #5548
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -278,6 +278,18 @@ export default class EmbedFormatSelection { | |
| } | ||
| } | ||
|
|
||
| // A Custom-size fitted embed must carry at least one explicit dimension — | ||
| // otherwise it serializes to the size-less bare `fitted`, which defeats the | ||
| // point of the Custom option. Every named format (atom/embedded/isolated and | ||
| // the fixed fitted variants) already carries its size, so it is always | ||
| // insertable; the consumer disables the insert CTA when this is false. | ||
| get hasValidSize(): boolean { | ||
| if (this.category !== 'custom') { | ||
| return true; | ||
| } | ||
| return this.width !== undefined || this.height !== undefined; | ||
| } | ||
|
Comment on lines
+286
to
+291
Contributor
There was a problem hiding this comment. [Claude Code 🤖] Editing an existing bare The path. Bare Scope: regression introduced here — before this guard the CTA was always enabled, so the edit round-trip worked. It's narrow (only bare A way out. The guard wants to block a new Custom insert left empty, not a seeded embed opened unchanged; dirtiness distinguishes the two — e.g. in private get ctaDisabled(): boolean {
return !this.args.selection.hasValidSize && this.args.selection.isDirty;
}An unchanged bare-fitted edit has Coverage. The new acceptance test covers the insert path (pick Custom → disabled → enter dims → enabled). The gap is the edit path: open the chooser on an existing Generated by Claude Code |
||
|
|
||
| // True once any format/placement/size field has diverged from the seeded | ||
| // snapshot. Target identity (the user swapped in a different card/file) is | ||
| // tracked separately by the tab panel, which owns the resolved target. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,20 +45,27 @@ import { | |
| } from '@codemirror/view'; | ||
|
|
||
| import { | ||
| type BfmRefFormat, | ||
| type BfmRefRange, | ||
| bfmRefFormatAndSize, | ||
| extractBfmRefRanges, | ||
| parseBfmSizeSpec, | ||
| } from '@cardstack/runtime-common/bfm-card-references'; | ||
|
|
||
| // ── Card widget target interface ──────────────────────────────────────────── | ||
|
|
||
| export interface CardWidgetTarget { | ||
| element: HTMLElement; | ||
| cardId: string; | ||
| format: BfmRefFormat; | ||
| kind: 'inline' | 'block'; | ||
| // 'card' refs (`:card[URL]`) resolve to CardDef instances; 'file' refs | ||
| // (`:file[URL]`) resolve to FileDef instances. | ||
| refType: 'card' | 'file'; | ||
| // Inline sizing (`width`/`height`, plus `overflow: hidden` for fitted) derived | ||
| // from the directive's size specifier. Undefined for non-fitted formats. | ||
| // Mirrors the style the saved/preview markdown renderers apply. | ||
| style?: string; | ||
| } | ||
|
|
||
| // ── State effect for opening card search ──────────────────────────────────── | ||
|
|
@@ -140,6 +147,12 @@ class CardWidget extends WidgetType { | |
| readonly cardId: string, | ||
| readonly kind: 'inline' | 'block', | ||
| readonly refType: 'card' | 'file' = 'card', | ||
| // Size specifier fields parsed from the directive's `| spec` segment. | ||
| // `width`/`height` stay strings for a clean DOM data-attr round-trip; | ||
| // `format` is the parsed render format. Undefined when absent. | ||
| readonly format?: BfmRefFormat, | ||
| readonly width?: string, | ||
| readonly height?: string, | ||
| ) { | ||
| super(); | ||
| } | ||
|
|
@@ -148,7 +161,12 @@ class CardWidget extends WidgetType { | |
| return ( | ||
| this.cardId === other.cardId && | ||
| this.kind === other.kind && | ||
| this.refType === other.refType && | ||
| // A size-only edit (e.g. `w:400` → `w:600`) must invalidate the widget so | ||
| // CM6 rebuilds the DOM with fresh data-attrs instead of reusing the old one. | ||
| this.format === other.format && | ||
| this.width === other.width && | ||
| this.height === other.height | ||
| ); | ||
| } | ||
|
|
||
|
|
@@ -158,6 +176,17 @@ class CardWidget extends WidgetType { | |
| el.setAttribute('data-card-id', this.cardId); | ||
| el.setAttribute('data-card-kind', this.kind); | ||
| el.setAttribute('data-bfm-ref-type', this.refType); | ||
| // Emit the size spec so `notifyTargets` can re-derive format + style off the | ||
| // DOM, using the same attribute names as the render-side BFM markup. | ||
| if (this.format !== undefined) { | ||
| el.setAttribute('data-boxel-bfm-format', this.format); | ||
| } | ||
| if (this.width !== undefined) { | ||
| el.setAttribute('data-boxel-bfm-width', this.width); | ||
| } | ||
| if (this.height !== undefined) { | ||
| el.setAttribute('data-boxel-bfm-height', this.height); | ||
| } | ||
| el.className = `cm-card-widget cm-card-widget--${this.kind}`; | ||
| el.contentEditable = 'false'; | ||
| return el; | ||
|
|
@@ -639,6 +668,34 @@ function buildLinkDecorations( | |
|
|
||
| // ── Card decorations ─────────────────────────────────────────────────────── | ||
|
|
||
| // Parse a directive's inner content (between `[` and `]`) into the URL/id and | ||
| // the CardWidget size args. `width`/`height` are kept as strings so they | ||
| // round-trip cleanly through DOM data-attrs, where `notifyTargets` re-derives | ||
| // format + style. | ||
| function parseCardWidgetContent(content: string): { | ||
| cardId: string; | ||
| format?: BfmRefFormat; | ||
| width?: string; | ||
| height?: string; | ||
| } { | ||
| let trimmed = content.trim(); | ||
| let pipeIdx = trimmed.indexOf('|'); | ||
| if (pipeIdx < 0) { | ||
| return { cardId: trimmed }; | ||
| } | ||
| let cardId = trimmed.substring(0, pipeIdx).trim(); | ||
| let spec = parseBfmSizeSpec(trimmed.substring(pipeIdx + 1).trim()); | ||
| if (!spec) { | ||
| return { cardId }; | ||
| } | ||
| return { | ||
| cardId, | ||
| format: spec.format, | ||
| width: spec.width !== undefined ? String(spec.width) : undefined, | ||
| height: spec.height !== undefined ? String(spec.height) : undefined, | ||
| }; | ||
| } | ||
|
|
||
| function buildCardDecorations( | ||
| state: EditorState, | ||
| cursorLine: number, | ||
|
|
@@ -657,11 +714,7 @@ function buildCardDecorations( | |
| let to = from + match[0].length; | ||
| if (isInsideCode(state, from, to)) continue; | ||
|
|
||
| let { cardId, format, width, height } = parseCardWidgetContent(match[1]); | ||
|
|
||
| let line = doc.lineAt(from); | ||
| let onCursor = livePreview && line.number === cursorLine; | ||
|
|
@@ -684,15 +737,29 @@ function buildCardDecorations( | |
| class: 'cm-bfm-card-ref cm-bfm-card-ref--inline', | ||
| }), | ||
| }); | ||
| let previewWidget = new CardWidget( | ||
| cardId, | ||
| 'block', | ||
| refType, | ||
| format, | ||
| width, | ||
| height, | ||
| ); | ||
| decos.push({ | ||
| from: to, | ||
| to: to, | ||
| value: Decoration.widget({ widget: previewWidget, side: 1 }), | ||
| }); | ||
| } else { | ||
| // Replace source text with widget | ||
| let widget = new CardWidget( | ||
| cardId, | ||
| 'block', | ||
| refType, | ||
| format, | ||
| width, | ||
| height, | ||
| ); | ||
| decos.push({ | ||
| from, | ||
| to, | ||
|
|
@@ -708,7 +775,7 @@ function buildCardDecorations( | |
| let to = from + match[0].length; | ||
| if (isInsideCode(state, from, to)) continue; | ||
|
|
||
| let { cardId, format, width, height } = parseCardWidgetContent(match[1]); | ||
| let onCursor = livePreview && isOnCursorLine(state, from, cursorLine); | ||
|
|
||
| if (!livePreview) { | ||
|
|
@@ -729,15 +796,29 @@ function buildCardDecorations( | |
| class: 'cm-bfm-card-ref cm-bfm-card-ref--inline', | ||
| }), | ||
| }); | ||
| let previewWidget = new CardWidget( | ||
| cardId, | ||
| 'inline', | ||
| refType, | ||
| format, | ||
| width, | ||
| height, | ||
| ); | ||
| decos.push({ | ||
| from: to, | ||
| to: to, | ||
| value: Decoration.widget({ widget: previewWidget, side: 1 }), | ||
| }); | ||
| } else { | ||
| // Replace source text with inline widget | ||
| let widget = new CardWidget( | ||
| cardId, | ||
| 'inline', | ||
| refType, | ||
| format, | ||
| width, | ||
| height, | ||
| ); | ||
| decos.push({ | ||
| from, | ||
| to, | ||
|
|
@@ -851,12 +932,30 @@ function createCardTargetNotifier( | |
| (el.getAttribute('data-bfm-ref-type') as 'card' | 'file') ?? | ||
| 'card'; | ||
| if (cardId) { | ||
| // Re-derive format + sizing from the directive's size attributes, | ||
| // matching the saved/preview markdown renderers. Inline embeds | ||
| // default to atom, block embeds to embedded. | ||
| let { format, sizeStyle } = bfmRefFormatAndSize( | ||
| el.getAttribute('data-boxel-bfm-format') ?? undefined, | ||
| el.getAttribute('data-boxel-bfm-width') ?? undefined, | ||
| el.getAttribute('data-boxel-bfm-height') ?? undefined, | ||
| kind === 'inline' ? 'atom' : 'embedded', | ||
| ); | ||
| // Fitted slots carry the width/height plus `overflow: hidden` so | ||
| // the resolved instance occupies the requested footprint. | ||
| let style = | ||
| format === 'fitted' | ||
| ? sizeStyle | ||
| ? `${sizeStyle}; overflow: hidden` | ||
| : 'overflow: hidden' | ||
| : undefined; | ||
|
Comment on lines
+946
to
+951
Contributor
There was a problem hiding this comment. [Claude Code 🤖] Confirming this is correct, and documenting the invariant it now depends on. This block is the compose-editor twin of the saved/preview renderer in Also confirming Generated by Claude Code |
||
| targets.push({ | ||
| element: el as HTMLElement, | ||
| cardId, | ||
| format, | ||
| kind, | ||
| refType, | ||
| style, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the chooser opens in edit mode for an existing
::card[url | fitted]directive,deriveFormatSeedstreats that valid bare fitted spec ascategory === 'custom'with empty width/height, so this returnsfalseand the pane disables the Done/Accept button. The markdown parser/renderer explicitly supports barefittedwith no size attributes, so users editing existing/manual content in that format cannot accept the unchanged embed unless they add an arbitrary dimension; please only block blank Custom for new inserts or preserve seeded bare-fitted edit state as valid.Useful? React with 👍 / 👎.