Repository navigation
feat: composed template preview foundation (#619, #620, #621, #622) - #654
Merged
ViewFromTheBox merged 7 commits intoAug 3, 2026
Merged
Conversation
📝 WalkthroughWalkthroughAdds an applied-template API and client hook, composes hydrated template chrome around the editor canvas, adds a view-mode toggle, rewrites block namespaces for editor compatibility, and prevents recursive template-part cycles. ChangesApplied template backend
Block compatibility
Composed-view client
Canvas composition
Editor controls
Template-part rendering
Estimated code review effort: 5 (Critical) | ~90 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (3)
resources/js/visual-editor/editor/composed-view/compose.ts (1)
188-198: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStatic
clientIdon the fallback content-slot host risks collisions in the block-editor store.
wrapAsContentSlotalways assigns the same literalclientId: '__composed_view_content_slot__'and bypasses typing viaas unknown as BlockInstance.clientIdis the primary key WordPress's block-editor Redux store uses to identify blocks; reusing a fixed id across renders/documents (rather than generating a fresh one per invocation) is fragile if this composed tree is ever registered with the store rather than only rendered read-only.♻️ Proposed fix
+import { createBlock } from '`@wordpress/blocks`'; + function wrapAsContentSlot(contentBlocks: readonly BlockInstance[]): BlockInstance { - return { - clientId: '__composed_view_content_slot__', - name: 'core/group', - isValid: true, - attributes: { - [COMPOSED_CONTENT_SLOT_MARKER]: true, - }, - innerBlocks: [...contentBlocks], - } as unknown as BlockInstance; + const group = createBlock('core/group', { + [COMPOSED_CONTENT_SLOT_MARKER]: true, + }, contentBlocks); + + return group; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resources/js/visual-editor/editor/composed-view/compose.ts` around lines 188 - 198, Update wrapAsContentSlot to generate a fresh unique clientId for each invocation instead of using the constant '__composed_view_content_slot__', while preserving the existing content-slot marker and innerBlocks behavior.resources/js/visual-editor/editor/composed-view/__tests__/compose.test.ts (1)
1-181: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for cyclic template-part refs and for a template with both
post-titleandpost-content.The suite doesn't exercise a self-/mutually-referencing
core/template-part(which would currently stack-overflow per theexpandTemplatePartscomment oncompose.ts), nor a template containing bothcore/post-titleandcore/post-contenttogether (which would currently mis-slot content into the title block, per theCONTENT_SLOT_BLOCK_NAMEScomment oncompose.ts). Worth adding once those fixes land.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resources/js/visual-editor/editor/composed-view/__tests__/compose.test.ts` around lines 1 - 181, Extend the composeBlocks test suite with coverage for cyclic core/template-part references, including self- or mutually-referencing parts, and assert expansion terminates without stack overflow while preserving the expected unresolved/locked behavior. Add a template containing both core/post-title and core/post-content, and assert content is slotted into core/post-content rather than core/post-title.resources/js/visual-editor/editor/composed-view/use-applied-template.ts (1)
67-82: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCached
errorstate is never retried.Once a fetch fails,
{status: 'error', ...}is written tocacheRef.currentfor that key, and the effect's cache-hit branch (Lines 90-94) will replay it forever for that(resource, id)— there's no way for the user to retry after a transient failure without a full remount.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resources/js/visual-editor/editor/composed-view/use-applied-template.ts` around lines 67 - 82, Update the error handling in the applied-template effect so a cached `{ status: 'error' }` result does not permanently short-circuit future loads for the same resource and id. Adjust the cache-hit logic around the applied-template state effect to retry fetching when the cached state is an error, while preserving reuse of successful or loading states.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@resources/js/visual-editor/editor/composed-view/__tests__/bindings.test.ts`:
- Around line 44-70: Update the test named “preserves a bindings map on
core/post-title in the template chrome” to use a real chrome block outside
CONTENT_SLOT_BLOCK_NAMES, such as core/post-author, so composeBlocks exercises
the chrome path rather than content-slot replacement. Keep the bindings
assertion and expected attributes aligned with the replacement block.
In `@resources/js/visual-editor/editor/composed-view/api.ts`:
- Around line 76-101: Update the successful-response path in the composed-view
API method to validate that the parsed body matches the expected AppliedTemplate
shape before returning `{ status: 'ok', template }`. Reject empty, primitive,
and otherwise malformed 2xx bodies by throwing ApiError with the response status
and body; preserve the existing 404 missing-payload handling and non-OK error
flow.
In `@resources/js/visual-editor/editor/composed-view/compose.ts`:
- Around line 93-127: Update expandTemplateParts to track the currently
expanding template-part slugs and detect self or mutual recursion before
descending into part.blocks. When a slug is already active, stop expanding that
reference and preserve it as an ordinary block; ensure the active slug is
removed after processing so separate non-cyclic references still expand
normally.
- Around line 44-48: Remove 'core/post-title' from CONTENT_SLOT_BLOCK_NAMES so
only 'core/post-content' is treated as the composed view’s content-slot host;
preserve the existing composed-view behavior and tests for the editable content
slot.
In `@resources/js/visual-editor/editor/composed-view/use-applied-template.ts`:
- Line 1: Update the cache handling in use-applied-template to account for
template changes: include the saved template in the cache key, or invalidate and
refetch the cached entry after handleTemplateChange completes. Ensure the
composed preview immediately uses the newly saved template without requiring a
remount.
- Around line 54-97: Add stale-request protection to the useAppliedTemplate
hook’s run/useEffect flow so responses from an earlier cacheKey cannot update
state or cache after resource or id changes. Track the active request key or
cancel the request, and guard both success and error completion paths before
calling setState or updating cacheRef; preserve the existing loading, caching,
and disabled behavior for the current key.
- Around line 45-53: The cache key in useAppliedTemplate must also reflect the
current page-template version or content, not only apiBase, resource, and id.
Thread a template or save-version key through the hook and include it in
cacheKey, or clear cacheRef after successful template saves, ensuring saved
template changes do not reuse stale composed-template state.
In `@resources/js/visual-editor/editor/editor-app.tsx`:
- Around line 834-839: Update the viewModeDisabledReason logic near the view
toggle to also handle appliedTemplateState.status values of missing and error
when viewMode is with-template. Provide an appropriate disabled reason or
existing banner treatment for those states, while preserving the current loading
behavior and preventing silent fallback to raw content.
- Around line 780-786: Update the useAppliedTemplate call in editor-app.tsx to
include the current template in its cache key or invalidate its cached result
whenever the assigned template changes, ensuring composedBlocks immediately
reflects the new template without requiring a remount.
---
Nitpick comments:
In `@resources/js/visual-editor/editor/composed-view/__tests__/compose.test.ts`:
- Around line 1-181: Extend the composeBlocks test suite with coverage for
cyclic core/template-part references, including self- or mutually-referencing
parts, and assert expansion terminates without stack overflow while preserving
the expected unresolved/locked behavior. Add a template containing both
core/post-title and core/post-content, and assert content is slotted into
core/post-content rather than core/post-title.
In `@resources/js/visual-editor/editor/composed-view/compose.ts`:
- Around line 188-198: Update wrapAsContentSlot to generate a fresh unique
clientId for each invocation instead of using the constant
'__composed_view_content_slot__', while preserving the existing content-slot
marker and innerBlocks behavior.
In `@resources/js/visual-editor/editor/composed-view/use-applied-template.ts`:
- Around line 67-82: Update the error handling in the applied-template effect so
a cached `{ status: 'error' }` result does not permanently short-circuit future
loads for the same resource and id. Adjust the cache-hit logic around the
applied-template state effect to retry fetching when the cached state is an
error, while preserving reuse of successful or loading states.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8d7ea086-1af4-44f9-bc2b-df95009957e8
📒 Files selected for processing (17)
database/migrations/testing/2026_07_14_000000_create_test_applied_template_table.phpresources/js/visual-editor/editor/__tests__/top-bar.test.tsxresources/js/visual-editor/editor/composed-view/__tests__/bindings.test.tsresources/js/visual-editor/editor/composed-view/__tests__/compose.test.tsresources/js/visual-editor/editor/composed-view/__tests__/use-applied-template.test.tsxresources/js/visual-editor/editor/composed-view/api.tsresources/js/visual-editor/editor/composed-view/compose.tsresources/js/visual-editor/editor/composed-view/extract.tsresources/js/visual-editor/editor/composed-view/index.tsresources/js/visual-editor/editor/composed-view/use-applied-template.tsresources/js/visual-editor/editor/editor-app.tsxresources/js/visual-editor/editor/top-bar.cssresources/js/visual-editor/editor/top-bar.tsxroutes/api.phpsrc/Http/Controllers/ResourceAppliedTemplateController.phptests/Feature/VisualEditor/ResourceAppliedTemplateControllerTest.phptests/Fixtures/TestAppliedTemplateModel.php
Comment on lines
+54
to
+97
| const run = useCallback(async (): Promise<void> => { | ||
| setState({ status: 'loading' }); | ||
|
|
||
| try { | ||
| const result = await fetchAppliedTemplate({ apiBase, resource, id }); | ||
|
|
||
| const next: AppliedTemplateState = | ||
| result.status === 'ok' | ||
| ? { status: 'ok', template: result.template } | ||
| : { status: 'missing', missing: result }; | ||
|
|
||
| cacheRef.current = { key: cacheKey, state: next }; | ||
| setState(next); | ||
| } catch (error: unknown) { | ||
| const normalized = | ||
| error instanceof ApiError | ||
| ? error | ||
| : error instanceof Error | ||
| ? error | ||
| : new Error('Failed to load applied template.'); | ||
|
|
||
| const next: AppliedTemplateState = { | ||
| status: 'error', | ||
| error: normalized, | ||
| }; | ||
|
|
||
| cacheRef.current = { key: cacheKey, state: next }; | ||
| setState(next); | ||
| } | ||
| }, [apiBase, cacheKey, id, resource]); | ||
|
|
||
| useEffect(() => { | ||
| if (!enabled) { | ||
| return; | ||
| } | ||
|
|
||
| if (cacheRef.current !== null && cacheRef.current.key === cacheKey) { | ||
| setState(cacheRef.current.state); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| void run(); | ||
| }, [cacheKey, enabled, run]); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file first
ast-grep outline resources/js/visual-editor/editor/composed-view/use-applied-template.ts --view expanded
# Read the relevant section with line numbers
cat -n resources/js/visual-editor/editor/composed-view/use-applied-template.ts | sed -n '1,220p'
# Search for related cache / fetch / cancellation patterns in nearby code
rg -n "fetchAppliedTemplate|cacheRef|cacheKey|AbortController|stale|useAppliedTemplate|handleTemplateChange" resources/js/visual-editor/editor -SRepository: ArtisanPack-UI/visual-editor
Length of output: 8703
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n resources/js/visual-editor/editor/composed-view/api.ts | sed -n '1,220p'Repository: ArtisanPack-UI/visual-editor
Length of output: 4035
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the hook tests
cat -n resources/js/visual-editor/editor/composed-view/__tests__/use-applied-template.test.tsx | sed -n '1,260p'
# Inspect the caller and template-change path
cat -n resources/js/visual-editor/editor/editor-app.tsx | sed -n '680,840p'
# Look for any explicit invalidation or cancellation around this hook
rg -n "useAppliedTemplate\(|cacheKey|cacheRef\.current|AbortController|invalidate|refresh|template change|handleTemplateChange" resources/js/visual-editor/editor -SRepository: ArtisanPack-UI/visual-editor
Length of output: 14872
Guard against stale applied-template responses
A slower in-flight request can still call setState after the hook has moved to a new (resource, id), briefly showing the old result and then forcing another fetch. Add a request-key guard or cancellation so outdated responses are ignored.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@resources/js/visual-editor/editor/composed-view/use-applied-template.ts`
around lines 54 - 97, Add stale-request protection to the useAppliedTemplate
hook’s run/useEffect flow so responses from an earlier cacheKey cannot update
state or cache after resource or id changes. Track the active request key or
cancel the request, and guard both success and error completion paths before
calling setState or updating cacheRef; preserve the existing loading, caching,
and disabled behavior for the current key.
9 tasks
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
resources/js/visual-editor/editor/editor-app.tsx (1)
786-791: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winInvalidate the applied-template cache when the page template changes.
useAppliedTemplateis keyed only byapiBase/resource/id, so changing the assigned template mid-session leaves the composed view rendering the old template until a remount occurs. Include the currenttemplatein the cache key.Update the options passed to the hook here, and ensure you also update the
useAppliedTemplatehook's signature and internalcacheKeyto include thetemplateparameter.💡 Proposed adjustment in editor-app
const appliedTemplateState = useAppliedTemplate({ apiBase: props.apiBase, resource: props.resource, id: props.id, + template, enabled: viewMode === 'with-template', });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resources/js/visual-editor/editor/editor-app.tsx` around lines 786 - 791, Update the useAppliedTemplate call in the editor component to pass the current template, then extend the hook signature and its internal cacheKey to include that template alongside apiBase, resource, and id, ensuring template changes invalidate the cached applied-template state.resources/js/visual-editor/editor/composed-view/api.ts (1)
100-104: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject empty or malformed 2xx bodies before returning
ok.The code still casts any
response.okpayload toAppliedTemplate. An empty or non-object success body will flow through as{ status: 'ok' }and throw when downstream code attempts to readtemplate.blocksortemplate.template_parts. Throw anApiErrorunless the body matches the expected shape.🛡️ Proposed fix to validate the payload
if (isMissingPayload(body)) { return body; } + if ( + typeof body !== 'object' || + body === null || + (body as Record<string, unknown>).status !== 'ok' + ) { + throw new ApiError( + 'Received invalid applied-template payload from server', + response.status, + body + ); + } + return body as AppliedTemplate;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resources/js/visual-editor/editor/composed-view/api.ts` around lines 100 - 104, Update the response-body handling around isMissingPayload so successful responses validate that body is a non-empty object matching the expected AppliedTemplate shape, including the fields consumed downstream such as blocks and template_parts. Throw an ApiError for empty or malformed 2xx payloads instead of returning a blind cast; preserve the existing return behavior for valid payloads.
🧹 Nitpick comments (1)
resources/js/visual-editor/editor/editor-app.css (1)
226-261: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer lowercase
currentcolorkeyword.While browsers accept
currentColor, standard CSS value keywords are case-insensitive and conventionally written in all lowercase (currentcolor). This also resolves Stylelintvalue-keyword-casewarnings.💅 Proposed refactor
- border: 1px dashed color-mix(in oklab, currentColor 25%, transparent); + border: 1px dashed color-mix(in oklab, currentcolor 25%, transparent); border-radius: 6px; - background: color-mix(in oklab, currentColor 4%, transparent); + background: color-mix(in oklab, currentcolor 4%, transparent); font-size: 12px; - color: color-mix(in oklab, currentColor 70%, transparent); + color: color-mix(in oklab, currentcolor 70%, transparent); } .ap-visual-editor__chrome-preview-header { display: flex; justify-content: space-between; align-items: baseline; margin-bottom: 8px; font-weight: 600; text-transform: uppercase; letter-spacing: 0.05em; } .ap-visual-editor__chrome-preview-template { font-weight: 400; text-transform: none; letter-spacing: 0; opacity: 0.75; } .ap-visual-editor__chrome-preview-list { display: flex; flex-wrap: wrap; gap: 6px; margin: 0; padding: 0; list-style: none; } .ap-visual-editor__chrome-preview-item { padding: 2px 8px; - border: 1px solid color-mix(in oklab, currentColor 20%, transparent); + border: 1px solid color-mix(in oklab, currentcolor 20%, transparent);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resources/js/visual-editor/editor/editor-app.css` around lines 226 - 261, Update all currentColor value keywords in the affected visual editor CSS rules to lowercase currentcolor, including the declarations surrounding ap-visual-editor__chrome-preview-header and ap-visual-editor__chrome-preview-item, without changing their color-mix values or other styling.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@resources/js/visual-editor/editor/composed-view/split.ts`:
- Around line 60-84: Update expandParts to accept and propagate a visited set of
template-part slugs, checking the set before recursively expanding a referenced
part and stopping expansion when that slug is already being expanded. Add each
slug before descending and remove it after recursion so shared non-circular
parts still expand correctly while circular references terminate safely.
In `@resources/js/visual-editor/editor/editor-app.tsx`:
- Around line 1037-1063: Update the canvas-stack rendering around
composedChromePreview and the applied-template state to show a banner when the
applied template is missing or errored instead of silently presenting raw
content. Ensure the banner explains that template chrome is unavailable and
keeps the existing header/footer ChromePreviewPanel rendering for valid
previews.
---
Duplicate comments:
In `@resources/js/visual-editor/editor/composed-view/api.ts`:
- Around line 100-104: Update the response-body handling around isMissingPayload
so successful responses validate that body is a non-empty object matching the
expected AppliedTemplate shape, including the fields consumed downstream such as
blocks and template_parts. Throw an ApiError for empty or malformed 2xx payloads
instead of returning a blind cast; preserve the existing return behavior for
valid payloads.
In `@resources/js/visual-editor/editor/editor-app.tsx`:
- Around line 786-791: Update the useAppliedTemplate call in the editor
component to pass the current template, then extend the hook signature and its
internal cacheKey to include that template alongside apiBase, resource, and id,
ensuring template changes invalidate the cached applied-template state.
---
Nitpick comments:
In `@resources/js/visual-editor/editor/editor-app.css`:
- Around line 226-261: Update all currentColor value keywords in the affected
visual editor CSS rules to lowercase currentcolor, including the declarations
surrounding ap-visual-editor__chrome-preview-header and
ap-visual-editor__chrome-preview-item, without changing their color-mix values
or other styling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6056ce33-6eca-4277-b1b3-bebce1051bec
📒 Files selected for processing (12)
resources/js/visual-editor/editor/composed-view/ChromePreviewPanel.tsxresources/js/visual-editor/editor/composed-view/__tests__/use-applied-template.test.tsxresources/js/visual-editor/editor/composed-view/api.tsresources/js/visual-editor/editor/composed-view/compose.tsresources/js/visual-editor/editor/composed-view/hydrate.tsresources/js/visual-editor/editor/composed-view/index.tsresources/js/visual-editor/editor/composed-view/split.tsresources/js/visual-editor/editor/composed-view/use-applied-template.tsresources/js/visual-editor/editor/editor-app.cssresources/js/visual-editor/editor/editor-app.tsxsrc/Http/Controllers/ResourceAppliedTemplateController.phptests/Feature/VisualEditor/ResourceAppliedTemplateControllerTest.php
🚧 Files skipped from review as they are similar to previous changes (5)
- resources/js/visual-editor/editor/composed-view/index.ts
- resources/js/visual-editor/editor/composed-view/use-applied-template.ts
- resources/js/visual-editor/editor/composed-view/tests/use-applied-template.test.tsx
- tests/Feature/VisualEditor/ResourceAppliedTemplateControllerTest.php
- resources/js/visual-editor/editor/composed-view/compose.ts
This was referenced Jul 26, 2026
Introduces the "composed view" feature that lets authors preview and edit content inside its resolved template without leaving the editor: - #619 backend: adds GET {resource}/{id}/applied-template. Resolves the content model's `template` attribute through the shared H5 TemplateResolver, returns the referenced template-parts inline, and signals empty/unknown-slug misses as 404 with a discriminated body so the client can trigger the fallback flow. - #620 top bar: adds the Content ⇄ With Template toggle to top-bar.tsx as an aria switch, wires ephemeral view-mode state on editor-app.tsx, and disables the toggle while the applied-template fetch is in flight. Every editor mount starts in `content` mode. - #621 composed view module: new resources/js/visual-editor/editor/composed-view/ containing the applied-template REST client, a pure `composeBlocks` function that expands template-part refs and locks chrome blocks, an inverse `extractContentBlocks` for round-tripping persistence, and a `useAppliedTemplate` hook that caches per (resource, id) so toggling is instant after the first fetch. Wired into editor-app.tsx: swaps the BlockEditorProvider tree without remounting the iframe canvas and extracts the content region back before every save. - #622 bindings: documents that the existing setBindingsResourceContext call already covers template-chrome bindings consumers (post-title/author/date/featured-image resolve against the current content), adds an unmount-clear effect for cross-mount hygiene, and pins the behavior with vitest coverage showing bindings are preserved on chrome blocks under composition + template-part expansion. Server: 6 new Pest tests (happy path, empty template, unknown slug, whitespace-only template, missing part, unauthenticated). Client: 6 new top-bar toggle tests, 8 compose tests, 4 useAppliedTemplate tests, 4 bindings-preservation tests. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two related changes to unblock the composed-view MVP after dev-app testing
surfaced problems with the initial cut:
1. Endpoint switched from 404 → 200 for the "missing template" signal.
The 404 was semantically correct but showed up as a scary error every
time the toggle was clicked on a resource without a template. The
response body still carries the same {status, reason} discriminator;
moving to 200 keeps devtools quiet on the routine case. Client fetch +
hook + tests updated to read `status` from the body.
2. Reverted the BlockEditorProvider value-swap composition. Feeding the
provider a composed tree (chrome + slot) fought Gutenberg's stateful
sync in ways every fix uncovered a new failure:
- hand-shaped blocks tripped block-validation warnings on every chrome
block (fixed by adding a `hydrateAppliedTemplate` helper that runs
each block through `wp.blocks.createBlock`);
- swapping `value` triggered onChange echoes that looped through
extractContentBlocks → onBlocksChange → new composed tree → new value
until the editor froze (even with a reference-shallow guard);
- `core/post-title` was in the slot-candidate set but is a leaf block
that can't accept innerBlocks, deepening the freeze.
Pivoted to render the resolved template as inert chrome preview
panels above / below the block canvas (`ChromePreviewPanel.tsx` +
`split.ts`). The block canvas is unchanged — one BlockEditorProvider,
one tree, existing persistence loop untouched — so selection, undo
history, and unsaved-changes survive by construction. The panels show
the block names in the header/footer as a first-cut "shape" preview.
This is knowingly a placeholder for the "real" in-canvas chrome
rendering the umbrella issue #618 promised. Filed #655 to track the
dual-tree rewrite that will replace it.
Foundation kept:
- `hydrateAppliedTemplate` (composed-view/hydrate.ts) — still useful for
the eventual real-rendering path; not currently called from editor-app.
- `composeBlocks` + `extractContentBlocks` (compose.ts + extract.ts) —
same. All 16 composed-view vitest cases still pass.
Follows: #655
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Rebased onto main (1.5.5) and cleared the blocking CodeRabbit findings.
- Cycle protection in both template-part expanders. `expandParts`
(split.ts) and `expandTemplateParts` (compose.ts) recursed without
tracking what they had already inlined, so a self- or
mutually-referencing part blew the stack and took the composed view
with it. Template parts are theme/DB content and can't be assumed
acyclic — the backend already guards its own walk but still ships the
circular parts in the flat map it returns. The visited set is passed
by value per branch so sibling refs to a shared part still expand.
- Validate the applied-template payload before trusting it. A 2xx that
wasn't a recognized `missing` body was cast straight to
`AppliedTemplate`, so an empty or truncated response surfaced as a
crash inside `splitTemplateAroundContentSlot` rather than at the
fetch. Now throws `ApiError`. Also corrects `AppliedTemplateResult`,
which described an `{ status, template }` wrapper the endpoint never
returned.
- Key the applied-template cache on the selected template. It was keyed
on `(apiBase, resource, id)` only, so switching templates in the
document panel kept serving the previous composed preview until the
editor remounted. Refetching alone would have raced the debounced
save and re-cached the *old* template under the new key, so the slug
now rides along as `?template=` and the endpoint resolves what the
editor asked for instead of what happens to be persisted. A blank
value is meaningful (selection cleared) and is distinct from omitting
the parameter (use the stored value).
- Banner the missing/error states. Composed mode with an unresolvable
template rendered bare content under a toggle still reading "With
template", which read as the toggle doing nothing. This is the
review-fix treatment only; the designed fallback (compose against a
minimal default template + toast) remains #624.
Tests: +6 split (new file, the critical path had no coverage), +3
compose, +5 use-applied-template, +5 Pest for the override contract.
Full suites green — 2492 vitest, 1610 pest.
Refs #619 #622 #654
ViewFromTheBox
force-pushed
the
feature/619-composed-view-preview
branch
from
August 3, 2026 01:04
517eb80 to
528c14a
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
resources/js/visual-editor/editor/composed-view/use-applied-template.ts (1)
54-114: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGuard
run()against stale in-flight responses.
run()is recreated on everycacheKeychange, and each invocation callssetStateunconditionally on completion (Lines 82-83, 97-98). If the user changes the template selection while a previous fetch for an oldercacheKeyis still in flight, the older response can resolve after the newer request starts and overwrite the current display with stale data.Track the active
cacheKeyand ignore responses that no longer match it.🔒️ Proposed fix using a request-key guard
export function useAppliedTemplate( options: UseAppliedTemplateOptions ): AppliedTemplateState { const { apiBase, resource, id, template, enabled } = options; const [state, setState] = useState<AppliedTemplateState>({ status: 'idle' }); const cacheRef = useRef<{ key: string; state: AppliedTemplateState; } | null>(null); + const activeKeyRef = useRef<string | null>(null); const cacheKey = `${apiBase}::${resource}::${id}::${template ?? ''}`; const run = useCallback(async (): Promise<void> => { + activeKeyRef.current = cacheKey; setState({ status: 'loading' }); try { const result = await fetchAppliedTemplate({ apiBase, resource, id, template, }); const next: AppliedTemplateState = result.status === 'ok' ? { status: 'ok', template: result } : { status: 'missing', missing: result }; cacheRef.current = { key: cacheKey, state: next }; + if (activeKeyRef.current === cacheKey) { setState(next); + } } catch (error: unknown) { const normalized = /* ... */; const next: AppliedTemplateState = { status: 'error', error: normalized }; cacheRef.current = { key: cacheKey, state: next }; + if (activeKeyRef.current === cacheKey) { setState(next); + } } }, [apiBase, cacheKey, id, resource, template]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resources/js/visual-editor/editor/composed-view/use-applied-template.ts` around lines 54 - 114, Update useAppliedTemplate and its run callback to guard async completions with the cacheKey captured when the request starts. Before applying success or error results, verify the captured key still matches the current active cacheKey; ignore stale responses, including their cacheRef and setState updates, while preserving normal handling for the current request.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@resources/js/visual-editor/editor/composed-view/api.ts`:
- Around line 152-180: Extend isAppliedTemplatePayload to require record.source
to be exactly "db" or "theme", and validate every entry in the template_parts
map as an object containing a blocks array. Preserve the existing acceptance of
an empty array for template_parts, while rejecting malformed nested entries
before expandParts processes them.
In `@resources/js/visual-editor/editor/composed-view/ChromePreviewPanel.tsx`:
- Around line 56-63: Update the aside element in ChromePreviewPanel to include
the panel’s label in its accessible name, ensuring header and footer landmarks
are distinguishable; either incorporate label into aria-label or reference the
visible label span via aria-labelledby while preserving the existing localized
context.
In `@resources/js/visual-editor/editor/editor-app.css`:
- Line 226: Replace all four currentColor tokens in the affected border
declarations with lowercase currentcolor to satisfy Stylelint, including the
declarations near the referenced lines.
---
Duplicate comments:
In `@resources/js/visual-editor/editor/composed-view/use-applied-template.ts`:
- Around line 54-114: Update useAppliedTemplate and its run callback to guard
async completions with the cacheKey captured when the request starts. Before
applying success or error results, verify the captured key still matches the
current active cacheKey; ignore stale responses, including their cacheRef and
setState updates, while preserving normal handling for the current request.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ab796481-ed1d-4add-bb52-5d5ac003affd
📒 Files selected for processing (20)
database/migrations/testing/2026_07_14_000000_create_test_applied_template_table.phpresources/js/visual-editor/editor/__tests__/top-bar.test.tsxresources/js/visual-editor/editor/composed-view/ChromePreviewPanel.tsxresources/js/visual-editor/editor/composed-view/__tests__/bindings.test.tsresources/js/visual-editor/editor/composed-view/__tests__/compose.test.tsresources/js/visual-editor/editor/composed-view/__tests__/split.test.tsresources/js/visual-editor/editor/composed-view/__tests__/use-applied-template.test.tsxresources/js/visual-editor/editor/composed-view/api.tsresources/js/visual-editor/editor/composed-view/compose.tsresources/js/visual-editor/editor/composed-view/extract.tsresources/js/visual-editor/editor/composed-view/hydrate.tsresources/js/visual-editor/editor/composed-view/index.tsresources/js/visual-editor/editor/composed-view/split.tsresources/js/visual-editor/editor/composed-view/use-applied-template.tsresources/js/visual-editor/editor/editor-app.cssresources/js/visual-editor/editor/editor-app.tsxresources/js/visual-editor/editor/top-bar.cssresources/js/visual-editor/editor/top-bar.tsxroutes/api.phpsrc/Http/Controllers/ResourceAppliedTemplateController.php
🚧 Files skipped from review as they are similar to previous changes (11)
- resources/js/visual-editor/editor/top-bar.css
- resources/js/visual-editor/editor/tests/top-bar.test.tsx
- resources/js/visual-editor/editor/top-bar.tsx
- resources/js/visual-editor/editor/composed-view/index.ts
- resources/js/visual-editor/editor/composed-view/tests/bindings.test.ts
- database/migrations/testing/2026_07_14_000000_create_test_applied_template_table.php
- resources/js/visual-editor/editor/editor-app.tsx
- resources/js/visual-editor/editor/composed-view/hydrate.ts
- resources/js/visual-editor/editor/composed-view/compose.ts
- routes/api.php
- resources/js/visual-editor/editor/composed-view/extract.ts
Comment on lines
+152
to
+180
| function isAppliedTemplatePayload(body: unknown): body is AppliedTemplate { | ||
| if (body === null || typeof body !== 'object' || Array.isArray(body)) { | ||
| return false; | ||
| } | ||
|
|
||
| const record = body as Record<string, unknown>; | ||
|
|
||
| if (record.status !== 'ok') { | ||
| return false; | ||
| } | ||
|
|
||
| if (typeof record.slug !== 'string' || typeof record.name !== 'string') { | ||
| return false; | ||
| } | ||
|
|
||
| if (!Array.isArray(record.blocks)) { | ||
| return false; | ||
| } | ||
|
|
||
| // `template_parts` is a slug-keyed map. PHP serializes an empty map as | ||
| // `[]`, so an empty array is valid here — anything else non-object is not. | ||
| const parts = record.template_parts; | ||
|
|
||
| return ( | ||
| typeof parts === 'object' && | ||
| parts !== null && | ||
| (!Array.isArray(parts) || parts.length === 0) | ||
| ); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Extend the payload guard to cover source and nested template_parts entries.
isAppliedTemplatePayload checks slug, name, and blocks, but never validates record.source against 'db' | 'theme', and never validates that each entry inside template_parts has a blocks array. A malformed template_parts entry (missing blocks) passes this guard and later throws inside expandParts in split.ts, which is the exact "throws far from the actual cause" failure this guard was added to prevent for the top-level shape.
Validate the nested shape too.
🛡️ Proposed fix to validate `source` and nested parts
if (typeof record.slug !== 'string' || typeof record.name !== 'string') {
return false;
}
+ if (record.source !== 'db' && record.source !== 'theme') {
+ return false;
+ }
+
if (!Array.isArray(record.blocks)) {
return false;
}
// `template_parts` is a slug-keyed map. PHP serializes an empty map as
// `[]`, so an empty array is valid here — anything else non-object is not.
const parts = record.template_parts;
- return (
- typeof parts === 'object' &&
- parts !== null &&
- (!Array.isArray(parts) || parts.length === 0)
- );
+ if (
+ typeof parts !== 'object' ||
+ parts === null ||
+ (Array.isArray(parts) && parts.length !== 0)
+ ) {
+ return false;
+ }
+
+ return Object.values(parts as Record<string, unknown>).every(
+ (part) =>
+ typeof part === 'object' &&
+ part !== null &&
+ Array.isArray((part as { blocks?: unknown }).blocks)
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function isAppliedTemplatePayload(body: unknown): body is AppliedTemplate { | |
| if (body === null || typeof body !== 'object' || Array.isArray(body)) { | |
| return false; | |
| } | |
| const record = body as Record<string, unknown>; | |
| if (record.status !== 'ok') { | |
| return false; | |
| } | |
| if (typeof record.slug !== 'string' || typeof record.name !== 'string') { | |
| return false; | |
| } | |
| if (!Array.isArray(record.blocks)) { | |
| return false; | |
| } | |
| // `template_parts` is a slug-keyed map. PHP serializes an empty map as | |
| // `[]`, so an empty array is valid here — anything else non-object is not. | |
| const parts = record.template_parts; | |
| return ( | |
| typeof parts === 'object' && | |
| parts !== null && | |
| (!Array.isArray(parts) || parts.length === 0) | |
| ); | |
| } | |
| function isAppliedTemplatePayload(body: unknown): body is AppliedTemplate { | |
| if (body === null || typeof body !== 'object' || Array.isArray(body)) { | |
| return false; | |
| } | |
| const record = body as Record<string, unknown>; | |
| if (record.status !== 'ok') { | |
| return false; | |
| } | |
| if (typeof record.slug !== 'string' || typeof record.name !== 'string') { | |
| return false; | |
| } | |
| if (record.source !== 'db' && record.source !== 'theme') { | |
| return false; | |
| } | |
| if (!Array.isArray(record.blocks)) { | |
| return false; | |
| } | |
| // `template_parts` is a slug-keyed map. PHP serializes an empty map as | |
| // `[]`, so an empty array is valid here — anything else non-object is not. | |
| const parts = record.template_parts; | |
| if ( | |
| typeof parts !== 'object' || | |
| parts === null || | |
| (Array.isArray(parts) && parts.length !== 0) | |
| ) { | |
| return false; | |
| } | |
| return Object.values(parts as Record<string, unknown>).every( | |
| (part) => | |
| typeof part === 'object' && | |
| part !== null && | |
| Array.isArray((part as { blocks?: unknown }).blocks) | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@resources/js/visual-editor/editor/composed-view/api.ts` around lines 152 -
180, Extend isAppliedTemplatePayload to require record.source to be exactly "db"
or "theme", and validate every entry in the template_parts map as an object
containing a blocks array. Preserve the existing acceptance of an empty array
for template_parts, while rejecting malformed nested entries before expandParts
processes them.
Comment on lines
+56
to
+63
| <aside | ||
| className="ap-visual-editor__chrome-preview" | ||
| data-testid={`ap-visual-editor-chrome-preview-${label.toLowerCase()}`} | ||
| aria-label={__('Template chrome preview', TEXT_DOMAIN)} | ||
| > | ||
| <header className="ap-visual-editor__chrome-preview-header"> | ||
| <span className="ap-visual-editor__chrome-preview-label"> | ||
| {label} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Include the panel label in the accessible name.
Each <aside> uses the same aria-label, so header and footer landmarks are indistinguishable to screen-reader landmark navigation. Build the accessible name from label, or reference the visible label with aria-labelledby.
Proposed fix
- aria-label={__('Template chrome preview', TEXT_DOMAIN)}
+ aria-label={`${label}: ${__('Template chrome preview', TEXT_DOMAIN)}`}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <aside | |
| className="ap-visual-editor__chrome-preview" | |
| data-testid={`ap-visual-editor-chrome-preview-${label.toLowerCase()}`} | |
| aria-label={__('Template chrome preview', TEXT_DOMAIN)} | |
| > | |
| <header className="ap-visual-editor__chrome-preview-header"> | |
| <span className="ap-visual-editor__chrome-preview-label"> | |
| {label} | |
| <aside | |
| className="ap-visual-editor__chrome-preview" | |
| data-testid={`ap-visual-editor-chrome-preview-${label.toLowerCase()}`} | |
| aria-label={`${label}: ${__('Template chrome preview', TEXT_DOMAIN)}`} | |
| > | |
| <header className="ap-visual-editor__chrome-preview-header"> | |
| <span className="ap-visual-editor__chrome-preview-label"> | |
| {label} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@resources/js/visual-editor/editor/composed-view/ChromePreviewPanel.tsx`
around lines 56 - 63, Update the aside element in ChromePreviewPanel to include
the panel’s label in its accessible name, ensuring header and footer landmarks
are distinguishable; either incorporate label into aria-label or reference the
visible label span via aria-labelledby while preserving the existing localized
context.
Replaces the inert "list of block names" panel with the actual template header and footer, rendered as real blocks inside the canvas iframe. ## Approach #655 described this as a dual-tree persistence rewrite. It isn't one. `__experimentalUseBlockPreview` mounts its own block-editor provider around a foreign tree, so the chrome never touches the content provider's `value` — which is what the earlier composed-tree attempt did, and why it froze (value swap → onChange echo → extract → state → new value, forever). The hook also wraps the subtree in `useDisabled()`, so chrome is genuinely non-selectable rather than merely locked, and sets `isPreviewMode` while inheriting the parent's editor settings. It deliberately clears `styles` because it expects to render inside a canvas whose EditorStyles already ran — exactly our case. So: content canvas untouched, one provider, one tree, existing persistence loop. Selection, undo history, and unsaved-changes survive a toggle by construction rather than by guard. ## The reason chrome couldn't render before Not the tree wiring — the namespace. The editor has registered only `artisanpack/*` since the I7 cutover (#415), while theme templates on disk are authored in core markup (`wp:group`, `wp:heading`, `wp:template-part`). The endpoint returned those names untouched, so every chrome block resolved to an unregistered type. The controller now rewrites `core/x` → `artisanpack/x` across the tree, mirroring `BlockMarkupHydrator::aliasFor()`. Blanket rather than registry-gated: `BlockTypeRegistry` only knows server-rendered PHP blocks, and a core name with no fork is unregistered under either namespace anyway. Rewriting `core/template-part` also hands part resolution to the fork from #674/#675, which fetches its own part through the core-data shim. That makes the client-side expanders redundant, so they're gone. ## Splitting `splitTemplateAroundContentSlot` only scanned top level, which meant the conventional shape — header part, `group[main]` wrapping the slot, footer part — found no slot and dumped the entire template into the header. It now descends, and clones the wrapper onto both sides when the slot is nested inside one. An approximation by construction; #618 scopes this to "close enough to see the shape". Templates with no slot at all now say so in the banner instead of silently misplacing the content. ## Cycle protection Moving resolution into the block moves the recursion risk with it: a part referencing itself (or two referencing each other) would mount the component inside itself until the stack blows. Guarded with a context carrying the ancestor part ids. Per-branch, so sibling refs to a shared part still both resolve. ## Removed `ChromePreviewPanel.tsx`, `compose.ts`, `extract.ts` and their tests — the last of the abandoned composed-tree approach, now unreachable. `hydrate.ts` stops being dead code; it's what keeps chrome from tripping block validation. `bindings.test.ts` went with `compose.ts`, but the #622 guarantee it pinned now rides on hydration, so `hydrate.test.ts` re-pins it there. Tests: +8 split, +9 hydrate, +5 template-part cycles, +3 Pest for the namespace rewrite. Full suites green — 2494 vitest, 1612 pest, and the Vite build compiles. Closes #655 Refs #618 #621 #622 #675
Verifying #655 against the dev app's `dev-sample` theme turned up a hole the whole test suite missed: `TemplateResolver` returns `ResolvedTemplate::$blocks` empty for every theme-file source. Those templates carry their content as raw block markup in `$rawContent`, so the applied-template endpoint was reading an empty array and the composed view rendered no chrome at all — for exactly the case it exists to serve, a template that ships with the theme. The tests didn't catch it because they register templates through the `ap.visual-editor.templates` filter with a populated `blocks` key, which bypasses the raw path entirely. Resolving `page` from `dev-sample` returned `blocks: []`; it now returns the template-part ref and the `main` group with its three children, correctly forked. #675 hit the same wall in the site editor and fixed it inside `TemplateAdapter`. Rather than duplicate ~60 lines, the parse and the tree conversion move to `Support\ThemeBlockMarkup` alongside the core→fork rewrite, and `TemplateAdapter` delegates to it. Its #675 tests pass unchanged, which is the point of routing them through the same code. Coverage: `parseToEditorBlocks` needs cms-framework 2.5+ (PHP 8.3+), so the end-to-end controller test carries the same skip guard the adapter tests use. The parser-independent halves — tree conversion, the fork rewrite, blank input — are covered unconditionally in a new unit suite, including the namespace-boundary case (`corefoo/bar` must not rewrite). Tests: +13 unit, +2 feature. 1625 pest, 2494 vitest, build clean. Refs #655 #675
The site-editor template/template-part endpoints rewrote only `core/template-part` to its `artisanpack/*` fork. Since the I7 cutover (#415) registered the artisanpack namespace exclusively, every *other* core name in a template or part reached the editor unregistered and rendered nothing — so the composed view's header/footer chrome resolved correctly and then mounted blank. #674 patched the same hole for `core/template-part` alone. Widen it to the whole namespace via the existing `ThemeBlockMarkup::rewriteCoreToFork` so theme files can keep authoring core markup (the WP convention) without each consuming app pre-forking its own block names. The rewrite runs after NavigationBlockRefResolver on purpose: that resolver matches `core/navigation` by name, so forking first would hide every nav block from it and regress the Keystone #48 `ref` stamping. `content.raw` keeps the narrow template-part-only rewrite — it exists for reserialization, and forking every name there would write artisanpack namespaces back into theme files on save. Also surface an info notice while the composed view is active, naming the applied template and stating that only the content is editable. The chrome renders as real but inert blocks, and without a word for it an author reading a header they cannot click assumes the editor is broken. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The blanket `core/` → `artisanpack/` rewrite means templates and parts now serve `artisanpack/block` for a synced-pattern reference, but the usage counter matched the core name only — so every count read zero. That number backs the delete-confirmation dialog, where "used nowhere" is the most dangerous possible wrong answer. Match both names: the raw fallback path still runs `parse()`, which emits core names regardless of what the adapter did to the parsed tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
6 tasks done
ViewFromTheBox
marked this pull request as ready for review
August 3, 2026 20:44
github-project-automation
Bot
moved this from In Review
to Done
in ArtisanPack UI Overview
5 of 15 tasks
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
resources/js/visual-editor/editor/composed-view/hydrate.ts (1)
65-94: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMissing
innerBlocksguard inhydrateBlocks, uncovered by tests.hydrateBlocksaccessesblock.innerBlocks.lengthwithout checking forundefined, whilesplit.ts— which processes the same raw endpoint payload upstream — explicitly guards the identical field. The test fixture helper masks this gap because it always defaultsinnerBlocksto[].
resources/js/visual-editor/editor/composed-view/hydrate.ts#L65-L94: addblock.innerBlocks !== undefined &&before the.length > 0check, mirroringsplit.ts's guard.resources/js/visual-editor/editor/composed-view/__tests__/hydrate.test.ts#L51-L57: add a test case that passes a block withinnerBlocks: undefinedto lock in the fix.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resources/js/visual-editor/editor/composed-view/hydrate.ts` around lines 65 - 94, The hydrateBlocks guard must handle missing innerBlocks values before reading .length. In resources/js/visual-editor/editor/composed-view/hydrate.ts:65-94, update hydrateBlocks to require block.innerBlocks to be defined before recursively hydrating non-empty children; in resources/js/visual-editor/editor/composed-view/__tests__/hydrate.test.ts:51-57, add coverage for a block with innerBlocks: undefined and verify hydration succeeds with no inner blocks.src/Http/Controllers/ResourceAppliedTemplateController.php (1)
264-291: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMatch both
core/template-partandartisanpack/template-part.
TemplateAdapter::toArray()rewrites everycore/template-partblock toartisanpack/template-partbefore the editor gets the tree. A template edited and saved through the editor can therefore carry the forked name, andextractPartSlugs()will not collect nestedcore/template-partrefs that exist asartisanpack/template-partin$part['blocks']returned fromcollectReferencedParts(). Match both names inextractPartSlugs()so resolved templates keep inlining referenced parts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Http/Controllers/ResourceAppliedTemplateController.php` around lines 264 - 291, Update extractPartSlugs() to recognize both core/template-part and artisanpack/template-part block names when collecting slugs, while preserving the existing recursive traversal and validation behavior so referenced parts are still inlined.
🧹 Nitpick comments (2)
resources/js/visual-editor/editor/composed-view/__tests__/hydrate.test.ts (1)
51-57: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTest helper never exercises a missing
innerBlockskey.
raw()always defaultsinnerBlocksto[]. No test in this suite passes a block withinnerBlocksentirely absent (undefined), which is the casehydrateBlocksdoesn't guard against (see the review comment onhydrate.tslines 65-94). Add a case withinnerBlocks: undefinedto lock in the fix there once applied.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resources/js/visual-editor/editor/composed-view/__tests__/hydrate.test.ts` around lines 51 - 57, Update the hydrateBlocks test coverage using the raw helper to create a block whose innerBlocks value is explicitly undefined, ensuring the suite exercises blocks with the key absent and verifies hydration handles that input without failure.resources/js/visual-editor/blocks/template-part/edit.tsx (1)
101-146: 📐 Maintainability & Code Quality | 🔵 TrivialCycle detection logic is correct.
The ancestor-tracking approach is sound.
useContextanduseMemoare called unconditionally on every render ofTemplatePartBody, so branching to eitherTemplatePartCycleStoporTemplatePartResolveddoes not violate the Rules of Hooks. Memoizingdescendantson[ancestors, compositeId]keeps the provider value referentially stable, avoiding unnecessary re-renders of the resolved subtree. Hand-tracing the self-reference, mutual-reference, longer-cycle, shared-sibling, and acyclic-chain test cases inedit-cycle.test.tsxagainst this logic confirms the expecteduseEntityBlockEditorcall counts and ordering in all five scenarios.One optional improvement:
TemplatePartCycleStopsilently drops content with no diagnostic. Since template parts are theme/DB content that "nothing upstream guarantees are acyclic" (per the JSDoc on Line 55), aconsole.warnwhen a cycle is detected would help a theme author notice and fix a malformed reference, instead of just seeing missing chrome with no explanation.♻️ Optional diagnostic on cycle detection
if (isCycle) { + if (process.env.NODE_ENV !== 'production') { + // eslint-disable-next-line no-console + console.warn( + `Template part cycle detected and stopped at "${compositeId}".`, + ); + } return <TemplatePartCycleStop tagName={tagName} />; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resources/js/visual-editor/blocks/template-part/edit.tsx` around lines 101 - 146, Add an optional diagnostic for detected template-part cycles in TemplatePartBody or TemplatePartCycleStop, including the relevant compositeId (and, if available, ancestor context) in a console.warn. Preserve the existing cycle-stop rendering behavior and hook ordering.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Http/Controllers/ResourceAppliedTemplateController.php`:
- Around line 104-121: Rename the foreach key variable in the loop over $parts
within the template response method to avoid shadowing the existing template
slug variable, while preserving its use for indexing $parts and leaving the
response construction unchanged.
---
Outside diff comments:
In `@resources/js/visual-editor/editor/composed-view/hydrate.ts`:
- Around line 65-94: The hydrateBlocks guard must handle missing innerBlocks
values before reading .length. In
resources/js/visual-editor/editor/composed-view/hydrate.ts:65-94, update
hydrateBlocks to require block.innerBlocks to be defined before recursively
hydrating non-empty children; in
resources/js/visual-editor/editor/composed-view/__tests__/hydrate.test.ts:51-57,
add coverage for a block with innerBlocks: undefined and verify hydration
succeeds with no inner blocks.
In `@src/Http/Controllers/ResourceAppliedTemplateController.php`:
- Around line 264-291: Update extractPartSlugs() to recognize both
core/template-part and artisanpack/template-part block names when collecting
slugs, while preserving the existing recursive traversal and validation behavior
so referenced parts are still inlined.
---
Nitpick comments:
In `@resources/js/visual-editor/blocks/template-part/edit.tsx`:
- Around line 101-146: Add an optional diagnostic for detected template-part
cycles in TemplatePartBody or TemplatePartCycleStop, including the relevant
compositeId (and, if available, ancestor context) in a console.warn. Preserve
the existing cycle-stop rendering behavior and hook ordering.
In `@resources/js/visual-editor/editor/composed-view/__tests__/hydrate.test.ts`:
- Around line 51-57: Update the hydrateBlocks test coverage using the raw helper
to create a block whose innerBlocks value is explicitly undefined, ensuring the
suite exercises blocks with the key absent and verifies hydration handles that
input without failure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 672354a2-c22f-435e-8d78-a649a5a4ba58
📒 Files selected for processing (25)
resources/js/visual-editor/blocks/template-part/__tests__/edit-cycle.test.tsxresources/js/visual-editor/blocks/template-part/edit.tsxresources/js/visual-editor/editor-settings.tsresources/js/visual-editor/editor/__tests__/canvas-styles.test.tsresources/js/visual-editor/editor/canvas-styles.tsresources/js/visual-editor/editor/composed-view/ChromeBlocks.tsxresources/js/visual-editor/editor/composed-view/__tests__/hydrate.test.tsresources/js/visual-editor/editor/composed-view/__tests__/split.test.tsresources/js/visual-editor/editor/composed-view/hydrate.tsresources/js/visual-editor/editor/composed-view/index.tsresources/js/visual-editor/editor/composed-view/split.tsresources/js/visual-editor/editor/editor-app.cssresources/js/visual-editor/editor/editor-app.tsxresources/js/visual-editor/editor/editor-canvas.tsxresources/js/visual-editor/site-editor/patterns/__tests__/use-pattern-usage.test.tsxresources/js/visual-editor/site-editor/patterns/use-pattern-usage.tssrc/Http/Controllers/ResourceAppliedTemplateController.phpsrc/Http/Resources/Adapters/CmsFramework/SiteEditor/TemplateAdapter.phpsrc/Support/ThemeBlockMarkup.phptests/Feature/SiteEditor/TemplateControllerTest.phptests/Feature/SiteEditor/TemplatePartControllerTest.phptests/Feature/VisualEditor/ResourceAppliedTemplateControllerTest.phptests/Unit/VisualEditor/SiteEditor/Adapters/TemplateAdapterTest.phptests/Unit/VisualEditor/SiteEditor/Adapters/TemplatePartAdapterTest.phptests/Unit/VisualEditor/Support/ThemeBlockMarkupTest.php
💤 Files with no reviewable changes (1)
- resources/js/visual-editor/editor/editor-app.css
🚧 Files skipped from review as they are similar to previous changes (2)
- resources/js/visual-editor/editor/composed-view/tests/split.test.ts
- resources/js/visual-editor/editor/editor-app.tsx
Comment on lines
+104
to
+121
| $blocks = $this->editorBlocksFor( $resolved ); | ||
| $parts = $this->collectReferencedParts( $blocks ); | ||
|
|
||
| foreach ( $parts as $slug => $part ) { | ||
| if ( is_array( $part ) && is_array( $part['blocks'] ?? null ) ) { | ||
| $parts[ $slug ]['blocks'] = ThemeBlockMarkup::rewriteCoreToFork( $part['blocks'] ); | ||
| } | ||
| } | ||
|
|
||
| return response()->json( [ | ||
| 'status' => 'ok', | ||
| 'slug' => $resolved->slug, | ||
| 'name' => $resolved->title, | ||
| 'source' => $resolved->source, | ||
| 'blocks' => ThemeBlockMarkup::rewriteCoreToFork( $blocks ), | ||
| 'template_parts' => $parts, | ||
| ] ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Rename the loop variable to avoid shadowing the template slug.
foreach ( $parts as $slug => $part ) (Line 107) reuses $slug, which already holds the resolved template slug from Line 88/94. The response body doesn't read $slug afterward (it uses $resolved->slug), so there's no active bug, but the shadowing is confusing and risks a real bug if this method is extended later.
♻️ Proposed rename
- foreach ( $parts as $slug => $part ) {
+ foreach ( $parts as $partSlug => $part ) {
if ( is_array( $part ) && is_array( $part['blocks'] ?? null ) ) {
- $parts[ $slug ]['blocks'] = ThemeBlockMarkup::rewriteCoreToFork( $part['blocks'] );
+ $parts[ $partSlug ]['blocks'] = ThemeBlockMarkup::rewriteCoreToFork( $part['blocks'] );
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| $blocks = $this->editorBlocksFor( $resolved ); | |
| $parts = $this->collectReferencedParts( $blocks ); | |
| foreach ( $parts as $slug => $part ) { | |
| if ( is_array( $part ) && is_array( $part['blocks'] ?? null ) ) { | |
| $parts[ $slug ]['blocks'] = ThemeBlockMarkup::rewriteCoreToFork( $part['blocks'] ); | |
| } | |
| } | |
| return response()->json( [ | |
| 'status' => 'ok', | |
| 'slug' => $resolved->slug, | |
| 'name' => $resolved->title, | |
| 'source' => $resolved->source, | |
| 'blocks' => ThemeBlockMarkup::rewriteCoreToFork( $blocks ), | |
| 'template_parts' => $parts, | |
| ] ); | |
| } | |
| $blocks = $this->editorBlocksFor( $resolved ); | |
| $parts = $this->collectReferencedParts( $blocks ); | |
| foreach ( $parts as $partSlug => $part ) { | |
| if ( is_array( $part ) && is_array( $part['blocks'] ?? null ) ) { | |
| $parts[ $partSlug ]['blocks'] = ThemeBlockMarkup::rewriteCoreToFork( $part['blocks'] ); | |
| } | |
| } | |
| return response()->json( [ | |
| 'status' => 'ok', | |
| 'slug' => $resolved->slug, | |
| 'name' => $resolved->title, | |
| 'source' => $resolved->source, | |
| 'blocks' => ThemeBlockMarkup::rewriteCoreToFork( $blocks ), | |
| 'template_parts' => $parts, | |
| ] ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Http/Controllers/ResourceAppliedTemplateController.php` around lines 104
- 121, Rename the foreach key variable in the loop over $parts within the
template response method to avoid shadowing the existing template slug variable,
while preserving its use for indexing $parts and leaving the response
construction unchanged.
17 of 21 tasks
This was referenced Aug 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Foundation for the umbrella feature #618 — Composed template preview in the post editor. Implements five sub-issues on one branch:
GET {resource}/{id}/applied-templateendpoint.Closes: #619 #620 #621 #622 #655
Type of Change
Related Issue
Umbrella: #618. Targets
feature/templates-preview, the integration branch for #618 — the whole feature lands in a release branch together, not piecemeal.Motivation and Context
Authors currently only see the raw block list on a bare canvas. To check how a page will look inside its template they leave the editor for the Preview, losing the ability to edit inline. This branch delivers the primitives that let the composed view render inside the existing iframe canvas without a server-side round-trip.
Architecture note — this branch pivoted
The original #621 design composed one tree: template blocks wrapped around a content slot, fed to a single
BlockEditorProvider. That approach was abandoned mid-branch and its modules (compose.ts,extract.ts) no longer exist.BlockEditorProvideris single-tree and stateful, and driving it that way produced block-validation warnings on hand-shaped chrome, then an editor freeze: swappingvalueechoed throughonChange→ extract → state → new composed tree → newvalue, forever.What shipped instead: the content canvas is left completely alone. The template's chrome renders as inert block previews above and below it, each mounting its own isolated block-editor store via
__experimentalUseBlockPreview. The content provider'svalueis never touched, so selection, undo history, and unsaved-changes survive a toggle by construction.Reviewers: read the code against this description, not the #621 issue text — that issue still describes the abandoned design and needs rewriting before it's closed.
Changes Made
Server (#619)
src/Http/Controllers/ResourceAppliedTemplateController.php— new controller. Reads the model'stemplateattribute (overridable via?template=so a preview taken right after a template change doesn't race the debounced save), resolves through the sharedTemplateResolver, and walks the block tree collecting every referencedcore/template-part— including nested refs, with a visited set for cycles.GET {resource}/{id}/applied-templateunder the existing visual-editor API middleware; authorized withGate::authorize('view', $model).{ status: 'missing', reason: 'empty' | 'unknown-slug', slug? }. "No template selected" is the routine response for most content, and a 404 on every editor mount makes devtools noise out of normal operation.src/Support/ThemeBlockMarkup.php— shared helper. Theme-file templates arrive withrawContentpopulated andblocksempty, so it parses raw block markup into the editor tree shape, and rewritescore/*names to their registeredartisanpack/*forks.Server — fork rewrite in the site-editor adapters
TemplateAdapterpreviously forked onlycore/template-part. Since the I7 cutover (#415) registered the artisanpack namespace exclusively, every other core name reached the editor unregistered and rendered nothing — so template parts resolved correctly and then mounted blank. Widened to the whole namespace viaThemeBlockMarkup::rewriteCoreToFork, so theme files keep authoring core markup (the WP convention) without each consuming app pre-forking its own block names.Two deliberate constraints:
NavigationBlockRefResolver, which matchescore/navigationby name — forking first would hide every nav block from it and regress the Keystone Navigation & Breadcrumbs #48refstamping.content.rawkeeps the narrow template-part-only rewrite. It exists for reserialization, and forking every name there would write artisanpack namespaces back into theme files on save.Downstream consequence, called out for review: the site editor now holds forked names, so saving a template writes
artisanpack/*intoblock_contentwhere it previously wrotecore/*for everything but template-part. Read is idempotent so round-trips are stable (asserted in the feature tests), but DB template content will drift toward the fork namespace over time.Client — top bar (#620)
top-bar.tsx: newViewModetype,viewMode/onViewModeChange/viewModeDisabledReasonprops. Renders as an aria switch with a disabled-state reason while the template resolves.editor-app.tsxholds ephemeral view-mode state — the editor always opens in bare-content mode.Client — composed view module (#621, #655)
resources/js/visual-editor/editor/composed-view/:api.ts— typed fetch with a discriminatedAppliedTemplateResult. Validates theokshape before handing it on, so a truncated body or a proxy's HTML error page fails at the boundary rather than deep inside the split.split.ts— splits the template around its content slot into header/footer chrome. Descends intoinnerBlocks, since the slot is conventionally nested inside a layout wrapper; when found nested, the wrapper is cloned onto both sides so surrounding layout still reads correctly. Template-part refs are not expanded here — theartisanpack/template-partfork resolves its own part through the core-data shim (fix(site-editor): render wp:template-part refs inline in the canvas #675), so expanding would double-render.hydrate.ts— runs server JSON throughcreateBlockfor fresh clientIds, schema defaults, andisValid: true. Raw JSON trips Gutenberg's validation pass and surfaces "unexpected or invalid content" on every block.ChromeBlocks.tsx— inert rendering via__experimentalUseBlockPreview, which wraps the subtree inuseDisabled()(genuinely non-selectable, not merely locked) and setsisPreviewMode.use-applied-template.ts—idle → loading → ok | missing | error, cached per(resource, id, template)so re-toggling is instant and a template change invalidates.Client — notices
The composed view surfaces one notice at a time above the canvas:
The minimal-default-template fallback + toast from #618's scope is not in this branch — that's #624.
Client — bindings (#622)
The existing
setBindingsResourceContext(props.resource, props.id)inEditorAppalready covers template-chrome consumers (post-title,post-author,post-date,post-featured-imageall resolve against the current content). Added an unmount-clear effect so a subsequent mount doesn't inherit stale sentinel state.Client — pattern usage counting
The blanket fork rewrite means templates and parts now serve
artisanpack/blockfor a synced-pattern reference, butuse-pattern-usage.tsmatched the core name only — so every usage count read zero. That number backs the delete-confirmation dialog, where "used nowhere" is the most dangerous possible wrong answer. Now matches both names; the raw fallback path still runsparse(), which emits core names regardless.How Has This Been Tested?
Testing Environment: PHP 8.4.3 · Vitest v2.1.9
Automated:
./vendor/bin/pest— 1654 passed, 1 skipped, 4422 assertions.npm test— 2923 passed. 18 failures, all insite-editor-app.test.tsx(window.localStorageundefined in that file's environment), confirmed pre-existing by re-running with this branch's changes stashed.npx vitest run resources/js/visual-editor/editor/composed-view— 26 passed.Manual (dev app, real theme + fixture theme side by side):
templateset — header/footer chrome renders around the live content, and the content stays editable.hasThemeFile: true, raw markup, no pre-parsed blocks) as well as filter-contributed fixtures, so both the parse leg and the pre-parsed leg are covered.templatecolumn.Not yet verified: that selection / undo history / unsaved-changes survive toggling. That property is structural (the content provider's
valueis never swapped) but has no explicit test — tracked on #621.Known Gaps (tracked, not in this PR)
?entity=template&slug=deep-link. Coupled to Composed view: locked-canvas ribbon with Edit template deep-link CTA #623 (the CTA is what targets it).docs/post-editor/Composed-View.md.Summary by CodeRabbit