Repository navigation
feat: composed-view ribbon and site-editor slug deep links - #696
Conversation
Adds the locked-canvas ribbon to the post editor's composed view and the query-string deep-link surface it targets in the site editor. The ribbon renders inside the canvas iframe (not the editor chrome) as a sticky bar naming the resolved template, with a single Edit template button that opens the site editor in a new tab. It is the only editing affordance for template chrome in v1 — individual template parts get no per-part badge. Composing against the built-in fallback template shows the name but hides the CTA, since that tree is a client-side constant with no site-editor record behind it. The site editor now parses `?entity=template&slug=…` on mount, resolves the slug to an entity id through the templates list endpoint, and navigates to that template's editor view. Path routing addresses entities by id and the applied-template endpoint returns only a slug, so the lookup keeps the router's single id-shaped contract instead of teaching it a second addressing mode. Handling is additive: no query string behaves exactly as before, and an unknown entity is a no-op rather than a throw. An unresolvable slug or a failed lookup lands on the Templates index with a toast naming the slug. Closes #623 Closes #625 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 51 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR adds a composed-view ribbon with a template edit link. It adds site-editor deep-link parsing, slug resolution, navigation, error toasts, URL canonicalization, iframe styling, tests, and documentation. ChangesTemplate deep-link flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant EditorCanvas
participant ComposedViewRibbon
participant SiteEditorApp
participant useSiteEditorDeepLink
participant TemplatesAPI
EditorCanvas->>ComposedViewRibbon: Pass template name and slug
ComposedViewRibbon->>SiteEditorApp: Open template deep-link in new tab
SiteEditorApp->>useSiteEditorDeepLink: Parse query parameters
useSiteEditorDeepLink->>TemplatesAPI: Resolve template slug
TemplatesAPI-->>useSiteEditorDeepLink: Return template entities
useSiteEditorDeepLink->>SiteEditorApp: Navigate to template editor or index
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 |
`site-editor-app.test.tsx` failed all 18 of its tests on `window.localStorage.clear()` with "Cannot read properties of undefined". jsdom implements Web Storage correctly; the global was being shadowed before the suite ever ran. vitest's `getWindowKeys()` filters jsdom's window properties with `if (k in global) return KEYS.includes(k)` — a key already present on Node's `globalThis` is only copied when it appears in vitest's own allow-list, and `localStorage` does not. Node 22+ ships an experimental Web Storage global whose accessor returns `undefined` unless the process was started with `--localstorage-file`, so on those versions the key is present, vitest skips it, and Node's inert stub shadows jsdom's working implementation: `'localStorage' in window` is true while `window.localStorage` is undefined. That is why this surfaced without a code change — on older Node the key is absent, vitest copies jsdom's version, and the suite passes. Install an in-memory `Storage` for `localStorage` and `sessionStorage` in the shared test setup, but only when the global is genuinely unusable, so a working jsdom implementation is never replaced. Defined via `defineProperty` rather than assignment because the property Node leaves behind is an accessor whose setter would swallow a plain write. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
github-project-automation
Bot
moved this from In Review
to Done
in ArtisanPack UI Overview
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@docs/site-editor.md`:
- Around line 158-171: Align the site editor documentation’s route identifier
model by making the deep-link example and canonical path use the same identifier
type. Update the `/visual-editor/site?entity=template&slug=single` example and
its resolution description, or the `/visual-editor/site/templates/12` route, so
both consistently describe either database IDs or slugs.
- Around line 198-204: Thread the configured route base through the editor mount
configuration: expose routeBase in EditorApp and EditorAppShell, pass it from
EditorCanvas to ComposerViewRibbon as siteEditorRouteBase, and ensure
ComposedViewRibbon/buildTemplateDeepLink uses that value instead of the package
default when provided. Preserve the default route base when no override is
configured.
In `@resources/js/visual-editor/site-editor/deep-link.ts`:
- Around line 95-98: Update the route-base normalization in the deep-link helper
so trimming trailing slashes does not turn the root route "/" into an empty
string; preserve "/" when routeBase is root, while retaining current
normalization for other bases. Add a test covering the root route and asserting
the generated URL starts with "/?entity=template".
In `@resources/js/visual-editor/site-editor/use-deep-link.ts`:
- Around line 69-76: Update useSiteEditorDeepLink to capture navigate, toast,
apiConfig, and search in one mount-time useRef snapshot instead of assigning to
separate refs during render. Ensure the cleanup-aware effect and pending
resolver read this stable committed snapshot, eliminating render-time ref
mutations.
🪄 Autofix
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: 5908ef6c-cde8-4319-991a-611ad8a7dd75
📒 Files selected for processing (15)
docs/site-editor.mdresources/js/visual-editor/editor/__tests__/canvas-styles.test.tsresources/js/visual-editor/editor/__tests__/editor-canvas.test.tsxresources/js/visual-editor/editor/canvas-styles.tsresources/js/visual-editor/editor/composed-view/__tests__/composed-view-ribbon.test.tsxresources/js/visual-editor/editor/composed-view/composed-view-ribbon.cssresources/js/visual-editor/editor/composed-view/composed-view-ribbon.tsxresources/js/visual-editor/editor/composed-view/index.tsresources/js/visual-editor/editor/editor-app.tsxresources/js/visual-editor/editor/editor-canvas.tsxresources/js/visual-editor/site-editor/__tests__/deep-link.test.tsresources/js/visual-editor/site-editor/__tests__/use-deep-link.test.tsxresources/js/visual-editor/site-editor/deep-link.tsresources/js/visual-editor/site-editor/site-editor-app.tsxresources/js/visual-editor/site-editor/use-deep-link.ts
| Path routes address entities by database id. Callers outside the SPA | ||
| usually do not have one — the post editor's composed view, for instance, | ||
| knows only the slug of the template it composed against. For those, the | ||
| site editor accepts a query-string deep link on the mount URL: | ||
|
|
||
| ```text | ||
| /visual-editor/site?entity=template&slug=single | ||
| ``` | ||
|
|
||
| On mount the SPA parses the query string, resolves the slug through the | ||
| templates list endpoint, and navigates to that template's editor view. | ||
| The `pushState` that follows rewrites the address bar to the canonical | ||
| path route (`/visual-editor/site/templates/12`), so the query-string form | ||
| is an entry point rather than a URL the user is left holding. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use one identifier model for path routes.
Lines 158-171 define path routes as database-ID routes and show /visual-editor/site/templates/12. Lines 146-150 show slug-like values such as single. Update the examples or the contract so both describe the same route identifier.
🤖 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 `@docs/site-editor.md` around lines 158 - 171, Align the site editor
documentation’s route identifier model by making the deep-link example and
canonical path use the same identifier type. Update the
`/visual-editor/site?entity=template&slug=single` example and its resolution
description, or the `/visual-editor/site/templates/12` route, so both
consistently describe either database IDs or slugs.
| `buildTemplateDeepLink()` defaults to the package's own mount path | ||
| (`/visual-editor/site`) and takes the route base as an optional second | ||
| argument. The composed-view ribbon currently relies on that default: the | ||
| post editor has no way to discover a `data-route-base` the host may have | ||
| overridden on the site-editor mount element. A host that mounts the site | ||
| editor at a non-default path will get a ribbon CTA pointing at the | ||
| default path until that value is threaded through to the post editor. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline resources/js/visual-editor/editor/editor-app.tsx --view expanded
ast-grep outline resources/js/visual-editor/editor/editor-canvas.tsx --view expanded
ast-grep outline resources/js/visual-editor/editor/composed-view/composed-view-ribbon.tsx --view expanded
rg -n -C 5 --glob '*.{ts,tsx}' \
'siteEditorRouteBase|routeBase|buildTemplateDeepLink|ComposedViewRibbon' \
resources/js/visual-editorRepository: ArtisanPack-UI/visual-editor
Length of output: 50384
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== editor-app outline =="
ast-grep outline resources/js/visual-editor/editor/editor-app.tsx --view expanded
echo "== editor-canvas relevant lines =="
sed -n '1,220p' resources/js/visual-editor/editor/editor-canvas.tsx
echo "== composed-view-ribbon relevant lines =="
sed -n '1,130p' resources/js/visual-editor/editor/composed-view/composed-view-ribbon.tsx
echo "== focused call sites for ComposedViewRibbon and route/data propagation =="
rg -n -C 4 --glob '*.{ts,tsx}' 'EditorApp|<EditorApp|EditorAppShell|<EditorAppShell|<ComposedViewRibbon|siteEditorRouteBase|data-route-base|routeBase' resources/js/visual-editor/editor resources/js/visual-editor/site-editor || true
echo "== visual-editor mount / post-editor related files =="
fd -a '.*post.*|.*visual.*|editor-app' resources/js/visual-editor | sed 's#^\./##'Repository: ArtisanPack-UI/visual-editor
Length of output: 50384
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== editor main config/readers =="
sed -n '80,180p' resources/js/visual-editor/editor/main.tsx
sed -n '300,460p' resources/js/visual-editor/editor/main.tsx
echo "== EditorCanvas / EditorApp rendering =="
sed -n '365,460p' resources/js/visual-editor/editor/editor-app.tsx
echo "== CanvasChrome / EditorCanvas call sites =="
rg -n -C 3 --glob '*.{ts,tsx}' 'type CanvasChrome|interface CanvasChrome|<EditorCanvas|EditorCanvas\(' resources/js/visual-editor/editor resources/js/visual-editor/site-editor
echo "== routeBase usages in editor/main/editor-app with file names =="
for f in $(rg -l --glob '*.{ts,tsx}' 'routeBase|data-route' resources/js/visual-editor/editor resources/js/visual-editor/site-editor); do
echo "--- $f"
rg -n -C 2 'routeBase|data-route' "$f"
done
echo "== docs limitation text =="
sed -n '190,210p' docs/site-editor.mdRepository: ArtisanPack-UI/visual-editor
Length of output: 34656
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
paths = [
Path('resources/js/visual-editor/editor/editor-app.tsx'),
Path('resources/js/visual-editor/editor/editor-canvas.tsx'),
Path('resources/js/visual-editor/editor/composed-view/composed-view- ribbon.tsx'),
Path('resources/js/visual-editor/editor/main.tsx'),
]
for p in paths:
print('path exists', p, p.exists())
files = {
'editor-app.tsx': Path('resources/js/visual-editor/editor/editor-app.tsx').read_text(),
'editor-canvas.tsx': Path('resources/js/visual-editor/editor/editor-canvas.tsx').read_text(),
'composed-view-ribbon.tsx': Path('resources/js/visual-editor/editor/composed-view/composed-view-ribbon.tsx').read_text(),
'main.tsx': Path('resources/js/visual-editor/editor/main.tsx').read_text(),
}
props = re.findall(r'^export interface EditorAppProps \{(?P<body>.*?)^\}', files['editor-app.tsx'], re.M|re.S)
prop_names = re.findall(r'^\s*(?:export )?([A-Za-z_$][\w$]*)\?', props[0], re.M) if props else []
print('EditorAppProps has apiBase and routeBase?', 'apiBase' in prop_names, 'routeBase' in prop_names)
print('EditorAppProps routeBase mentions data-route-base/config?', any('routeBase' in prop_names and ('data-route' in props[0] or 'route-base' in props[0])) for _ in [1])
print('EditorCanvasProps has routeBase?', 'routeBase' in files['editor-canvas.tsx'])
print('EditorAppShell routeBase parsed/used?', bool(re.search(r'routeBase', files['editor-app.tsx'], re.I)))
print('EditorAppShell EditorCanvas chrome props routeBase?', bool(re.search(r'<EditorCanvas[\s\S]{0,2500}routeBase', files['editor-app.tsx'])))
print('MountConfig has routeBase?', 'routeBase:' in re.search(r'export interface MountConfig \{(?P<body>.*?)\n\}', files['main.tsx'], re.S).group(0))
PYRepository: ArtisanPack-UI/visual-editor
Length of output: 496
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import re
from pathlib import Path
files = {
'editor-app': Path('resources/js/visual-editor/editor/editor-app.tsx').read_text(),
'editor-canvas': Path('resources/js/visual-editor/editor/editor-canvas.tsx').read_text(),
'composed-view-ribbon': Path('resources/js/visual-editor/editor/composed-view/composed-view-ribbon.tsx').read_text(),
'main': Path('resources/js/visual-editor/editor/main.tsx').read_text(),
}
app_props = re.search(r'export interface EditorAppProps \{(?P<body>.*?)\n\}', files['editor-app'], re.S).group('body')
app_props_names = re.findall(r'^\s*([A-Za-z_$][\w$]*)\?', app_props, re.M)
print('EditorAppProps contains routeBase:', 'routeBase' in app_props_names)
print('EditorAppProps contains apiBase:', 'apiBase' in app_props_names)
print('EditorCanvasProps contains routeBase:', bool(re.search(r'\brouteBase\b', files['editor-canvas'])))
print('EditorAppShell EditorCanvas props contain routeBase:', bool(re.search(r'<EditorCanvas(?:[\\s\\S]*?)routeBase', files['editor-app'])))
editor_app_route_mentions = re.findall(r'\brouteBase\b', files['editor-app'], re.I)
print('EditorApp routeBase mentions:', len(editor_app_route_mentions))
mount_config_lines = re.search(r'export interface MountConfig \{(?P<body>.*?)\n\}', files['main'], re.S).group('body')
print('Editor mount MountConfig routeBase:', 'routeBase' in mount_config_lines)
print('Editor mount MountConfig data-route/base text:', 'data-route' in mount_config_lines or 'route-base' in mount_config_lines)
PY
echo "== EditorApp Shell / EditorCanvas call from editor-app =="
sed -n '1420,1475p' resources/js/visual-editor/editor/editor-app.tsxRepository: ArtisanPack-UI/visual-editor
Length of output: 2795
Thread the configured route base through the post editor.
ComposerViewRibbon accepts siteEditorRouteBase, but EditorCanvas never passes it, and EditorApp/EditorAppShell do not expose routeBase at all. Hosts that mount the site editor at /data-ap-site-editor/directive/mount?data-route-base=%2Fcustom%2Fsite-editor still get the ribbon Edit template ↗ CTA built to the package default unless the configured value is added to the mount config and forwarded through to ComposedViewRibbon.
🤖 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 `@docs/site-editor.md` around lines 198 - 204, Thread the configured route base
through the editor mount configuration: expose routeBase in EditorApp and
EditorAppShell, pass it from EditorCanvas to ComposerViewRibbon as
siteEditorRouteBase, and ensure ComposedViewRibbon/buildTemplateDeepLink uses
that value instead of the package default when provided. Preserve the default
route base when no override is configured.
| const normalizedBase = routeBase.replace(/\/+$/, ''); | ||
| const query = new URLSearchParams({ entity: 'template', slug }); | ||
|
|
||
| return `${normalizedBase}?${query.toString()}`; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep / as a route base.
When routeBase is /, Line 95 produces an empty base. The helper emits ?entity=template... instead of /?entity=template.... Preserve / and add a root-route test.
Proposed fix
- const normalizedBase = routeBase.replace(/\/+$/, '');
+ const normalizedBase =
+ routeBase === '/' ? '/' : routeBase.replace(/\/+$/, '');📝 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.
| const normalizedBase = routeBase.replace(/\/+$/, ''); | |
| const query = new URLSearchParams({ entity: 'template', slug }); | |
| return `${normalizedBase}?${query.toString()}`; | |
| const normalizedBase = | |
| routeBase === '/' ? '/' : routeBase.replace(/\/+$/, ''); | |
| const query = new URLSearchParams({ entity: 'template', slug }); | |
| return `${normalizedBase}?${query.toString()}`; |
🤖 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/site-editor/deep-link.ts` around lines 95 - 98,
Update the route-base normalization in the deep-link helper so trimming trailing
slashes does not turn the root route "/" into an empty string; preserve "/" when
routeBase is root, while retaining current normalization for other bases. Add a
test covering the root route and asserting the generated URL starts with
"/?entity=template".
| const navigateRef = useRef(navigate); | ||
| navigateRef.current = navigate; | ||
| const toastRef = useRef(toast); | ||
| toastRef.current = toast; | ||
| const apiConfigRef = useRef(apiConfig); | ||
| apiConfigRef.current = apiConfig; | ||
| const searchRef = useRef(search); | ||
| searchRef.current = search; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline resources/js/visual-editor/site-editor/use-deep-link.ts \
--match useSiteEditorDeepLink --view expanded
# Expect no render-time assignments after the fix.
rg -n -C 3 '[A-Za-z]+Ref\.current\s*=' \
resources/js/visual-editor/site-editor/use-deep-link.ts
# Locate the deferred-lookup regression coverage.
rg -n -C 4 'useSiteEditorDeepLink|rerender|mockResolvedValue' \
resources/js/visual-editor/site-editor/__tests__/use-deep-link.test.tsxRepository: ArtisanPack-UI/visual-editor
Length of output: 3358
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,170p' resources/js/visual-editor/site-editor/use-deep-link.ts
sed -n '120,165p' resources/js/visual-editor/site-editor/__tests__/use-deep-link.test.tsxRepository: ArtisanPack-UI/visual-editor
Length of output: 7008
🌐 Web query:
React useRef mutating during render concurrent rendering discouraged
💡 Result:
In React, reading or writing to ref.current during the render phase is explicitly discouraged and considered unsafe [1][2][3]. This practice is strongly cautioned against, particularly in the context of concurrent rendering, because it makes component behavior unpredictable and unreliable [1][2][4][5]. The primary reasons and rules regarding this are: 1. Unpredictability in Concurrent Mode: During concurrent rendering, React may invoke your component function multiple times before committing the result to the screen [4][5]. If you perform side effects—such as writing to a ref—during these render passes, your component may behave differently than expected, or ref values may become inconsistent with the rendered output [4][5]. 2. Purity and Side Effects: React components should ideally be pure functions of their props and state [4]. Writing to a ref during render is a side effect. Since React does not guarantee how many times a component will render, any side effect occurring during this phase can lead to bugs that are difficult to reproduce and debug [1][2][4]. 3. The Exception for Initialization: The only widely accepted exception is using a ref for lazy initialization, such as the pattern if (!ref.current) ref.current = new Thing() [1][2][6][3]. This is considered safe because the result is deterministic and occurs only once, essentially treating the ref as a stable object container rather than a dynamic source of state during rendering [2][6][3]. 4. Recommended Alternatives: If you need to store or read information that affects rendering, you should use state (useState) instead [1][2]. State is managed by React, follows the "snapshot" model of rendering, and ensures that your UI remains consistent across re-renders [1][2][3]. If you must perform a side effect based on a value, use useEffect or useLayoutEffect, which are designed to run only after the commit phase [2][4][7]. In summary, treating ref.current as a source of truth to be read or written during the render phase violates the component lifecycle model and can lead to issues in any React application, especially those leveraging concurrent features [1][2][4][5]. Top Results: [1][2][4][5]
Citations:
- 1: https://react.dev/learn/referencing-values-with-refs
- 2: https://react.dev/reference/react/useRef
- 3: https://he.react.dev/learn/referencing-values-with-refs
- 4: https://blog.axlight.com/posts/how-to-properly-use-the-react-useref-hook-in-concurrent-mode/
- 5: https://stackoverflow.com/questions/68025789/is-it-safe-to-change-a-refs-value-during-render-instead-of-in-useeffect
- 6: react/react@c59c3dfe5
- 7: Add ESLint Rule for Safe Ref Usage react/react#19095
Capture the mount-time context without render-time ref writes.
useSiteEditorDeepLink reads these refs in a cleanup-aware effect, so navigate/toast mutations during a discarded render can leave the pending resolver calling a callback from uncommitted work. Store the mount-time snapshot instead, such as a single useRef({ navigate, toast, apiConfig, search }) created in the hook’s render phase.
🧰 Tools
🪛 React Doctor (0.9.3)
[error] 70-70: This ref is mutated during render. React can replay or discard render work, so the mutation can leak from UI that never commits.
Move ref writes into an event handler or effect. Render must stay pure because React can replay or discard it. The predictable null-guarded lazy initialization pattern remains supported.
(no-ref-current-in-render)
[error] 72-72: This ref is mutated during render. React can replay or discard render work, so the mutation can leak from UI that never commits.
Move ref writes into an event handler or effect. Render must stay pure because React can replay or discard it. The predictable null-guarded lazy initialization pattern remains supported.
(no-ref-current-in-render)
[error] 74-74: This ref is mutated during render. React can replay or discard render work, so the mutation can leak from UI that never commits.
Move ref writes into an event handler or effect. Render must stay pure because React can replay or discard it. The predictable null-guarded lazy initialization pattern remains supported.
(no-ref-current-in-render)
[error] 76-76: This ref is mutated during render. React can replay or discard render work, so the mutation can leak from UI that never commits.
Move ref writes into an event handler or effect. Render must stay pure because React can replay or discard it. The predictable null-guarded lazy initialization pattern remains supported.
(no-ref-current-in-render)
🤖 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/site-editor/use-deep-link.ts` around lines 69 -
76, Update useSiteEditorDeepLink to capture navigate, toast, apiConfig, and
search in one mount-time useRef snapshot instead of assigning to separate refs
during render. Ensure the cleanup-aware effect and pending resolver read this
stable committed snapshot, eliminating render-time ref mutations.
Source: Linters/SAST tools
Description
Adds the composed view's locked-canvas ribbon (#623) and the site-editor
deep-link surface its CTA targets (#625). Shipped together because the
ribbon's Edit template ↗ button is the deep link's first consumer —
splitting them would land a button pointing at a route that doesn't
resolve yet.
Closes: #623, #625
Type of Change
Related Issue
Issue: #623, #625 (both under parent #618, milestone v1.6)
Motivation and Context
The composed view renders a template's header and footer as inert blocks
around the author's content. Until now nothing on that surface explained
what the template was or offered a way to go edit it — an author reading
a header they can't click had no next step.
The ribbon is that step, and it is deliberately the only one: per-part
chrome would put four or five competing controls on a surface whose whole
point is to read like the finished page.
The CTA needed somewhere to land. Path routes address entities by id, and
the applied-template endpoint returns only a slug, so the site editor
gained a slug-keyed query-string entry point rather than the router
learning a second addressing mode.
Changes Made
#623 — composed-view ribbon
editor/composed-view/composed-view-ribbon.tsx+.css— sticky bar rendered inside the canvas iframe so it scrolls with the preview and reads as part of the page, not another editor toolbareditor/canvas-styles.ts— ribbon CSS injected?inlineinto the iframe (theblocks/*/*.cssglob doesn't reachcomposed-view/)editor/editor-canvas.tsx—CanvasChromegainedtemplateName/templateSlug; the ribbon mounts only when chrome is present, so bare-content mode never renders iteditor/editor-app.tsx— forwards the name/slug it had already resolved#625 — site-editor deep link
site-editor/deep-link.ts—parseDeepLink()/buildTemplateDeepLink(), both pure. The ribbon builds its href with the builder so the two sides of the contract move togethersite-editor/use-deep-link.ts— resolves slug → id via the templates list endpoint on mount, then navigates. Unknown slug or failed lookup lands on the Templates index with a toastsite-editor/site-editor-app.tsx— wrapped inToastProvider(mirrors the post editor) and calls the hookdocs/site-editor.md— new "Deep links by slug" section documenting the parameter table and guaranteesHow Has This Been Tested?
Testing Environment:
feature/templates-preview)Tests Performed:
/visual-editor/site/templates/{id}?entity=template&slug=does-not-exist— Templates index plus the not-found toast/visual-editor/site— unchanged from beforeAccessibility Tests Run
Details: The ribbon is a
role="region"landmark labelled"Composed-view controls", so screen-reader users can reach it by landmark
rather than having to scroll the canvas. The CTA is a real
<a>— keyboardfocusable in document order, with a visible
:focus-visibleoutline drawnfrom the theme's focus-width token. Colours resolve from the ArtisanPack UI
--color-base-*tokens (with hex fallbacks for non-DaisyUI hosts) so theribbon inherits whatever contrast the active theme guarantees. Not yet
verified with an actual screen reader.
Tests Added
Test details:
composed-view-ribbon.test.tsx(6) — name rendering, landmark labelling, deep-link href, slug encoding, host route-base override, CTA hidden on the fallbackeditor-canvas.test.tsx(+3) — ribbon absent in bare-content mode, mounted inside theBlockCanvasstub rather than beside it, CTA dropped on the fallbackdeep-link.test.ts(16) — parse/build round-trip plus every rejected shape (unknown entity, blank slug, no entity, …)use-deep-link.test.tsx(10) — resolve-and-navigate, explicit slug matching when the host ignores the filter, unknown-slug and lookup-failure toasts,entity_idshort-circuit, no-op cases, once-per-mountcanvas-styles.test.ts— cascade-order assertion updated for the appended ribbon sheetnpm test: 326 files, 2996 tests, all passing.This branch also fixes a test-harness break it inherited. All 18 tests in
site-editor-app.test.tsxwere failing onwindow.localStorage.clear(),on this branch and on
feature/templates-previewalike. Root cause wasneither jsdom nor this repo: vitest's
getWindowKeys()only copies ajsdom window property onto the test global if the key is absent from
Node's
globalThisor present in vitest's own allow-list. Node 22+ shipsan experimental Web Storage global that returns
undefinedwithout--localstorage-file, andlocalStorageis not on vitest's list — soNode's inert stub shadowed jsdom's working implementation
(
'localStorage' in windowtrue,window.localStorageundefined). Itsurfaced on a Node upgrade rather than a code change.
test-setup.tsnow installs an in-memoryStorageforlocalStorageand
sessionStorage, but only when the global is genuinely unusable, soa working jsdom implementation is never replaced. That restores real
coverage of
SiteEditorApp— including this PR'sToastProviderwrapperand the deep-link hook — rather than leaving it silently unexercised.
Also verified with a real
npm run buildthat the?inlineribbonstylesheet lands in the editor chunk.
Documentation
Documentation details:
docs/site-editor.mdgained a "Deep links byslug" section under URL routing — the parameter table, the additive /
unknown-entity-no-op / access-gate-unchanged guarantees, and a pointer at
buildTemplateDeepLink()as the supported way to construct links.It also records a known limitation surfaced during review: the ribbon's
CTA uses the default
/visual-editor/sitebase because the post editorhas no way to discover a
data-route-basea host may have overridden onthe site-editor mount element.
buildTemplateDeepLink()and the ribbonboth already accept the base as a parameter, so threading the host value
through is the follow-up — left out of scope here rather than guessed at.
Pre-Submission Checklist