Render the broken-ref visual in the markdown-embed chooser#5444
Render the broken-ref visual in the markdown-embed chooser#5444FadhlanR wants to merge 8 commits into
Conversation
A `:card[...]` / `:file[...]` reference that no longer resolves (deleted,
moved, no permission) showed the same empty "search & preview" placeholder as
a not-yet-picked slot: the chooser collapsed "still loading" and "resolution
failed" into one undefined target. The chooser preview pane and the Edit modal
now render the rich broken-ref visual that broken linksTo / linksToMany field
values already use — the placeholder box with the type/file label plus the
reveal overlay carrying the URL, copy button, and error diagnostics.
To make that component reachable from host with a plain static import (rather
than a runtime module load), relocate `BrokenLinkTemplate` from base to
`@cardstack/boxel-ui`, the shared component package both base and host already
consume. Its three outside dependencies are decoupled so boxel-ui takes on no
dependency on runtime-common (which would invert the existing
runtime-common -> boxel-ui edge into a cycle):
- `cardTypeName` -> a caller-supplied `@typeName` arg (base card sites pass the
card type name; the file chooser passes the filename). Falls back to 'Card'.
- `SerializedError` -> a local `BrokenLinkErrorDoc` interface listing only the
fields the overlay reads (SerializedError is a structural superset).
- base `ViewCardFn` -> a local `BrokenLinkViewFn`.
The five base call sites switch to the boxel-ui import and pass
`@typeName={{cardTypeName reference}}`, reproducing the label the component
previously computed internally — no visual change to the shipped linksTo UI.
In the chooser, tab-panel threads a broken state (url / state / type name /
error doc) through the pane into MarkdownEmbedPreview, which renders
BrokenLinkTemplate in place of the embed. The Edit modal's existing
Remove / Replace tile is the fix/remove affordance; there is no "Open anyway"
here. The inline document render (rendered-markdown.gts) is unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Every boxel-ui component ships a usage.gts consumed by the component explorer; add one for the relocated BrokenLinkTemplate (interactive controls for the state, format, type name, errorDoc fields, and the optional viewCard handler) and regenerate the auto-generated usage registry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A fitted broken ref rendered at the template's 40px min-height because the chooser passed no dimensions — the box collapsed to a thin bar instead of the tile the real fitted embed previews. Thread the selected width/height onto the broken template (reusing the same bfmRefFormatAndSize the embed uses), so the fitted broken placeholder occupies the picked footprint. The size string omits `overflow: hidden` on purpose: the broken template's root must not clip, or the reveal overlay would be cut off — the inner box does its own clipping. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
In edit mode the tile above Replace / Remove falls back to the raw reference URL when there's no title to show (a broken ref, or a card without a title). When that reference lives in the realm the user currently has open, collapse it to its realm-relative path — a shorter, less noisy label. References outside the current realm keep their absolute URL. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The broken-ref overlay headline is now driven by a `@noun` arg on
BrokenLinkTemplate ("Linked card not found" vs "Linked file not found"),
defaulting to 'card' so the linksTo/linksToMany field visuals are
unchanged. The markdown-embed chooser threads 'file' for `:file[...]`
refs so a broken file no longer reads as a card.
Tighten the chooser label test to assert the exact realm-relative path
for an in-realm ref, and add a case for an out-of-realm ref keeping its
absolute URL.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![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: 61da84f011
ℹ️ 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".
| @sizeStyle={{this.sizeStyle}} | ||
| /> | ||
| {{else if @brokenUrl}} | ||
| <BrokenLinkTemplate |
There was a problem hiding this comment.
When the chooser is editing/preloading a broken inline ref with a non-atom format, this branch renders BrokenLinkTemplate directly instead of going through the Embed wrapper that receives @kind. BrokenLinkTemplate roots as a block-level placeholder for embedded/fitted/isolated formats, so an inline directive such as :card[url | embedded] is previewed as a block even though the CTA preserves it as inline; mirror the inline wrapper/classes used for resolved embeds so broken refs preview the same placement they will serialize.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
[Claude Code 🤖] Fixed in 013c0c5. Broken refs now render through a BrokenEmbed placement wrapper that mirrors the resolved-embed inline slot, so an inline non-atom broken ref previews inline-block (matching how it serializes) instead of a block. The wrapper adds no card-frame and never sets overflow: hidden, so the reveal overlay stays unclipped. Added a preview test covering the inline placement.
Thread the editing document's own URL (the codemirror editor's cardReferenceBaseUrl) through the chooser — dispatch, service, modal, tab-panel, and pane — so a picked or edited ref serializes as a `../`-relative directive, matching the format-picker insertion path and the `../`-relative storage convention. The current-target tile label relativizes against the same document URL rather than the realm root, so the fallback URL label reads as the same form that gets inserted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR makes unresolved :card[...] / :file[...] references in the markdown-embed chooser visually distinct from an “empty/not-yet-picked” slot by rendering the shared broken-reference UI (the same treatment used by linksTo / linksToMany). It also adds support for relativizing inserted BFM refs against the current document URL, aligning chooser insertion with the format-picker behavior.
Changes:
- Relocate/re-expose
BrokenLinkTemplatevia@cardstack/boxel-uiand update base + host call sites to pass the new caller-supplied@typeName(and optional noun). - Teach the host markdown-embed chooser to preserve and render a “broken ref” state (url + error payload) instead of collapsing it into the empty placeholder.
- Add
documentBaseUrlplumbing so chooser insertion (and labels) can emit../-relative BFM refs where applicable, with updated integration/acceptance coverage.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/runtime-common/bfm-card-references.ts | Extends chooser bridge types/options to include documentBaseUrl. |
| packages/host/app/services/markdown-embed-chooser.ts | Threads documentBaseUrl through chooser requests (choose + edit). |
| packages/host/app/components/markdown-embed-chooser/modal.gts | Passes documentBaseUrl to both tabs so labels/insertion can be relativized. |
| packages/host/app/components/markdown-embed-chooser/tab-panel.gts | Tracks selectedError to distinguish broken refs from “nothing selected” and supplies broken-ref args to the pane. |
| packages/host/app/components/markdown-embed-chooser/pane.gts | Serializes inserted BFM refs relative to documentBaseUrl; supports broken-ref preview + insertion. |
| packages/host/app/components/markdown-embed-chooser/preview/index.gts | Renders BrokenLinkTemplate when brokenUrl is present and no resolved target exists; adds fitted sizing for broken placeholders. |
| packages/base/codemirror-editor.gts | Supplies documentBaseUrl into chooser calls from the editor context. |
| packages/base/card-api.gts | Switches broken-link rendering to the boxel-ui component and passes caller-derived @typeName. |
| packages/base/links-to-editor.gts | Uses boxel-ui BrokenLinkTemplate and passes @typeName. |
| packages/base/links-to-many-component.gts | Uses boxel-ui BrokenLinkTemplate and passes @typeName at all broken-link render sites. |
| packages/boxel-ui/addon/src/components/broken-link/index.gts | Refactors broken-link component to remove runtime-common/base dependencies; adds @typeName/@noun and local error/view types. |
| packages/boxel-ui/addon/src/components.ts | Re-exports BrokenLinkTemplate and its public types from boxel-ui. |
| packages/boxel-ui/addon/src/usage.ts | Registers BrokenLink usage panel in freestyle usage index. |
| packages/boxel-ui/addon/src/components/broken-link/usage.gts | Adds a freestyle usage page for the broken-link component. |
| packages/boxel-ui/test-app/tests/integration/components/broken-link-test.gts | New test-app coverage for broken-link rendering and overlay behaviors. |
| packages/host/tests/integration/components/broken-link-template-test.gts | Updates host integration test to import the boxel-ui component and pass the new @typeName. |
| packages/host/tests/integration/components/markdown-embed-preview-test.gts | Adds coverage for broken-ref preview rendering (404 vs error, card vs file noun/type label, fitted sizing). |
| packages/host/tests/integration/components/markdown-embed-preview-pane-test.gts | Adds coverage for relative ref insertion and broken-ref preview + CTA serialization. |
| packages/host/tests/integration/components/markdown-embed-chooser-modal-test.gts | Adds edit-mode broken preload coverage + relative insertion result coverage. |
| packages/host/tests/acceptance/markdown-embed-chooser-test.gts | Updates acceptance expectation to the new document-relative inserted ref form. |
| packages/host/tests/integration/realm-indexing-test.gts | Updates dependency snapshots to remove the now-relocated base broken-link template module entry. |
Comments suppressed due to low confidence (1)
packages/boxel-ui/addon/src/components/broken-link/index.gts:50
nounis documented/used as an enum-like value ('card' | 'file') but is typed asstring, which loses type safety/autocomplete and allows unexpected values to flow into user-visible copy (headline strings). Consider narrowing it to a union type (and updating any callers/types accordingly).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // The broken template requires a state + error doc; default them so the arg | ||
| // types stay optional at this boundary (the parent only sets them alongside | ||
| // `brokenUrl`, but the correlation isn't expressible in the type). | ||
| private get brokenState(): BrokenLinkState { | ||
| return this.args.brokenState ?? 'error'; | ||
| } | ||
|
|
||
| private get brokenErrorDoc(): BrokenLinkErrorDoc { | ||
| return this.args.errorDoc ?? {}; | ||
| } |
There was a problem hiding this comment.
[Claude Code 🤖] Done in 013c0c5 — the fallback returns a module-level frozen EMPTY_ERROR_DOC constant instead of a fresh {} each render.
burieberry
left a comment
burieberry
left a comment
There was a problem hiding this comment.
I made a couple comments. Other than those, can you make sure all css properties properly match the designs: https://app.zeplin.io/project/6a1f141256365a9e08e74147/screen/6a1f14c4aadcc7b409bc2366
I also recommend using our context button for the x on the overlay.
| // context. | ||
| class WarningIcon extends GlimmerComponent<{ | ||
| Element: SVGElement; | ||
| const WarningIcon: TOC<{ |
There was a problem hiding this comment.
can you get a version of this where the fill and stroke properties are already in the svg? Then put it through image optimizer or compress it, and place it in icons in boxel-ui raw icons
There was a problem hiding this comment.
[Claude Code 🤖] Done in 013c0c5. Added raw-icons/warning-triangle-filled.svg with fill/stroke baked in per-path (SVGO drops a root fill, so the colour lives on each path), ran it through the icon pipeline (pnpm rebuild:icons → SVGO + committed src/icons/warning-triangle-filled.gts), and the component imports that instead of the inline SVG. The triangle uses the design amber #ffa515.
| @tracked typeName = 'Author'; | ||
| @tracked noun = 'card'; |
There was a problem hiding this comment.
noun is a very vague property name. how about calling noun -> itemType and calling typeName -> displayName? If there's no displayName, you can fall back to the capitalized itemType. If there's any way of knowing the itemType from the payload, we could use that too
There was a problem hiding this comment.
[Claude Code 🤖] Renamed in 013c0c5: noun→itemType (now typed 'card' | 'file') and typeName→displayName. When displayName is absent the label falls back to the capitalized itemType ("Card" / "File"). itemType is caller-supplied from the payload — linksTo sites pass 'card'; the BFM chooser derives card-vs-file from the ref type.
- Rename BrokenLink args: noun→itemType ('card'|'file' union) and
typeName→displayName, with the label falling back to the capitalized
itemType.
- Move the warning triangle to a baked-fill boxel-ui raw icon
(warning-triangle-filled) instead of an inline two-tone SVG.
- Use ContextButton (@ICON='close') for the reveal-overlay close control.
- Align the broken-ref CSS to the design: 5px radii on the atom pill and
error boxes, --boxel-border-color hairlines.
- Preview an inline broken ref inline (not as a block) via a placement
wrapper mirroring the resolved-embed inline slot.
- Reuse a stable empty errorDoc object in the preview instead of a fresh {}.
- Resolve document-relative refs to their absolute URL before loading the
edit-mode preview, so the pencil-opened chooser renders the card instead
of a blank pane.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
[Claude Code 🤖] Thanks for the review — all addressed in 013c0c5: CSS vs the designs — went through the broken-ref styling against the Zeplin screen. Most of it already used Context button — the overlay × is now Scope note — the full V2 error panel (scroll / copy-link / placement fallbacks) is a much larger feature than this PR; happy to file a follow-up ticket for it rather than pull it in here. While verifying the pencil-edit flow live I also found and fixed two bugs (same root cause): editing a card ref opened the chooser with a blank preview and threw a Glimmer backtracking assertion. The document-relative ref ( Screen.Recording.2026-07-13.at.13.43.44.mov |
…ering # Conflicts: # packages/base/codemirror-editor.gts
Linear: CS-11678: Broken card / broken file rendering
Problem
A
:card[...]/:file[...]reference that no longer resolves (deleted, moved, no permission) rendered the same empty "Search for a … & preview its format here" placeholder as a not-yet-picked slot. The chooser'sloadTargetcollapsed "still loading / nothing picked" and "resolution failed" into oneselectedTarget = undefined, so a broken ref was indistinguishable from an empty pane — in the chooser preview and in the Edit modal.Change
The chooser preview pane and the Edit modal now render the same rich broken-ref visual that broken
linksTo/linksToManyfield values already use: the placeholder box with a type/file label plus the reveal overlay carrying the URL, a copy button, and error diagnostics.To reach that component from host with a plain static import (rather than a runtime module load),
BrokenLinkTemplateis relocated frombaseto@cardstack/boxel-ui— the shared component package both base and host already consume, and whose scoped CSS is already shipped in the host bundle. Its three outside dependencies are decoupled so boxel-ui takes on no dependency onruntime-common(which would invert the existingruntime-common → boxel-uiedge into a cycle):cardTypeName→ a caller-supplied@typeNamearg (base card sites pass the card type name; the file chooser passes the filename; falls back to'Card').SerializedError→ a localBrokenLinkErrorDocinterface listing only the fields the overlay reads (SerializedErroris a structural superset, so base callers pass one unchanged).ViewCardFn→ a localBrokenLinkViewFn.base (visual parity, no behavior change)
The five call sites (
card-api,links-to-editor,links-to-many-component) switch to the boxel-ui import and pass@typeName={{cardTypeName reference}}— reproducing the label the component previously computed internally, so the shippedlinksTo/linksToManybroken UI is unchanged.host chooser (the ticket)
tab-paneltracks aselectedError, mounts the pane on target-or-error, and threads a broken state (url / state / type name / error doc) throughpaneintoMarkdownEmbedPreview, which rendersBrokenLinkTemplatein place of the embed. The Edit modal's existing Remove / Replace tile is the fix/remove affordance; there is no "Open anyway" in the chooser. The inline document render (rendered-markdown.gts) is left as-is.Tests
@typeName, reveal overlay + copy, not-found vs error) — passing locally (4/4).broken-link-template-testre-pointed at the boxel-ui import +@typeName.markdown-embed-preview(404 + error, card vs file label, no resolved embed),markdown-embed-preview-pane(broken preview surfaces; CTA still serializes the URL),markdown-embed-chooser-modal(Edit-mode broken preload shows the broken preview + Remove/Replace, no empty placeholder).realm-indexing-testdep snapshots: dropped the two now-stale@cardstack/base/default-templates/broken-link-templatelines (thelink-officon stays — still reachable via the boxel-ui bundle).linksto-broken-link-placeholder-test/linksto-many-broken-link-placeholder-testare the regression guard for the shippedlinksToUI (unchanged type-name value).Verified locally: glint/type-check clean (boxel-ui addon, host, test-app), eslint + template-lint clean, boxel-ui test-app suite green. Host integration tests rely on CI (they need the realm-server dev stack).