Repository navigation
feat: D2 · Template browser + editor integration - #375
Merged
Merged
Conversation
Wires the Templates and Template Parts sections into the D1 site-editor
shell: list views in the navigator, open-in-canvas editing, save via the
C1/C2 REST endpoints, and a reused A1 inspector with kind-specific
Document panels. Implements design brief §3.4, §3.5, §5.1, §5.5 and
principles P7 (visible fallback chains) + P9 (destructive confirmations).
Frontend additions under resources/js/visual-editor/site-editor/:
- api-client.ts — typed REST client for templates + parts (CSRF, 422
validation extraction, paginated list envelope).
- use-entity-list.ts / use-entity-editor.ts — list + single-entity
state with dirty tracking, save/load status, and refresh signals.
- entity-browser.tsx — shared list UI with filter chips and row-level
keyboard navigation.
- entity-editor-canvas.tsx — BlockList / WritingFlow / BlockTools
surface, announces the entity for screen readers.
- entity-editor.tsx — stitches canvas + inspector, bubbles save state
to the shell top bar.
- templates-section.tsx / template-parts-section.tsx — kind-specific
browser wrappers, Document panels, and create-new dialogs with the
fallback-chain visualizer (P7) and area selector.
- create-entity-dialog.tsx — focus-trapping dialog shell with
accessible form controls and a custom-styled native select.
- fallback-chain.ts — pure client-side mirror of the PHP resolver.
- slug.ts — live input normalizer (lowercase + space→dash).
Shell wiring (site-editor-app.tsx):
- Registers core blocks on boot, unregisters the D2 out-of-scope
entity blocks (core/template-part, core/post-*, core/site-*, etc.)
so seeded templates that embed them do not trip the block-crash
boundary — E4 will re-enable them.
- Threads apiBase/theme from the blade through the React mount.
- Lifts per-section refresh counters so the navigator list refetches
after a successful create.
- Owns the top-bar Save handler and dirty indicator, wired to whichever
entity editor is currently mounted.
Backend:
- routes/web.php — wraps the SPA mount in `web` middleware so Laravel
starts a session and the CSRF token in the blade actually matches
the session on API mutation requests.
- resources/views/site-editor/index.blade.php — passes apiBase +
theme + csrf-token meta to the SPA.
Tests (79 site-editor tests total, up from 51 pre-D2):
- api-client, use-entity-editor, entity-browser, create-entity-dialog,
entity-editor, fallback-chain, plus updates to main.test /
site-editor-app.test for the new required props and D2 stubs.
Closes #369
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds D2 template and template-part editing: a typed SiteEditor REST client with CSRF handling, list/pagination hooks, entity browser and editor UIs (including a Gutenberg-like canvas), create/edit dialogs and inspector panels, slug/fallback utilities, shell wiring, styles, and comprehensive Vitest tests. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Browser UI / Dialog
participant Editor as useEntityEditor / Canvas
participant Api as SiteEditor API Client
participant Server
Note over User,UI: Create new entity
User->>UI: Open Create Dialog & Submit payload
UI->>Api: POST /{templates|template-parts} (X-CSRF-TOKEN)
Api->>Server: HTTP POST
Server-->>Api: 201 Created (entity JSON)
Api-->>UI: created entity record
UI->>Editor: open(entity.id)
Editor->>Api: GET /{kind}/{id}
Api->>Server: HTTP GET
Server-->>Api: 200 Entity JSON
Api-->>Editor: entity + content
Editor-->>UI: render canvas (blocks loaded)
Note over User,Editor: Save existing entity
User->>Editor: Click Save
Editor->>Api: PUT /{kind}/{id} (X-CSRF-TOKEN, body)
Api->>Server: HTTP PUT
alt validation error 422
Server-->>Api: 422 with errors
Api-->>Editor: SiteEditorApiError (validationErrors)
Editor-->>UI: show field errors, set saveStatus='error'
else success 200
Server-->>Api: 200 updated entity
Api-->>Editor: updated record
Editor-->>UI: clear dirty, show saved
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 12
🧹 Nitpick comments (5)
resources/js/visual-editor/site-editor/site-editor-app.tsx (1)
86-151: Consider movingensureEditorBoot()into auseEffect.Calling
ensureEditorBoot()directly during render (line 166) is a side effect, which React's concurrent mode may call multiple times. While theeditorBootedflag makes it idempotent, moving the call into auseEffectwith an empty dependency array would be more aligned with React conventions and avoid potential issues if the flag gets reset (e.g., during HMR in development).Proposed refactor
export function SiteEditorApp(props: SiteEditorAppProps): JSX.Element { - ensureEditorBoot(); - const { routeBase, postEditorUrl, apiBase, theme = 'default' } = props; + useEffect(() => { + ensureEditorBoot(); + }, []); + const apiConfig = useMemo<SiteEditorApiConfig>(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/js/visual-editor/site-editor/site-editor-app.tsx` around lines 86 - 151, The ensureEditorBoot() invocation is a render-time side effect; instead, remove the direct call from the component render and invoke ensureEditorBoot() inside a React useEffect with an empty dependency array so it runs once after mount (keep the ensureEditorBoot function and its editorBooted flag unchanged for idempotency). Add the useEffect import from React if missing and place the effect where the component hooks live; do not move the boot logic out of ensureEditorBoot itself, only change the call site to useEffect. Ensure the change preserves HMR behavior (the editorBooted guard remains).resources/js/visual-editor/site-editor/entity-browser.tsx (1)
278-289: Minor: HoisthasActiveoutside the map.
hasActiveis recomputed identically for every item in the list. Moving it before the.map()avoids redundantArray.some()calls on each iteration.Proposed refactor
+ const hasActive = items.some( + (candidate) => String(candidate.id) === activeEntityId + ); {items.map((entity, index) => { const entityId = String(entity.id); const isActive = activeEntityId === entityId; - const hasActive = items.some( - (candidate) => String(candidate.id) === activeEntityId - ); // Roving tabindex: one row must always be tab-reachable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/js/visual-editor/site-editor/entity-browser.tsx` around lines 278 - 289, The computation of hasActive is repeated inside the items.map loop; hoist the hasActive calculation above the map to compute it once (use the same check: items.some(candidate => String(candidate.id) === activeEntityId)), then inside the map keep using hasActive to derive isTabStop (alongside activeEntityId, index, isActive) and leave extractTitle(entity) and the rest unchanged; update the block that currently declares hasActive within the map to remove the per-iteration some() call.resources/js/visual-editor/site-editor/entity-editor.tsx (1)
233-253:handlePatchrecreated insideuseMemo.The
handlePatchfunction is defined inside theuseMemofordocumentPanel, meaning it gets a new identity whenever the memo dependencies change. Since the document panels likely passonPatchto child components, this could cause unnecessary re-renders.Consider extracting
handlePatchto a stableuseCallback:Proposed refactor
+ const handlePatch = useCallback( + (overrides: UpdatePayload): void => { + patch(overrides); + }, + [patch] + ); + const documentPanel = useMemo((): ReactNode => { - const handlePatch = (overrides: UpdatePayload): void => { - patch(overrides); - }; - if (kind === 'template') { return ( <TemplateDocumentPanel🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/js/visual-editor/site-editor/entity-editor.tsx` around lines 233 - 253, The local function handlePatch is being recreated inside the useMemo for documentPanel causing a new function identity each memo and likely unnecessary re-renders of child panels; move handlePatch out of the useMemo and wrap it in a stable useCallback that calls patch (e.g., const handlePatch = useCallback((overrides: UpdatePayload) => patch(overrides), [patch])), then pass that handlePatch into the TemplateDocumentPanel and TemplatePartDocumentPanel and update the documentPanel hook dependencies to include handlePatch if you keep it in the deps list.resources/js/visual-editor/site-editor/use-entity-editor.ts (1)
97-108: Fallback cast ofcontent.blocksbypasses validation.When
rawis empty,content.blocksis cast directly toBlockInstance[]without validating that each element has the expected shape (name,attributes, etc.). If the backend returns malformed data, this could cause runtime errors in Gutenberg's rendering.Consider adding minimal validation or using
parse()on a re-serialized form if the shape is uncertain.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/js/visual-editor/site-editor/use-entity-editor.ts` around lines 97 - 108, The hydrateBlocks function currently casts content.blocks to BlockInstance[] without validation; update hydrateBlocks to defensive-validate and normalize that array before returning: ensure content.blocks is an array and each item has required keys (e.g., name, attributes, innerBlocks) and types, filter out or fill missing fields with safe defaults, and if validation fails for any element fallback to a safe path (e.g., re-serialize content.blocks and run parse(...) on the serialized string or return an empty array) so Gutenberg never receives malformed BlockInstance objects. Locate hydrateBlocks and implement the validation/normalization logic around the return of content.blocks as BlockInstance[] and use parse(...) as the fallback when elements are invalid.resources/js/visual-editor/site-editor/entity-editor-canvas.tsx (1)
45-62: TypeblocksasBlockInstance[]for type safety.The
blocksprop is typed asreadonly unknown[], butBlockEditorProviderexpectsBlockInstance[]. This loose typing bypasses TypeScript's help in catching mismatches. The parent (use-entity-editor.ts) already maintains blocks asBlockInstance[]internally.Proposed fix
+import type { BlockInstance } from '@wordpress/blocks'; + export interface EntityEditorCanvasProps { /** Identifier for the active entity (used in a11y announcements). */ entityTitle: string; /** Parsed block tree loaded from the REST response. */ - blocks: readonly unknown[]; - onChange: (blocks: readonly unknown[]) => void; - onInput: (blocks: readonly unknown[]) => void; + blocks: readonly BlockInstance[]; + onChange: (blocks: readonly BlockInstance[]) => void; + onInput: (blocks: readonly BlockInstance[]) => void;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/js/visual-editor/site-editor/entity-editor-canvas.tsx` around lines 45 - 62, The blocks prop on EntityEditorCanvasProps is currently typed as readonly unknown[] which defeats type safety; change its type to BlockInstance[] (and readonly where appropriate) so the signature in EntityEditorCanvasProps aligns with the BlockEditorProvider and the blocks produced by use-entity-editor.ts; update the interface property `blocks` and any related callbacks (`onChange`, `onInput`) to accept BlockInstance[] to restore static type checking and prevent mismatches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@resources/js/visual-editor/site-editor/entity-browser.css`:
- Around line 92-107: The CSS uses the keyword currentColor in the
.ap-site-editor__entity-browser-retry rules (border: 1px solid currentColor; and
outline: 2px solid currentColor;) which violates the stylelint keyword-case
rule; update those keyword usages to lowercase (currentcolor) in the
.ap-site-editor__entity-browser-retry block and its :focus-visible rule so the
property values match the configured lowercase keyword expectation.
In `@resources/js/visual-editor/site-editor/entity-browser.tsx`:
- Around line 105-124: The entity list hook isn't resetting pagination when the
active filter chip changes; update useEntityList to either accept a reset signal
or return a setter for its internal page state (e.g., export setPage alongside
items/status/errorMessage/refresh) and then call setPage(1) whenever
activeChipId (or activeChip) changes in entity-browser.tsx; specifically, modify
the useEntityList hook to expose setPage (or a resetPage() method) and invoke
setPage(1) in a useEffect watching activeChipId (or activeChip) after calling
refresh so the list always starts at page 1 for new filters.
In `@resources/js/visual-editor/site-editor/entity-editor-canvas.css`:
- Around line 90-97: The CSS rule for the pseudo-element
.ap-site-editor__dirty-indicator::before uses the keyword "currentColor" with
mixed-case; update the background property to use the lowercase keyword
"currentcolor" to satisfy Stylelint's casing rule and normalize CSS keyword
casing.
- Around line 61-71: The .ap-site-editor__sr-only utility uses the deprecated
clip property; update the rule in the CSS for .ap-site-editor__sr-only to remove
the clip declaration and replace it with a modern equivalent such as clip-path:
inset(50%) (and keep the existing position/size/overflow/white-space/border
rules) so the element remains visually hidden but accessible; locate the
.ap-site-editor__sr-only selector in the diff and swap the clip declaration for
clip-path: inset(50%) (or another suitable clip-path) to satisfy the lint rule.
In `@resources/js/visual-editor/site-editor/fallback-chain.ts`:
- Around line 67-121: The getTemplateKindOptions function returns the list of
template kinds but omits the "custom" kind referenced in the header/docs; update
getTemplateKindOptions (and the TemplateKindOption entries it produces) to
include a { slug: 'custom', label: 'Custom', description: 'User-defined custom
template.' } entry (or alternatively update the header text if you intend to
remove support) so the create-new picker contract matches the documentation and
the create dialog can explicitly select the custom-template flow.
In `@resources/js/visual-editor/site-editor/site-editor-app.tsx`:
- Around line 237-243: The useEffect dependency array includes an unnecessary
dependency: remove activeSection.id and leave only isD2Section in the dependency
array of the effect that resets entity state; locate the effect using useEffect,
isD2Section, setEntityState and IDLE_ENTITY_STATE and change its dependencies to
[isD2Section] (or, if you actually intend to reset when switching between D2
sections, modify the effect condition to compare previous and current
activeSection.id instead of relying solely on isD2Section).
In `@resources/js/visual-editor/site-editor/slug.ts`:
- Around line 10-12: The normalizeSlugInput function produces leading/trailing
dashes for inputs with surrounding spaces; update normalizeSlugInput to trim the
input first (call .trim()) before lowercasing and collapsing whitespace into
hyphens so " Foo " becomes "foo" and matches backend normalization; modify the
implementation referenced as normalizeSlugInput(value: string) to apply trim()
prior to toLowerCase() and .replace(/\s+/g, '-').
In `@resources/js/visual-editor/site-editor/template-parts-section.tsx`:
- Around line 78-87: The renderRow callback is rendering the raw machine slug
(entity.area) instead of the localized label; update renderRow in
template-parts-section.tsx to look up the human label via getTemplatePartAreas()
(e.g., const areas = getTemplatePartAreas()), render areas[entity.area] (or
areas[entity.area]?.label) and fall back to entity.area if missing, so the span
with data-testid={`ap-site-editor-template-part-area-${entity.id}`} shows the
translated area name rather than the raw slug; ensure getTemplatePartAreas is
imported where renderRow is defined.
In `@resources/js/visual-editor/site-editor/templates-section.tsx`:
- Around line 47-60: The template chip list currently only filters by
publication status; update buildTemplateChips to include chips for template
source/kind so users can filter Theme and Custom templates (e.g., add entries
with id 'theme' and 'custom', labels like __('Theme', TEXT_DOMAIN) and
__('Custom', TEXT_DOMAIN), and filters such as { kind: 'theme' } and { kind:
'custom' } or the appropriate property used by your data model), ensuring the
new FilterChip objects are returned alongside existing 'all', 'publish', and
'draft' chips so the navigator can show only theme or custom templates.
- Around line 67-75: renderRow currently only shows slug, source and status;
update the renderRow (EntityRecord<'template'>) to also render the template
fallback-chain breadcrumbs (use entity.fallbackChain or entity.fallbacks—render
each ancestor as a breadcrumb element, separated by chevrons) and add a "Used by
N" affordance showing the usage count (use entity.usageCount or call the
navigator/getter that returns usage for this template) as a clickable element
that opens the navigator/usage view for that template (call the existing
navigator/openTemplateUsage or navigator.showUsage with entity.id). Ensure
accessibility and keep existing TEXT_DOMAIN i18n calls for the new link text.
- Around line 256-259: buildPayload currently returns null when activeSlug is
empty which causes the submit handler to silently no-op; update buildPayload
(and the submit handler that consumes it) so that when activeKind === 'custom'
and the normalized activeSlug is blank you set a visible validation error on the
slug field (e.g. call the existing form error/validation setter used in this
component) and then return null/abort, or alternatively have the submit handler
detect a null payload and call the same form error setter to surface feedback to
the user; ensure you apply the same change for the other buildPayload call site
that handles custom templates.
In `@resources/js/visual-editor/site-editor/use-entity-editor.ts`:
- Around line 183-186: The fallback error string used when building the
`message` variable in use-entity-editor (where the code checks `error instanceof
SiteEditorApiError`) is hardcoded and must be localized; replace the hardcoded
`'Failed to load entity.'` with a call to the i18n helper (e.g. `__('Failed to
load entity.')`) so it matches other modules, and make the same change for the
similar fallback error around the later fallback near line ~280 in this file;
ensure you import/use the same `__` function used in `use-entity-list.ts` and
`create-entity-dialog.tsx` if not already imported.
---
Nitpick comments:
In `@resources/js/visual-editor/site-editor/entity-browser.tsx`:
- Around line 278-289: The computation of hasActive is repeated inside the
items.map loop; hoist the hasActive calculation above the map to compute it once
(use the same check: items.some(candidate => String(candidate.id) ===
activeEntityId)), then inside the map keep using hasActive to derive isTabStop
(alongside activeEntityId, index, isActive) and leave extractTitle(entity) and
the rest unchanged; update the block that currently declares hasActive within
the map to remove the per-iteration some() call.
In `@resources/js/visual-editor/site-editor/entity-editor-canvas.tsx`:
- Around line 45-62: The blocks prop on EntityEditorCanvasProps is currently
typed as readonly unknown[] which defeats type safety; change its type to
BlockInstance[] (and readonly where appropriate) so the signature in
EntityEditorCanvasProps aligns with the BlockEditorProvider and the blocks
produced by use-entity-editor.ts; update the interface property `blocks` and any
related callbacks (`onChange`, `onInput`) to accept BlockInstance[] to restore
static type checking and prevent mismatches.
In `@resources/js/visual-editor/site-editor/entity-editor.tsx`:
- Around line 233-253: The local function handlePatch is being recreated inside
the useMemo for documentPanel causing a new function identity each memo and
likely unnecessary re-renders of child panels; move handlePatch out of the
useMemo and wrap it in a stable useCallback that calls patch (e.g., const
handlePatch = useCallback((overrides: UpdatePayload) => patch(overrides),
[patch])), then pass that handlePatch into the TemplateDocumentPanel and
TemplatePartDocumentPanel and update the documentPanel hook dependencies to
include handlePatch if you keep it in the deps list.
In `@resources/js/visual-editor/site-editor/site-editor-app.tsx`:
- Around line 86-151: The ensureEditorBoot() invocation is a render-time side
effect; instead, remove the direct call from the component render and invoke
ensureEditorBoot() inside a React useEffect with an empty dependency array so it
runs once after mount (keep the ensureEditorBoot function and its editorBooted
flag unchanged for idempotency). Add the useEffect import from React if missing
and place the effect where the component hooks live; do not move the boot logic
out of ensureEditorBoot itself, only change the call site to useEffect. Ensure
the change preserves HMR behavior (the editorBooted guard remains).
In `@resources/js/visual-editor/site-editor/use-entity-editor.ts`:
- Around line 97-108: The hydrateBlocks function currently casts content.blocks
to BlockInstance[] without validation; update hydrateBlocks to
defensive-validate and normalize that array before returning: ensure
content.blocks is an array and each item has required keys (e.g., name,
attributes, innerBlocks) and types, filter out or fill missing fields with safe
defaults, and if validation fails for any element fallback to a safe path (e.g.,
re-serialize content.blocks and run parse(...) on the serialized string or
return an empty array) so Gutenberg never receives malformed BlockInstance
objects. Locate hydrateBlocks and implement the validation/normalization logic
around the return of content.blocks as BlockInstance[] and use parse(...) as the
fallback when elements are invalid.
🪄 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: 2e6f209d-fd22-411b-99a5-e74d487d1e5b
📒 Files selected for processing (27)
resources/js/visual-editor/site-editor/__tests__/api-client.test.tsresources/js/visual-editor/site-editor/__tests__/create-entity-dialog.test.tsxresources/js/visual-editor/site-editor/__tests__/entity-browser.test.tsxresources/js/visual-editor/site-editor/__tests__/entity-editor.test.tsxresources/js/visual-editor/site-editor/__tests__/fallback-chain.test.tsresources/js/visual-editor/site-editor/__tests__/main.test.tsxresources/js/visual-editor/site-editor/__tests__/site-editor-app.test.tsxresources/js/visual-editor/site-editor/__tests__/use-entity-editor.test.tsxresources/js/visual-editor/site-editor/api-client.tsresources/js/visual-editor/site-editor/create-entity-dialog.cssresources/js/visual-editor/site-editor/create-entity-dialog.tsxresources/js/visual-editor/site-editor/entity-browser.cssresources/js/visual-editor/site-editor/entity-browser.tsxresources/js/visual-editor/site-editor/entity-editor-canvas.cssresources/js/visual-editor/site-editor/entity-editor-canvas.tsxresources/js/visual-editor/site-editor/entity-editor.tsxresources/js/visual-editor/site-editor/fallback-chain.tsresources/js/visual-editor/site-editor/main.tsxresources/js/visual-editor/site-editor/site-editor-app.cssresources/js/visual-editor/site-editor/site-editor-app.tsxresources/js/visual-editor/site-editor/slug.tsresources/js/visual-editor/site-editor/template-parts-section.tsxresources/js/visual-editor/site-editor/templates-section.tsxresources/js/visual-editor/site-editor/use-entity-editor.tsresources/js/visual-editor/site-editor/use-entity-list.tsresources/views/site-editor/index.blade.phproutes/web.php
Comment on lines
+67
to
+121
| * Known template kinds the create-new picker surfaces. The design brief | ||
| * (§3.4, §5.1) calls out "Front page, Home, Single post, Single page, | ||
| * Archive, 404, Search, Custom" — kept in this order so the picker reads | ||
| * left-to-right from most specific to most generic. | ||
| */ | ||
| export interface TemplateKindOption { | ||
| slug: string; | ||
| label: string; | ||
| description: string; | ||
| } | ||
|
|
||
| export function getTemplateKindOptions(): readonly TemplateKindOption[] { | ||
| return [ | ||
| { | ||
| slug: 'front-page', | ||
| label: 'Front page', | ||
| description: 'The site homepage when a static front page is assigned.', | ||
| }, | ||
| { | ||
| slug: 'home', | ||
| label: 'Blog home', | ||
| description: 'The posts archive when not used as the front page.', | ||
| }, | ||
| { | ||
| slug: 'single', | ||
| label: 'Single post', | ||
| description: 'The default layout for individual posts.', | ||
| }, | ||
| { | ||
| slug: 'page', | ||
| label: 'Single page', | ||
| description: 'The default layout for individual pages.', | ||
| }, | ||
| { | ||
| slug: 'archive', | ||
| label: 'Archive', | ||
| description: 'Listings for categories, tags, and custom taxonomies.', | ||
| }, | ||
| { | ||
| slug: 'search', | ||
| label: 'Search results', | ||
| description: 'The search results page.', | ||
| }, | ||
| { | ||
| slug: '404', | ||
| label: '404 (not found)', | ||
| description: 'Shown when no other template matches the request.', | ||
| }, | ||
| { | ||
| slug: 'index', | ||
| label: 'Index (fallback)', | ||
| description: 'The last-resort fallback used when no other template matches.', | ||
| }, | ||
| ]; | ||
| } |
There was a problem hiding this comment.
Expose a custom kind (or correct the contract/docs).
Line 68-70 says the picker includes Custom, but Line 78-120 returns no custom option. If this array drives the create dialog, custom-template flow is not explicitly selectable.
Proposed fix
export function getTemplateKindOptions(): readonly TemplateKindOption[] {
return [
@@
{
slug: 'search',
label: 'Search results',
description: 'The search results page.',
},
+ {
+ slug: 'custom',
+ label: 'Custom',
+ description: 'A custom template with a manually chosen slug.',
+ },
{
slug: '404',
label: '404 (not found)',
description: 'Shown when no other template matches the request.',
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@resources/js/visual-editor/site-editor/fallback-chain.ts` around lines 67 -
121, The getTemplateKindOptions function returns the list of template kinds but
omits the "custom" kind referenced in the header/docs; update
getTemplateKindOptions (and the TemplateKindOption entries it produces) to
include a { slug: 'custom', label: 'Custom', description: 'User-defined custom
template.' } entry (or alternatively update the header text if you intend to
remove support) so the create-new picker contract matches the documentation and
the create dialog can explicitly select the custom-template flow.
Comment on lines
+47
to
+60
| function buildTemplateChips(): readonly FilterChip[] { | ||
| return [ | ||
| { id: 'all', label: __('All', TEXT_DOMAIN), filter: {} }, | ||
| { | ||
| id: 'publish', | ||
| label: __('Published', TEXT_DOMAIN), | ||
| filter: { status: 'publish' }, | ||
| }, | ||
| { | ||
| id: 'draft', | ||
| label: __('Drafts', TEXT_DOMAIN), | ||
| filter: { status: 'draft' }, | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
Expose the required Theme/Custom template filter in the navigator.
This chip set only narrows by publication state. D2’s template browser requirements call for browsing by kind/source as well, so there’s currently no way to view just theme or custom templates from the list.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@resources/js/visual-editor/site-editor/templates-section.tsx` around lines 47
- 60, The template chip list currently only filters by publication status;
update buildTemplateChips to include chips for template source/kind so users can
filter Theme and Custom templates (e.g., add entries with id 'theme' and
'custom', labels like __('Theme', TEXT_DOMAIN) and __('Custom', TEXT_DOMAIN),
and filters such as { kind: 'theme' } and { kind: 'custom' } or the appropriate
property used by your data model), ensuring the new FilterChip objects are
returned alongside existing 'all', 'publish', and 'draft' chips so the navigator
can show only theme or custom templates.
Comment on lines
+67
to
+75
| const renderRow = useCallback((entity: EntityRecord<'template'>): JSX.Element => { | ||
| return ( | ||
| <> | ||
| <code>{entity.slug}</code> | ||
| <span>{entity.source === 'theme' ? __('Theme', TEXT_DOMAIN) : __('Custom', TEXT_DOMAIN)}</span> | ||
| <span>{entity.status}</span> | ||
| </> | ||
| ); | ||
| }, []); |
There was a problem hiding this comment.
List rows are missing the fallback breadcrumbs and usage affordance.
These rows only render slug, source, and status. The D2 browser acceptance also calls for visible fallback-chain breadcrumbs and a Used by N link/count from the navigator, so users still have to open each template to see routing and impact.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@resources/js/visual-editor/site-editor/templates-section.tsx` around lines 67
- 75, renderRow currently only shows slug, source and status; update the
renderRow (EntityRecord<'template'>) to also render the template fallback-chain
breadcrumbs (use entity.fallbackChain or entity.fallbacks—render each ancestor
as a breadcrumb element, separated by chevrons) and add a "Used by N" affordance
showing the usage count (use entity.usageCount or call the navigator/getter that
returns usage for this template) as a clickable element that opens the
navigator/usage view for that template (call the existing
navigator/openTemplateUsage or navigator.showUsage with entity.id). Ensure
accessibility and keep existing TEXT_DOMAIN i18n calls for the new link text.
- Normalize CSS `currentColor` → `currentcolor` (stylelint casing rule). - Swap deprecated `clip: rect(0, 0, 0, 0)` for `clip-path: inset(50%)` on the screen-reader-only utility. - Reset entity-browser pagination to page 1 when the active filter chip changes (was carrying a stale cursor across filters). - Drop redundant `activeSection.id` dep from the D2-section-leave effect — the guard only fires on `!isD2Section` so the extra dep churned renders without changing behaviour. - Trim leading/trailing whitespace in `normalizeSlugInput` so " Foo " becomes `foo` instead of `-foo-`. - Template-part browser rows now show the localized area label (Header/Footer/...) instead of the raw machine slug. - Template-create dialog surfaces a visible validation error when the custom slug is empty, instead of silently no-op'ing the submit. - `use-entity-editor`'s load and save fallback strings go through `__()` for i18n parity with the rest of the site-editor. - Update `getTemplateKindOptions` docblock to clarify why `custom` isn't in the list (the dialog renders it as a dedicated sentinel).
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
resources/js/visual-editor/site-editor/templates-section.tsx (2)
47-60:⚠️ Potential issue | 🟠 MajorAdd the Theme/Custom navigator chips.
The file header and D2 browser requirements both call for
All / Theme / Custom, but this helper still only exposes publication-state filters. Users currently cannot narrow the navigator to theme templates vs custom templates without scanning the full list.Suggested change
function buildTemplateChips(): readonly FilterChip[] { return [ { id: 'all', label: __('All', TEXT_DOMAIN), filter: {} }, + { + id: 'theme', + label: __('Theme', TEXT_DOMAIN), + filter: { source: 'theme' }, + }, + { + id: 'custom', + label: __('Custom', TEXT_DOMAIN), + filter: { source: 'custom' }, + }, { id: 'publish', label: __('Published', TEXT_DOMAIN), filter: { status: 'publish' }, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/js/visual-editor/site-editor/templates-section.tsx` around lines 47 - 60, buildTemplateChips currently only returns publication-state chips and must include navigator chips for Theme and Custom; update the buildTemplateChips function to add two new chips (e.g. id: 'theme', label: __('Theme', TEXT_DOMAIN), filter: { source: 'theme' } and id: 'custom', label: __('Custom', TEXT_DOMAIN), filter: { source: 'custom' }) so consumers can filter by template origin; keep the existing 'all', 'publish', 'draft' chips, preserve the readonly FilterChip[] return type, and ensure labels use the TEXT_DOMAIN translation helper.
67-75:⚠️ Potential issue | 🟠 MajorSurface fallback breadcrumbs and usage in each template row.
The navigator rows still stop at slug/source/status, so the D2 browser is missing two of its key affordances: visible fallback-chain context and the “Used by N” impact signal. Users still have to open each template to understand routing or blast radius.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/js/visual-editor/site-editor/templates-section.tsx` around lines 67 - 75, renderRow currently only shows slug/source/status; update it to also render the template's fallback chain and its usage count so each row shows routing context and impact. Inside renderRow (EntityRecord<'template'>), read the fallback list (e.g., entity.fallbackChain or entity.fallbacks) and render as breadcrumb text (joined with → or similar) and read the usage metric (e.g., entity.usedBy or entity.usageCount) and render "Used by N" with proper pluralization; ensure the EntityRecord fields you use are available and add them to the component props/state if necessary, and update the useCallback dependency array to include any external values you reference.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@resources/js/visual-editor/site-editor/site-editor-app.tsx`:
- Around line 132-134: The guard that calls registerCoreBlocks() should check
for specific core block registration rather than an empty registry; replace the
current getBlockTypes().length === 0 check with a presence check for required
core blocks (for example use getBlockType('core/paragraph') or search
getBlockTypes() for 'core/paragraph' and any other essential names) and call
registerCoreBlocks() if any required core block is missing. Locate the
conditional around getBlockTypes() and update it to detect missing core blocks
(e.g., missing result from getBlockType('core/paragraph') or absent names in
getBlockTypes()), then invoke registerCoreBlocks() when those core blocks are
not present.
In `@resources/js/visual-editor/site-editor/template-parts-section.tsx`:
- Around line 138-141: The panel currently only displays the slug as read-only;
update the TemplatePartDocumentPanel (using the TemplatePartDocumentPanelProps
and the onPatch callback) to render an editable slug input (or inline text
field) alongside existing title/area controls and call onPatch({ slug: newSlug
}) when the user changes it; ensure you preserve null-checks on entity:
TemplatePartRecord, keep validation/sanitization similar to title handling, and
fall back to the existing read-only code display when entity is null or the slug
is invalid.
In `@resources/js/visual-editor/site-editor/templates-section.tsx`:
- Around line 110-113: The template document panel currently renders the slug as
read-only even though TemplateDocumentPanelProps and onPatch support editing it;
update the slug field in the TemplateDocumentPanel component (using
entity/TemplateRecord for initial value) to be an editable input in the
inspector and wire its onChange to call onPatch({ slug: newValue }) so renames
propagate; do the same change for the other occurrence that renders slug (the
similar inspector block around the other slug rendering) to ensure both places
allow editing.
In `@resources/js/visual-editor/site-editor/use-entity-editor.ts`:
- Around line 268-275: The save path currently calls hydrateFromRecord(updated)
unconditionally, allowing a stale save response for entity A to overwrite the
editor when the user has navigated to entity B; add the same out-of-order
protection used in the load path (requestCounterRef or an equivalent
saveRequestCounterRef / currentEntityIdRef) to the save flow around updateEntity
and hydrateFromRecord so you only call hydrateFromRecord(updated) when the
response corresponds to the most recent save/request for the currently open
entity (e.g., increment a counter before calling updateEntity and check the
counter or that entityId still matches before hydrating and setting save
status/lastSavedAt).
- Around line 230-233: The patch function only writes overrides into
pendingPatch, so the rendered inputs still read from the original entity; fix by
exposing a merged draft entity that combines entity and pendingPatch (or
applying the overrides into entity) before returning from useEntityEditor:
compute a draftEntity (e.g., via useMemo) that spreads the current entity and
pendingPatch (respecting UpdatePayload fields like title.rendered, description,
area) and ensure inspector panels bind to draftEntity instead of raw entity;
keep patch (useCallback) updating setPendingPatch as-is but make the hook return
or expose the merged draftEntity so edits immediately reflect in the UI.
---
Duplicate comments:
In `@resources/js/visual-editor/site-editor/templates-section.tsx`:
- Around line 47-60: buildTemplateChips currently only returns publication-state
chips and must include navigator chips for Theme and Custom; update the
buildTemplateChips function to add two new chips (e.g. id: 'theme', label:
__('Theme', TEXT_DOMAIN), filter: { source: 'theme' } and id: 'custom', label:
__('Custom', TEXT_DOMAIN), filter: { source: 'custom' }) so consumers can filter
by template origin; keep the existing 'all', 'publish', 'draft' chips, preserve
the readonly FilterChip[] return type, and ensure labels use the TEXT_DOMAIN
translation helper.
- Around line 67-75: renderRow currently only shows slug/source/status; update
it to also render the template's fallback chain and its usage count so each row
shows routing context and impact. Inside renderRow (EntityRecord<'template'>),
read the fallback list (e.g., entity.fallbackChain or entity.fallbacks) and
render as breadcrumb text (joined with → or similar) and read the usage metric
(e.g., entity.usedBy or entity.usageCount) and render "Used by N" with proper
pluralization; ensure the EntityRecord fields you use are available and add them
to the component props/state if necessary, and update the useCallback dependency
array to include any external values you reference.
🪄 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: 93f62ad1-ad84-4185-a24e-191eafcb8694
📒 Files selected for processing (9)
resources/js/visual-editor/site-editor/entity-browser.cssresources/js/visual-editor/site-editor/entity-browser.tsxresources/js/visual-editor/site-editor/entity-editor-canvas.cssresources/js/visual-editor/site-editor/fallback-chain.tsresources/js/visual-editor/site-editor/site-editor-app.tsxresources/js/visual-editor/site-editor/slug.tsresources/js/visual-editor/site-editor/template-parts-section.tsxresources/js/visual-editor/site-editor/templates-section.tsxresources/js/visual-editor/site-editor/use-entity-editor.ts
✅ Files skipped from review due to trivial changes (2)
- resources/js/visual-editor/site-editor/entity-browser.css
- resources/js/visual-editor/site-editor/entity-editor-canvas.css
🚧 Files skipped from review as they are similar to previous changes (2)
- resources/js/visual-editor/site-editor/slug.ts
- resources/js/visual-editor/site-editor/entity-browser.tsx
Comment on lines
+138
to
+141
| export interface TemplatePartDocumentPanelProps { | ||
| entity: TemplatePartRecord | null; | ||
| onPatch: (overrides: { slug?: string; title?: string; area?: string }) => void; | ||
| } |
There was a problem hiding this comment.
Allow slug edits from the template-part inspector.
This panel accepts slug in onPatch, but only renders the current slug as read-only code. That misses the D2 settings requirement to edit entity-specific fields from the reused inspector.
Also applies to: 181-188
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@resources/js/visual-editor/site-editor/template-parts-section.tsx` around
lines 138 - 141, The panel currently only displays the slug as read-only; update
the TemplatePartDocumentPanel (using the TemplatePartDocumentPanelProps and the
onPatch callback) to render an editable slug input (or inline text field)
alongside existing title/area controls and call onPatch({ slug: newSlug }) when
the user changes it; ensure you preserve null-checks on entity:
TemplatePartRecord, keep validation/sanitization similar to title handling, and
fall back to the existing read-only code display when entity is null or the slug
is invalid.
Comment on lines
+110
to
+113
| export interface TemplateDocumentPanelProps { | ||
| entity: TemplateRecord | null; | ||
| onPatch: (overrides: { slug?: string; title?: string; description?: string }) => void; | ||
| } |
There was a problem hiding this comment.
Expose slug as an editable inspector field.
onPatch already supports slug, and the D2 settings scope explicitly includes slug editing, but the document panel renders slug as read-only text. That leaves no way to rename a template from the reused inspector.
Also applies to: 157-164
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@resources/js/visual-editor/site-editor/templates-section.tsx` around lines
110 - 113, The template document panel currently renders the slug as read-only
even though TemplateDocumentPanelProps and onPatch support editing it; update
the slug field in the TemplateDocumentPanel component (using
entity/TemplateRecord for initial value) to be an editable input in the
inspector and wire its onChange to call onPatch({ slug: newValue }) so renames
propagate; do the same change for the other occurrence that renders slug (the
similar inspector block around the other slug rendering) to ensure both places
allow editing.
…hes in UI - site-editor-app: probe `core/paragraph` specifically instead of an empty-registry check, so a host app that pre-registered an unrelated block doesn't silently skip core-block registration. - use-entity-editor: snapshot `requestCounterRef` at save start and bail out of `hydrateFromRecord` / status updates if the user navigated away mid-save. Without this, a save response for entity A could clobber the editor after the user opened entity B. - use-entity-editor: expose a merged `draftEntity` that overlays `pendingPatch` on top of the loaded record. Inspector inputs bind to `entity.title.rendered`, `entity.slug`, etc., so typing needs to show up in the UI — the committed record alone can't reflect in-flight edits. - Add tests covering both behaviors. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
resources/js/visual-editor/site-editor/__tests__/site-editor-app.test.tsx (1)
212-232: Add a sibling assertion for Template Parts navigator wiring.You now assert templates mounting (Line 212-218), but template parts has only mocks and no equivalent coverage. Adding one navigator/deep-link test for the parts stub would guard the second D2 entrypoint against regressions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/js/visual-editor/site-editor/__tests__/site-editor-app.test.tsx` around lines 212 - 232, Add a sibling test that mirrors the templates mounting check to cover the Template Parts entrypoint: render the app with renderApp(), then either assert directly that screen.getByTestId('ap-site-editor-stub-template-parts-browser') is in the document (like the templates test using 'ap-site-editor-stub-templates-browser') or simulate navigation by clicking screen.getByTestId('ap-site-editor-navigator-template-parts') and then assert the section outlet screen.getByTestId('ap-site-editor-section-outlet-template-parts') is present and contains the expected fallback text; place this new assertion/test next to the existing "mounts the D2 templates browser inside the navigator" test so the Template Parts wiring is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@resources/js/visual-editor/site-editor/use-entity-editor.ts`:
- Around line 151-164: When handling the entityId === null branch in the
use-entity-editor hook, increment/bump the requestCounterRef to invalidate any
in-flight load/save responses so older async callbacks cannot pass the
stale-request check; update the requestCounterRef (e.g.,
requestCounterRef.current++) inside the same block where you clear state
(setEntity, setBlocksState, setPendingPatch, etc.) so outstanding promises are
ignored after navigation/close.
---
Nitpick comments:
In `@resources/js/visual-editor/site-editor/__tests__/site-editor-app.test.tsx`:
- Around line 212-232: Add a sibling test that mirrors the templates mounting
check to cover the Template Parts entrypoint: render the app with renderApp(),
then either assert directly that
screen.getByTestId('ap-site-editor-stub-template-parts-browser') is in the
document (like the templates test using 'ap-site-editor-stub-templates-browser')
or simulate navigation by clicking
screen.getByTestId('ap-site-editor-navigator-template-parts') and then assert
the section outlet
screen.getByTestId('ap-site-editor-section-outlet-template-parts') is present
and contains the expected fallback text; place this new assertion/test next to
the existing "mounts the D2 templates browser inside the navigator" test so the
Template Parts wiring is covered.
🪄 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: 2887363e-5cc8-4dc6-95af-3599f77d4301
📒 Files selected for processing (4)
resources/js/visual-editor/site-editor/__tests__/site-editor-app.test.tsxresources/js/visual-editor/site-editor/__tests__/use-entity-editor.test.tsxresources/js/visual-editor/site-editor/site-editor-app.tsxresources/js/visual-editor/site-editor/use-entity-editor.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- resources/js/visual-editor/site-editor/tests/use-entity-editor.test.tsx
- resources/js/visual-editor/site-editor/site-editor-app.tsx
The null-entity branch of the load effect cleared component state without bumping requestCounterRef, so a save (or load) that resolved after the user closed the editor could slip past the stale-request guard and re-populate the just-cleared record. Bump the counter alongside the reset so late responses are always ignored. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
resources/js/visual-editor/site-editor/use-entity-editor.ts (1)
171-175:⚠️ Potential issue | 🔴 CriticalReset the working copy when switching to a new entity.
Lines 171-175 only flip the load state, so the previous entity’s
blocks,pendingPatch, dirty flag, and save metadata stay live until the fetch completes. During that window,save()can PUT the old draft to the newentityId, and a failed fetch leaves stale editor state behind. Clear the working copy here as well, not just in theentityId === nullbranch.💡 Suggested fix
const requestId = ++requestCounterRef.current; + committedBlocksRef.current = []; + setEntity(null); + setBlocksState([]); + setPendingPatch({}); + setIsDirty(false); + setSaveStatus('idle'); + setSaveErrorMessage(null); + setValidationErrors(null); + setLastSavedAt(null); setLoadStatus('loading'); setLoadErrorMessage(null);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/js/visual-editor/site-editor/use-entity-editor.ts` around lines 171 - 175, When starting a new load (the branch that increments requestId via requestCounterRef.current and calls setLoadStatus('loading') / setLoadErrorMessage(null)), also reset the editor's working copy state so remnants of the previous entity can't be saved to the new entity: clear blocks (e.g. setBlocks([]) or equivalent), clear pendingPatch (setPendingPatch(null)), clear the dirty flag (setIsDirty(false)), and clear any save metadata (lastSavedAt/saveError/saveInProgress refs) so save() cannot PUT old draft to the new entityId; do this in the same place (not only in the entityId === null branch) before issuing the fetch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@resources/js/visual-editor/site-editor/use-entity-editor.ts`:
- Around line 317-323: The current code snapshots requestCounterRef.current so
concurrent saves for the same entity can get the same requestId; instead
reserve/allocate a new unique request id when starting a save by incrementing
requestCounterRef.current (or calling an allocator) and store that incremented
value in requestId, then keep using isStale() to compare
requestCounterRef.current !== requestId; update the save/start logic around
requestCounterRef and isStale so each save gets a unique requestId and late
responses are rejected.
---
Duplicate comments:
In `@resources/js/visual-editor/site-editor/use-entity-editor.ts`:
- Around line 171-175: When starting a new load (the branch that increments
requestId via requestCounterRef.current and calls setLoadStatus('loading') /
setLoadErrorMessage(null)), also reset the editor's working copy state so
remnants of the previous entity can't be saved to the new entity: clear blocks
(e.g. setBlocks([]) or equivalent), clear pendingPatch (setPendingPatch(null)),
clear the dirty flag (setIsDirty(false)), and clear any save metadata
(lastSavedAt/saveError/saveInProgress refs) so save() cannot PUT old draft to
the new entityId; do this in the same place (not only in the entityId === null
branch) before issuing the fetch.
🪄 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: e807b55c-a9a7-49da-8ae7-f6c8a2542822
📒 Files selected for processing (2)
resources/js/visual-editor/site-editor/__tests__/use-entity-editor.test.tsxresources/js/visual-editor/site-editor/use-entity-editor.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- resources/js/visual-editor/site-editor/tests/use-entity-editor.test.tsx
…n't clobber each other Snapshotting requestCounterRef.current gave two concurrent saves the same id, so a slower save-1 resolving after save-2 would pass the stale-request guard and hydrate the editor with save-1's (outdated) response on top of save-2's authoritative result. Increment the counter when starting a save so each save has a unique id and any save that resolves after a newer one is correctly rejected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@resources/js/visual-editor/site-editor/use-entity-editor.ts`:
- Around line 173-175: When starting a new entity load you only reset
load-specific state; also reset all editor/save metadata so state from entity A
doesn't leak into entity B: alongside setLoadStatus('loading') and
setLoadErrorMessage(null) call the corresponding save/editor reset setters (e.g.
setSaveStatus('idle' or null), setSaveErrorMessage(null), setLastSavedAt(null))
and clear any dirty/validation state via the editor dirty/validation setters
(e.g. setIsDirty(false), setValidationErrors([]) or resetEditorValidation).
Place these resets in the same branch that runs when initiating a new fetch so
they always run when switching entities and before handling load failures.
🪄 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: 87223d9b-f787-4d20-901e-fec808a43d8a
📒 Files selected for processing (2)
resources/js/visual-editor/site-editor/__tests__/use-entity-editor.test.tsxresources/js/visual-editor/site-editor/use-entity-editor.ts
…om clobbering editor state Two related hazards in use-entity-editor: - Switching entity A → B only reset loadStatus / loadErrorMessage, so A's saveStatus, lastSavedAt, validationErrors, isDirty, and even the pendingPatch stayed on screen until B's fetch resolved. Extract a resetEditorState helper and call it at the start of every entity transition (both null-out and new-entity paths) so B is always loaded from a clean slate. - Data-loss path on slow saves: the existing guard only rejected responses invalidated by a *newer* request. If the user kept typing after clicking save, the original save's response still passed the guard and hydrateFromRecord would overwrite the post-save edits + clear isDirty. Add an editVersionRef bumped by setBlocks / patch / reset; snapshot it at save start; if it moves by the time the response arrives, confirm the save (status + timestamp) but skip the hydrate so the user's in-flight edits survive and isDirty stays true. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
Author
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
github-project-automation
Bot
moved this from In Review
to Done
in ArtisanPack UI Overview
This was referenced Apr 24, 2026
14 of 27 tasks
19 of 20 tasks
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
Wires the Templates and Template Parts sections into the D1 site-editor shell: list views in the navigator-outlet, open-in-canvas editing, save via the C1/C2 REST endpoints, and a reused A1 inspector with kind-specific Document panels. Implements design brief §3.4, §3.5, §5.1, §5.5 and principles P7 (visible fallback chains) + P9 (destructive confirmations).
Closes: #369
Type of Change
Related Issue
Issue: #369
Motivation and Context
C1 (#357) and C2 (#358) landed the REST endpoints. D1 (#368) landed the shell. D2 is the surface users actually touch — without it, templates and parts exist on disk but can't be edited.
Changes Made
Frontend —
resources/js/visual-editor/site-editor/:api-client.ts— typed REST client for templates + parts, handling CSRF, 422 validation-error extraction, and the paginated list envelopeuse-entity-list.ts/use-entity-editor.ts— list + single-entity hooks with dirty tracking, save/load status, and refresh signalsentity-browser.tsx— shared navigator-sub-panel list with filter chips and row-level keyboard navigation (roving tabindex)entity-editor-canvas.tsx—BlockList/WritingFlow/BlockToolseditor surface with screen-reader announcemententity-editor.tsx— stitches canvas + inspector, bubbles save state to the top bartemplates-section.tsx/template-parts-section.tsx— kind-specific browsers, Document panels, and create-new dialogs (slug picker with fallback-chain preview for templates, area selector for parts)create-entity-dialog.tsx— focus-trapping dialog shell with accessible form controls + custom-styled native selectfallback-chain.ts— pure client-side mirror of the PHP resolverslug.ts— live input normalizer (lowercase + space→dash)Shell wiring (
site-editor-app.tsx):core/template-part,core/post-*,core/site-*,core/query,core/navigation, etc.) so seeded templates that embed them don't trip the block-crash boundary. E4 re-enables them with real core-data selectors.apiBase/themefrom the blade through the React mount.Backend:
routes/web.php— wraps the SPA mount in thewebmiddleware group so Laravel starts a session and the CSRF token in the blade matches the session when the API mutation requests land.resources/views/site-editor/index.blade.php— passesdata-api-base,data-theme, and a<meta name="csrf-token">tag to the SPA.How Has This Been Tested?
Testing Environment:
Tests Performed:
core/template-partblocks render as "block not registered" placeholders (expected for D2)./visual-editor/api/templates/{id}; status cycles saving → saved; dirty clears.my-slug!in real time.Accessibility Tests Run
aria-liveannouncements in code; not run in a real screen reader)Details:
aria-live="polite""Editing {title}" announcement on open.:focus-visiblefocus rings.Tests Added
Test details:
79 site-editor JS tests total (up from 51 pre-D2):
api-client.test.ts— list/fetch/create/update/delete + CSRF + 422 extraction (8 tests)use-entity-editor.test.tsx— load, dirty tracking, save success + validation error, idle-on-null (5 tests)entity-browser.test.tsx— loading/ready/empty/error states, row open, chip filter refetch (6 tests)entity-editor.test.tsx— inactive state, load + announcement, dirty flag, save clears dirty (4 tests)create-entity-dialog.test.tsx— template and template-part dialog submit paths + validation error display (4 tests)fallback-chain.test.ts— PHP resolver parity (7 tests)main.test.tsx+site-editor-app.test.tsxfor the new requiredapiBaseprop and D2 module stubs.All 626 JS tests pass (
npm test); all 341 PHP tests pass (vendor/bin/pest);npm run buildsucceeds.Documentation
Documentation details:
Every new module has a file-level docblock explaining its role and how it connects to the D0 design brief / D1 shell. Non-obvious decisions (e.g.,
appearance: noneon the select,!importanton WordPress-overridden resets, the deliberateD2_DISABLED_BLOCKSlist mirroring the PHP config) are commented inline.Screenshots
N/A — the UX matches the design brief's navigator-sub-panel list + canvas-edit + right-rail inspector layout.
Pre-Submission Checklist
release/1.0, 3 passes)Summary by CodeRabbit
New Features
Bug Fixes
Tests