the builder draws the sections it finds
An empty section renders as nothing. That is correct on the public site and useless in an editor: somebody who has just chosen "two columns" sees a blank page and has to take it on trust. So the canvas outlines every band it finds, names it, says its shape, and offers Delete — and outlines the columns inside it, because a one-column band and a two-column band are identical until something is in them, which is exactly when it is too late to notice the wrong choice. The site provides three attributes and no behaviour: data-page-empty, data-section-id, data-column. Markers rather than classes, so they cannot be mistaken for styling, cannot be purged by a build that has never heard of this file, and read as what they are. That is the whole contract — one attribute on the public site buys the entire editor. Section names and layouts are FETCHED, not read out of the DOM. The markup says which row a band came from and nothing else; inferring the layout from its classes would be the admin reading the site's stylesheet, which is the coupling all of this exists to avoid. ── The race, and why the fix is not another ref ── Creating a section starts two things that do not finish in a fixed order: the frame fetches a new document, and the panel fetches the sections to draw on it. Painting only on load meant painting with whatever had arrived by then, which after a create was usually the data from before it existed. So SitePreview re-runs the injection when EITHER moves — `loaded` is state, `onDocument` is a dependency, and whichever arrives second triggers the paint that counts. The caller then controls repainting through its own useCallback dependencies, which is what those are for. The handlers still go through a ref, and the data does not. Callbacks are rebuilt every render and would repaint on every one; the sections are the thing being drawn and should be exactly what decides when to redraw. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -3,7 +3,7 @@ import { useSearchParams } from 'react-router';
|
||||
import { useClient } from 'hooks/useClient';
|
||||
import { readableError } from '../errors';
|
||||
import { SitePreview, useDevPort } from '../SitePreview';
|
||||
import { paintCanvas } from '../canvas';
|
||||
import { paintCanvas, type CanvasSection } from '../canvas';
|
||||
import { AddSectionDialog, type SectionLayout } from './AddSectionDialog';
|
||||
import { usePages, type Page } from '../usePages';
|
||||
|
||||
@@ -34,6 +34,17 @@ export const PageBuilderPanel = () => {
|
||||
const { pages, announce } = usePages();
|
||||
|
||||
const [error, setError] = useState<string | null>(null);
|
||||
const [sections, setSections] = useState<CanvasSection[]>([]);
|
||||
|
||||
/*
|
||||
The handlers, through a ref; the DATA, as a dependency.
|
||||
|
||||
`onDocument` is re-run whenever its identity changes, so what it should depend on is the thing being
|
||||
drawn — the sections — and nothing else. The callbacks are ordinary functions rebuilt every render,
|
||||
so putting them in the dependency list would repaint on every render; through a ref they are always
|
||||
the current ones without being part of the identity.
|
||||
*/
|
||||
const handlersRef = useRef<{ add: () => void; remove: (id: number) => void }>({ add: () => {}, remove: () => {} });
|
||||
const [adding, setAdding] = useState(false);
|
||||
const [creating, setCreating] = useState(false);
|
||||
/*
|
||||
@@ -52,10 +63,34 @@ export const PageBuilderPanel = () => {
|
||||
be a new injection each render.
|
||||
*/
|
||||
const onDocument = useCallback(
|
||||
(frameDocument: Document) => paintCanvas(frameDocument, { onAddSection: () => setAdding(true) }),
|
||||
[],
|
||||
(frameDocument: Document) =>
|
||||
paintCanvas(frameDocument, {
|
||||
sections,
|
||||
onAddSection: () => handlersRef.current.add(),
|
||||
onDeleteSection: (section) => handlersRef.current.remove(section.id),
|
||||
}),
|
||||
[sections],
|
||||
);
|
||||
|
||||
/*
|
||||
The handlers, through a ref; the DATA, as a dependency.
|
||||
|
||||
`onDocument` is re-run whenever its identity changes, so what it should depend on is the thing being
|
||||
drawn — the sections — and nothing else. The callbacks are ordinary functions rebuilt every render,
|
||||
so putting them in the dependency list would repaint on every render; through a ref they are always
|
||||
the current ones without being part of the identity.
|
||||
*/
|
||||
const deleteSection = async (sectionId: number) => {
|
||||
if (!page) return;
|
||||
setError(null);
|
||||
try {
|
||||
await clientRef.current.delete(`/fofos-admin/pages/${page.id}/sections/${sectionId}`);
|
||||
setRendered((n) => n + 1);
|
||||
} catch (failure) {
|
||||
setError(readableError(failure));
|
||||
}
|
||||
};
|
||||
|
||||
const createSection = async (values: { name: string; layout: SectionLayout }) => {
|
||||
if (!page) return;
|
||||
setCreating(true);
|
||||
@@ -89,6 +124,25 @@ export const PageBuilderPanel = () => {
|
||||
|
||||
const page = pages?.find((row) => row.id === id) ?? null;
|
||||
|
||||
/*
|
||||
The page's sections, which the canvas needs to name and outline what it finds in the document.
|
||||
|
||||
Fetched here rather than read out of the DOM. The markup says which id a band came from and nothing
|
||||
else — no name, no layout — and inferring the layout from the classes would be the admin reading the
|
||||
site's stylesheet, which is exactly the coupling all of this avoids.
|
||||
*/
|
||||
useEffect(() => {
|
||||
if (!page) return;
|
||||
void clientRef.current
|
||||
.get<{ data: CanvasSection[] }>(`/fofos-admin/pages/${page.id}/sections`)
|
||||
.then((response) => setSections(response.data))
|
||||
.catch((failure: unknown) => setError(readableError(failure)));
|
||||
}, [page, rendered]);
|
||||
|
||||
// Assigned on every render, after both functions above exist, so the ref always holds the current
|
||||
// pair without either being part of `onDocument`'s identity.
|
||||
handlersRef.current = { add: () => setAdding(true), remove: (sectionId: number) => void deleteSection(sectionId) };
|
||||
|
||||
if (error) return <div className="p-4 text-xs text-destructive">{error}</div>;
|
||||
if (!pages) return <div className="p-4 text-xs text-muted-foreground">Loading…</div>;
|
||||
|
||||
@@ -107,6 +161,8 @@ export const PageBuilderPanel = () => {
|
||||
|
||||
{/* The real page, at its own address, with the builder's own controls drawn inside it. `path`
|
||||
has no leading slash — Home's is the empty string, which the proxy resolves to the root. */}
|
||||
{/* `reloadToken` moves on every write, which refetches the document AND re-runs `onDocument` —
|
||||
so the decoration is redrawn against the sections that now exist. */}
|
||||
<SitePreview port={devPort} path={page.slug} onDocument={onDocument} reloadToken={rendered} />
|
||||
|
||||
<AddSectionDialog
|
||||
|
||||
Reference in New Issue
Block a user