diff --git a/apps/web/src/app/api/pages/[pageId]/form-target/route.ts b/apps/web/src/app/api/pages/[pageId]/form-target/route.ts index 507f3c8edf..ce9479adb9 100644 --- a/apps/web/src/app/api/pages/[pageId]/form-target/route.ts +++ b/apps/web/src/app/api/pages/[pageId]/form-target/route.ts @@ -1,6 +1,6 @@ /** - * Authenticated Forms settings API for a Canvas page — lets the Forms tab - * (apps/web/.../canvas/CanvasFormsSettingsTab.tsx) wire an existing
+ * Authenticated Forms settings API for a Canvas page — lets the Forms category + * (apps/web/.../canvas/settings/CanvasFormsSettingsSection.tsx) wire an existing * tag on the page to a Sheet, and manage the resulting grant, without going * through the AI tool (apps/web/src/lib/ai/tools/form-tools.ts). `pageId` * here is the CANVAS page's id, not the target Sheet's — targets are looked diff --git a/apps/web/src/app/api/pages/[pageId]/publish/__tests__/route.test.ts b/apps/web/src/app/api/pages/[pageId]/publish/__tests__/route.test.ts index ff9ec19597..d81f2e3fae 100644 --- a/apps/web/src/app/api/pages/[pageId]/publish/__tests__/route.test.ts +++ b/apps/web/src/app/api/pages/[pageId]/publish/__tests__/route.test.ts @@ -21,9 +21,11 @@ vi.mock('@/lib/auth', () => ({ isAuthError: (result: unknown) => typeof result === 'object' && result !== null && 'error' in result, checkMCPPageScope: vi.fn().mockResolvedValue(null), canPrincipalEditPage: (auth: { userId: string }, pageId: string) => canUserEditPage(auth.userId, pageId), + canPrincipalViewPage: (auth: { userId: string }, pageId: string) => canUserViewPage(auth.userId, pageId), })); const canUserEditPage = vi.fn(); +const canUserViewPage = vi.fn(); vi.mock('@pagespace/lib/permissions/permissions', () => ({ canUserEditPage: (...args: unknown[]) => canUserEditPage(...args), })); @@ -130,6 +132,7 @@ beforeEach(() => { vi.clearAllMocks(); authenticateRequestWithOptions.mockResolvedValue({ userId: 'user-1' }); canUserEditPage.mockResolvedValue(true); + canUserViewPage.mockResolvedValue(true); allocatePublishSubdomain.mockResolvedValue('acme'); isPublishConfigured.mockReturnValue(true); getPublishAssetBaseUrl.mockReturnValue('https://test-publish.t3.tigrisfiles.io'); @@ -239,6 +242,7 @@ describe('POST /api/pages/[pageId]/publish', () => { description: null, ogImageUrl: null, noindex: false, + themeBridgeEnabled: true, }); }); @@ -262,6 +266,7 @@ describe('POST /api/pages/[pageId]/publish', () => { description: null, ogImageUrl: null, noindex: false, + themeBridgeEnabled: true, }); }); @@ -291,6 +296,7 @@ describe('POST /api/pages/[pageId]/publish', () => { description: null, ogImageUrl: null, noindex: false, + themeBridgeEnabled: true, }); }); @@ -365,6 +371,18 @@ describe('POST /api/pages/[pageId]/publish', () => { })); }); + it('accepts themeBridgeEnabled: false and threads it into the rendered page as injectThemeBridge: false', async () => { + findFirstPage.mockResolvedValue({ id: 'page-1', type: 'CANVAS', title: 'Welcome', content: '

hi

', driveId: 'drive-1' }); + findFirstDrive.mockResolvedValue({ id: 'drive-1', slug: 'acme', publishSubdomain: 'acme', kind: 'STANDARD' }); + findFirstPublished.mockResolvedValue(null); + + const res = await POST(makeReq({ themeBridgeEnabled: false }), { params }); + expect(res.status).toBe(200); + expect(renderPublishedPage).toHaveBeenCalledWith(expect.objectContaining({ injectThemeBridge: false })); + const json = await res.json(); + expect(json.themeBridgeEnabled).toBe(false); + }); + it('returns 400 when ogImageUrl is not a valid URL', async () => { findFirstPage.mockResolvedValue({ id: 'page-1', type: 'CANVAS', title: 'Welcome', content: '

hi

', driveId: 'drive-1' }); findFirstDrive.mockResolvedValue({ id: 'drive-1', slug: 'acme', publishSubdomain: 'acme', kind: 'STANDARD' }); @@ -457,18 +475,52 @@ describe('POST /api/pages/[pageId]/publish', () => { }); describe('GET /api/pages/[pageId]/publish', () => { - it('returns 403 when the user cannot edit the page', async () => { + it('returns 403 when the user can neither edit nor view the page', async () => { canUserEditPage.mockResolvedValue(false); + canUserViewPage.mockResolvedValue(false); const res = await GET(makeReq(), { params }); expect(res.status).toBe(403); }); - it('returns { published: false, available: true } when no row exists and publishing is configured', async () => { + it('given view-but-not-edit access to a published page, returns 200 with only the rendering-relevant field', async () => { + canUserEditPage.mockResolvedValue(false); + canUserViewPage.mockResolvedValue(true); + findFirstPublished.mockResolvedValue({ + driveId: 'drive-1', path: 'welcome', publishedAt: new Date(), updatedAt: new Date(), themeBridgeEnabled: false, + }); + + const res = await GET(makeReq(), { params }); + expect(res.status).toBe(200); + const json = await res.json(); + // Exact-equal so a future change to the route can't silently start + // leaking edit-oriented fields (SEO overrides, url, isStale) to a + // view-only collaborator without this test catching it. + expect(json).toEqual({ published: true, canEdit: false, themeBridgeEnabled: false }); + // The drive/domain lookups are skipped entirely for a view-only caller. + expect(findFirstDrive).not.toHaveBeenCalled(); + }); + + it('returns { published: false, available: true, canEdit: true } when no row exists and publishing is configured', async () => { findFirstPublished.mockResolvedValue(undefined); const res = await GET(makeReq(), { params }); expect(res.status).toBe(200); const json = await res.json(); - expect(json).toEqual({ published: false, available: true }); + expect(json).toEqual({ published: false, available: true, canEdit: true }); + }); + + it('given view-but-not-edit access to an unpublished page, returns canEdit: false (not just available)', async () => { + // Regression test: PublishControls decides whether to show an enabled + // Publish button from `available` alone on this branch — without + // `canEdit` here too, a view-only viewer would see an enabled button + // that only reveals the permission failure once its POST 403s. + canUserEditPage.mockResolvedValue(false); + canUserViewPage.mockResolvedValue(true); + findFirstPublished.mockResolvedValue(undefined); + + const res = await GET(makeReq(), { params }); + expect(res.status).toBe(200); + const json = await res.json(); + expect(json).toEqual({ published: false, available: true, canEdit: false }); }); it('returns available: false when the page is in a Home drive (no published row)', async () => { @@ -489,7 +541,7 @@ describe('GET /api/pages/[pageId]/publish', () => { const res = await GET(makeReq(), { params }); expect(res.status).toBe(200); const json = await res.json(); - expect(json).toEqual({ published: false, available: false }); + expect(json).toEqual({ published: false, available: false, canEdit: true }); }); it('reports available: false when the kill-switch is engaged even if the bucket is configured', async () => { @@ -502,7 +554,7 @@ describe('GET /api/pages/[pageId]/publish', () => { expect(res.status).toBe(200); const json = await res.json(); // POST would 503 ("temporarily disabled") here, so the UI must hide the control. - expect(json).toEqual({ published: false, available: false }); + expect(json).toEqual({ published: false, available: false, canEdit: true }); } finally { if (prev === undefined) delete process.env.CANVAS_PUBLISHING_DISABLED; else process.env.CANVAS_PUBLISHING_DISABLED = prev; @@ -521,6 +573,7 @@ describe('GET /api/pages/[pageId]/publish', () => { const json = await res.json(); expect(json).toEqual({ published: true, + canEdit: true, available: true, isStale: false, url: 'https://acme.pagespace.site/welcome', @@ -531,6 +584,7 @@ describe('GET /api/pages/[pageId]/publish', () => { description: null, ogImageUrl: null, noindex: false, + themeBridgeEnabled: true, }); }); diff --git a/apps/web/src/app/api/pages/[pageId]/publish/route.ts b/apps/web/src/app/api/pages/[pageId]/publish/route.ts index 94b214a201..3d83192955 100644 --- a/apps/web/src/app/api/pages/[pageId]/publish/route.ts +++ b/apps/web/src/app/api/pages/[pageId]/publish/route.ts @@ -2,7 +2,7 @@ import { NextResponse } from 'next/server'; import { z } from 'zod/v4'; import { loggers } from '@pagespace/lib/logging/logger-config'; import { auditRequest } from '@pagespace/lib/audit/audit-log'; -import { authenticateRequestWithOptions, isAuthError, checkMCPPageScope, canPrincipalEditPage } from '@/lib/auth'; +import { authenticateRequestWithOptions, isAuthError, checkMCPPageScope, canPrincipalEditPage, canPrincipalViewPage } from '@/lib/auth'; import { isPublishConfigured } from '@/lib/canvas/published-storage'; import { publishCanvasPage, clearPublishedHomeRoot, regeneratePublishedSiteFiles, PublishError, PUBLISH_HOST } from '@/lib/canvas/publish-page'; import { deletePageFromCustomHosts, getActiveDomainRecords } from '@/lib/canvas/custom-domain-mirror'; @@ -29,6 +29,9 @@ const publishSchema = z.object({ // publishCanvasPage — never trusted as a URL directly. ogImageFileId: z.string().min(1).optional(), noindex: z.boolean().optional(), + // Canvas pages only: whether the published artifact follows PageSpace's + // light/dark theme. Ignored (has no effect) for other publishable page types. + themeBridgeEnabled: z.boolean().optional(), }).nullable(); export async function GET(req: Request, { params }: { params: Promise<{ pageId: string }> }) { @@ -41,8 +44,14 @@ export async function GET(req: Request, { params }: { params: Promise<{ pageId: const scopeError = await checkMCPPageScope(auth, pageId); if (scopeError) return scopeError; + // A viewer who can't edit still needs this page's `themeBridgeEnabled` — + // it drives the canvas View tab's live preview for EVERY collaborator, not + // just editors, and it's not sensitive (it's baked into the publicly + // published HTML either way). Everything else this route returns (SEO + // overrides, publish URL, staleness) stays edit-gated below. const canEdit = await canPrincipalEditPage(auth, pageId); - if (!canEdit) { + const canView = canEdit || await canPrincipalViewPage(auth, pageId); + if (!canView) { return NextResponse.json({ error: 'You do not have permission to view this page' }, { status: 403 }); } @@ -61,6 +70,7 @@ export async function GET(req: Request, { params }: { params: Promise<{ pageId: publishDescription: true, publishOgImageUrl: true, noindex: true, + themeBridgeEnabled: true, }, }); @@ -78,7 +88,21 @@ export async function GET(req: Request, { params }: { params: Promise<{ pageId: if (drv?.kind === 'HOME') available = false; } } - return NextResponse.json({ published: false, available }); + // `canEdit` matters even for an unpublished page: a view-only caller + // reaching here (canView but !canEdit) must not have PublishControls + // read `available` alone and offer an enabled Publish button that + // only reveals the permission failure once its POST 403s. + return NextResponse.json({ published: false, available, canEdit }); + } + + // View-only: stop here with just the rendering-relevant field. Skip the + // drive/domain/staleness lookups below — nothing else in this response + // is meant for a non-editor (SEO overrides, the publish URL, staleness). + // `canEdit: false` tells consumers like the Settings tab's Publish/ + // Appearance categories not to treat `published: true` as "safe to show + // editable, savable fields" — those two are otherwise-independent facts. + if (!canEdit) { + return NextResponse.json({ published: true, canEdit: false, themeBridgeEnabled: row.themeBridgeEnabled ?? true }); } const [drive, livePage] = await Promise.all([ @@ -119,6 +143,7 @@ export async function GET(req: Request, { params }: { params: Promise<{ pageId: return NextResponse.json({ published: true, + canEdit: true, available, isStale, url, @@ -130,6 +155,7 @@ export async function GET(req: Request, { params }: { params: Promise<{ pageId: description: row.publishDescription ?? null, ogImageUrl: row.publishOgImageUrl ?? null, noindex: row.noindex ?? false, + themeBridgeEnabled: row.themeBridgeEnabled ?? true, }); } catch (error) { loggers.api.error('Error reading publish status:', error as Error); @@ -208,6 +234,7 @@ export async function POST(req: Request, { params }: { params: Promise<{ pageId: description: parsedBody?.description, ogImageUrl, noindex: parsedBody?.noindex, + themeBridgeEnabled: parsedBody?.themeBridgeEnabled, }); auditRequest(req, { diff --git a/apps/web/src/components/canvas/CanvasFrame.tsx b/apps/web/src/components/canvas/CanvasFrame.tsx index 47f4c91d60..216b4f23d8 100644 --- a/apps/web/src/components/canvas/CanvasFrame.tsx +++ b/apps/web/src/components/canvas/CanvasFrame.tsx @@ -15,6 +15,10 @@ import { useNonce } from '@/contexts/NonceContext'; interface CanvasFrameProps { html: string; title?: string; + /** Mirrors the persisted `published_pages.themeBridgeEnabled` setting (see + * the canvas Settings tab's Appearance category) so the editor's live + * preview matches what publishing will produce. Defaults to `true`. */ + themeBridgeEnabled?: boolean; } /** @@ -69,7 +73,7 @@ function hasRecentUserActivation(): boolean { * The document string is the same one produced for published pages, so the * in-app view and the published artifact render identically. */ -export function CanvasFrame({ html, title }: CanvasFrameProps) { +export function CanvasFrame({ html, title, themeBridgeEnabled = true }: CanvasFrameProps) { const nonce = useNonce(); const router = useRouter(); const [previewHtml, setPreviewHtml] = useState(html); @@ -119,14 +123,16 @@ export function CanvasFrame({ html, title }: CanvasFrameProps) { // baseTarget '_blank': inside the sandboxed frame an ordinary (no // target) would navigate the frame itself — and many sites refuse framing — // so default links to a new tab (works with the iframe's allow-popups). - // injectThemeBridge: true injects a script that listens for theme messages - // so the canvas iframe's dark/light state matches the app's current theme. + // injectThemeBridge injects a script that listens for theme messages so the + // canvas iframe's dark/light state matches the app's current theme — unless + // the author disabled it (canvas Settings > Appearance) for a design that + // shouldn't be fought by an injected `dark` class. // navigationBridge: true injects a script that intercepts clicks on internal // dashboard page links and hands them to the message handler below, so they // route in-app instead of falling through to baseTarget's new-tab behavior. const srcDoc = useMemo( - () => renderCanvasDocument({ html: previewHtml, title, baseTarget: '_blank', injectThemeBridge: true, navigationBridge: true, nonce }), - [previewHtml, title, nonce], + () => renderCanvasDocument({ html: previewHtml, title, baseTarget: '_blank', injectThemeBridge: themeBridgeEnabled, navigationBridge: true, nonce }), + [previewHtml, title, nonce, themeBridgeEnabled], ); // Send the current theme to the canvas iframe whenever it changes or the diff --git a/apps/web/src/components/layout/middle-content/content-header/PublishControls.tsx b/apps/web/src/components/layout/middle-content/content-header/PublishControls.tsx index 296e938ffb..fd9d5a88a0 100644 --- a/apps/web/src/components/layout/middle-content/content-header/PublishControls.tsx +++ b/apps/web/src/components/layout/middle-content/content-header/PublishControls.tsx @@ -14,12 +14,12 @@ import { DialogTitle, } from '@/components/ui/dialog'; import { Button } from '@/components/ui/button'; -import { Input } from '@/components/ui/input'; -import { Label } from '@/components/ui/label'; -import { Textarea } from '@/components/ui/textarea'; -import { Switch } from '@/components/ui/switch'; -import { PagePickerPopover } from '@/components/common/PagePickerPopover'; -import { PageType } from '@pagespace/lib/utils/enums'; +import { PublishSettingsFields, type PublishSettings } from './PublishSettingsFields'; +import { + usePublishStatusStore, + publishStatusFromResponse, + type PublishSettingsSnapshot, +} from '@/stores/usePublishStatusStore'; interface PublishControlsProps { pageId: string; @@ -34,50 +34,21 @@ interface PublishControlsProps { variant?: 'header' | 'panel'; } -/** Author-supplied SEO overrides for a published page. */ -interface PublishSettings { - title: string; - description: string; - ogImageUrl: string; - noindex: boolean; -} - -interface PublishState { - published: boolean; - url: string | null; - // Whether the server can publish at all (dedicated public bucket configured). - // When false (e.g. a deployment without PUBLISH_BUCKET) the control is hidden - // rather than offering a Publish button that only ever 503s. - available: boolean; - isStale: boolean; - settings: PublishSettings; - // True only for a transient failure to load status (network error, 5xx) — - // distinct from `!available`, which means the status request succeeded (or - // definitively 403'd for a read-only viewer) and reported publishing as - // genuinely unavailable. Keeping these separate stops a blip from rendering - // the same "isn't available" message as a real, durable unavailability - // signal. Lives alongside `available` (rather than as its own useState) so - // every status-fetch outcome sets both fields in one atomic update. - hasLoadError: boolean; -} - -const EMPTY_SETTINGS: PublishSettings = { title: '', description: '', ogImageUrl: '', noindex: false }; - -const EMPTY_STATE: PublishState = { - published: false, - url: null, - available: false, - isStale: false, - settings: EMPTY_SETTINGS, - hasLoadError: false, -}; - -/** PublishSettings plus a transient "picked an uploaded file" alternative to - * pasting a URL — resolved server-side, never persisted as its own field. */ -interface PublishOverrides extends PublishSettings { +/** PublishSettingsSnapshot plus a transient "picked an uploaded file" + * alternative to pasting a URL — resolved server-side, never persisted as + * its own field. */ +interface PublishOverrides extends PublishSettingsSnapshot { ogImageFileId?: string; } +/** What `PublishSettingsDialog` can actually produce — the shared SEO fields + * it renders, plus the same transient uploaded-file alternative. No + * `themeBridgeEnabled`: the dialog is shared across every publishable page + * type, and that field only means anything for canvas pages (see the + * Appearance category) — `PublishControls` merges in the page's current + * value before calling `handlePublish`. */ +type PublishDialogOverrides = PublishSettings & { ogImageFileId?: string }; + interface PublishStatusResponse { published: boolean; url?: string; @@ -87,15 +58,9 @@ interface PublishStatusResponse { description?: string | null; ogImageUrl?: string | null; noindex?: boolean; + themeBridgeEnabled?: boolean; } -const settingsFromResponse = (data: PublishStatusResponse): PublishSettings => ({ - title: data.title ?? '', - description: data.description ?? '', - ogImageUrl: data.ogImageUrl ?? '', - noindex: data.noindex ?? false, -}); - const readError = async (res: Response): Promise => { try { const data = (await res.json()) as { error?: unknown }; @@ -108,58 +73,30 @@ const readError = async (res: Response): Promise => { const PublishControls = ({ pageId, contentDirty, variant = 'header' }: PublishControlsProps) => { const params = useParams<{ driveId?: string }>(); const driveId = params?.driveId; - const [state, setState] = useState(EMPTY_STATE); - const [isLoading, setIsLoading] = useState(true); + // Shared across every surface showing this page's publish state (this + // header control, the canvas Settings tab's Publish/Appearance categories) + // so an action taken in one is immediately reflected in the others instead + // of each independently caching a stale snapshot from its own mount-time fetch. + const status = usePublishStatusStore((s) => s.statuses.get(pageId)); + const fetchStatus = usePublishStatusStore((s) => s.fetchStatus); + const setStatus = usePublishStatusStore((s) => s.setStatus); const [isBusy, setIsBusy] = useState(false); const [settingsOpen, setSettingsOpen] = useState(false); const prevDirtyRef = useRef(undefined); useEffect(() => { - let cancelled = false; - setIsLoading(true); - (async () => { - try { - const res = await fetchWithAuth(`/api/pages/${pageId}/publish`); - if (!res.ok) { - if (cancelled) return; - // 403 means the viewer definitively lacks permission to publish — - // a genuine unavailability signal, not a failed request. Anything - // else (5xx, etc.) is a real load failure. - setState({ ...EMPTY_STATE, hasLoadError: res.status !== 403 }); - return; - } - const data = (await res.json()) as PublishStatusResponse; - if (!cancelled) { - setState({ - published: data.published, - url: data.published ? data.url ?? null : null, - available: data.available ?? false, - isStale: data.isStale ?? false, - settings: settingsFromResponse(data), - hasLoadError: false, - }); - } - } catch { - if (!cancelled) { - setState({ ...EMPTY_STATE, hasLoadError: true }); - } - } finally { - if (!cancelled) setIsLoading(false); - } - })(); - return () => { - cancelled = true; - }; - }, [pageId]); + fetchStatus(pageId); + }, [pageId, fetchStatus]); // When a save completes (dirty → clean), mark the published version as stale // so the Update button appears without requiring a page reload. useEffect(() => { if (prevDirtyRef.current === true && contentDirty === false) { - setState(prev => prev.published ? { ...prev, isStale: true } : prev); + const current = usePublishStatusStore.getState().statuses.get(pageId); + if (current?.published) setStatus(pageId, { ...current, isStale: true }); } prevDirtyRef.current = contentDirty; - }, [contentDirty]); + }, [contentDirty, pageId, setStatus]); // Publish (or re-publish) the page. `overrides`, when provided, carries the // author's SEO settings; omitting it preserves whatever is persisted. @@ -173,6 +110,7 @@ const PublishControls = ({ pageId, contentDirty, variant = 'header' }: PublishCo ogImageUrl: overrides.ogImageUrl, ogImageFileId: overrides.ogImageFileId, noindex: overrides.noindex, + themeBridgeEnabled: overrides.themeBridgeEnabled, }) : undefined; const res = await fetchWithAuth(`/api/pages/${pageId}/publish`, { @@ -184,18 +122,26 @@ const PublishControls = ({ pageId, contentDirty, variant = 'header' }: PublishCo return false; } const data = (await res.json()) as PublishStatusResponse & { url: string }; - setState((prev) => ({ - ...prev, + setStatus(pageId, { published: true, + // Reaching this line required a successful POST, which is itself + // edit-gated server-side — this viewer can edit. + canEdit: true, url: data.url, available: true, isStale: false, - // Read the effective settings back from the server rather than caching - // the request payload: when `overrides.ogImageFileId` was set, the - // request's `ogImageUrl` is a blank placeholder — the server resolves - // it to the real CDN URL and returns that resolved value here. - settings: overrides ? settingsFromResponse(data) : prev.settings, - })); + hasLoadError: false, + // Always read settings back from the server response, even when no + // `overrides` were passed: it's not just "whatever was persisted + // before" — an unpublish (DELETE) removes the published_pages row + // entirely, so a bare republish after one gets fresh server + // defaults, not the pre-unpublish settings. Caching the latter here + // would show (and let Save silently restore) values the server has + // already reset. This also covers the `overrides.ogImageFileId` + // case: the request's `ogImageUrl` is a blank placeholder that the + // server resolves to the real CDN URL, returned here. + settings: publishStatusFromResponse(data).settings, + }); toast.success(isUpdate ? 'Page updated' : 'Page published'); return true; } catch { @@ -204,7 +150,7 @@ const PublishControls = ({ pageId, contentDirty, variant = 'header' }: PublishCo } finally { setIsBusy(false); } - }, [pageId]); + }, [pageId, setStatus]); const handleUnpublish = useCallback(async () => { setIsBusy(true); @@ -214,40 +160,47 @@ const PublishControls = ({ pageId, contentDirty, variant = 'header' }: PublishCo toast.error(await readError(res)); return; } - setState((prev) => ({ ...prev, published: false, url: null, isStale: false })); + const prev = usePublishStatusStore.getState().statuses.get(pageId); + if (prev) setStatus(pageId, { ...prev, published: false, url: null, isStale: false }); toast.success('Page unpublished'); } catch { toast.error('Failed to unpublish page'); } finally { setIsBusy(false); } - }, [pageId]); + }, [pageId, setStatus]); const handleCopy = useCallback(async () => { - if (!state.url) return; + if (!status?.url) return; try { - await navigator.clipboard.writeText(state.url); + await navigator.clipboard.writeText(status.url); toast.success('Link copied'); } catch { toast.error('Failed to copy link'); } - }, [state.url]); + }, [status?.url]); - if (isLoading) { + if (!status) { return Loading…; } - // Publishing isn't configured on this deployment (e.g. no PUBLISH_BUCKET), or - // this viewer lacks permission to publish (a definitive 403 — see - // `hasLoadError` on PublishState for the transient-failure case, handled - // separately). In the header (among other populated buttons) staying silent - // is fine; in a standalone panel (the canvas Settings tab) silence would - // leave the tab blank, so explain instead. - if (!state.available) { + // Publishing isn't configured on this deployment (e.g. no PUBLISH_BUCKET), + // or this viewer can view but not edit — the GET route now serves + // `published`/`available` to any viewer (see the route's view-permission + // comment) so the View tab's preview stays accurate for everyone, but + // that means this control must check `canEdit` itself rather than relying + // on a 403 to hide it: without this, a view-only viewer on an unpublished + // page would see an enabled Publish button that only reveals the + // permission failure once its own POST 403s. (A genuine 403 — no view + // access at all — still surfaces via `hasLoadError`'s transient-failure + // case, handled separately.) In the header (among other populated + // buttons) staying silent is fine; in a standalone panel (the canvas + // Settings tab) silence would leave the tab blank, so explain instead. + if (!status.available || !status.canEdit) { if (variant === 'panel') { return (

- {state.hasLoadError + {status.hasLoadError ? "Couldn't load publishing status. Try again shortly." : "Publishing isn't available for this page."}

@@ -256,72 +209,87 @@ const PublishControls = ({ pageId, contentDirty, variant = 'header' }: PublishCo return null; } - if (!state.published || !state.url) { + if (!status.published || !status.url) { return ( - + ); } + // The dialog's SEO fields are shared across every publishable page type + // (see PublishSettingsFields) and don't include themeBridgeEnabled, which + // only canvas pages have any use for — see the Appearance category instead. + const dialogSettings: PublishSettings = { + title: status.settings.title, + description: status.settings.description, + ogImageUrl: status.settings.ogImageUrl, + noindex: status.settings.noindex, + }; + return (
- {state.url} + {status.url}
- {state.isStale && ( + {status.isStale && ( <> Stale - + )} - - + - +
{ - const ok = await handlePublish(true, next); + const ok = await handlePublish(true, { ...next, themeBridgeEnabled: status.settings.themeBridgeEnabled }); if (ok) setSettingsOpen(false); }} /> @@ -335,20 +303,27 @@ interface PublishSettingsDialogProps { initial: PublishSettings; driveId?: string; isBusy: boolean; - onSave: (settings: PublishOverrides) => void; + onSave: (settings: PublishDialogOverrides) => void; } function PublishSettingsDialog({ open, onOpenChange, initial, driveId, isBusy, onSave }: PublishSettingsDialogProps) { const [form, setForm] = useState(initial); const [pickedImageId, setPickedImageId] = useState(null); - // Re-seed the form from the latest persisted values whenever the dialog opens. + // Re-seed the form from the latest persisted values on the closed→open + // transition only — `initial` is intentionally NOT a dependency. The + // parent recreates that object on every render (it's a plain literal + // derived from `status.settings`), so depending on it here would reseed + // (and clobber any in-progress edit) on every unrelated parent re-render + // while the dialog stays open — e.g. `contentDirty` flipping, or a shared + // publish-status refresh completing elsewhere. + // eslint-disable-next-line react-hooks/exhaustive-deps useEffect(() => { if (open) { setForm(initial); setPickedImageId(null); } - }, [open, initial]); + }, [open]); // Validate the share-image URL client-side so the user gets a specific message // instead of the server's generic rejection (the route also enforces this). @@ -384,72 +359,14 @@ function PublishSettingsDialog({ open, onOpenChange, initial, driveId, isBusy, o Control how this page appears in search results and link previews. Leave a field blank to use the page's own content. -
-
- - setForm((f) => ({ ...f, title: e.target.value }))} - placeholder="Defaults to the page title" - /> -
-
- -