Skip to content

Commit a917622

Browse files
gxvrclaude
gxvr
and
claude
authored
fix(editor): make page slug editable via a new Page settings dialog (CoreBunch#220)
* fix(editor): make page slug editable via a new Page settings dialog A page's slug could only be set at creation time — the site explorer's inline rename only ever updates the title (renamePage(id, title) with no third arg leaves the slug untouched), and there was no other slug editor for a regular (non-template) page. The Data tab correctly locks slug as an editor-managed built-in field, but the editor had nowhere to actually manage it. Add a "Page settings" context-menu item (regular pages only — template pages already get title+slug through the existing Template settings dialog) that opens a small Title + Slug form, reusing the same slug validation helpers as TemplateSettingsDialog. The homepage's slug field is disabled since "index" is a reserved sentinel, not a freely editable value. Extracted the Template/Page settings dialog state and handlers into a usePageSettingsDialogs hook — adding the new dialog inline pushed SiteExplorerPanel.tsx over the 700-line module-size-budgets ceiling. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(data): hide Add row / Duplicate row on tables with no editable fields Pages, Components, and Layouts start out with every field marked built-in and value-locked (isBuiltInValueLocked), so a generic "Add row" or "Duplicate row" through the Data tab has nothing it can actually fill in — buildEmptyCells and buildDuplicateRowCells always include the locked field keys, and the server's lockedBuiltInCellKey guard rejects the request every time. The UI offered both actions unconditionally, so clicking them silently did nothing beyond a console.error, making the whole table look broken/read-only. Add tableHasEditableFields(table) to systemTableGuard.ts (true when at least one field isn't value-locked) and use it in DataCanvas to hide both affordances for tables where every field is locked. As defense in depth, also surface handleAddRow / handleDuplicateRow failures via the toast bus instead of console.error-only, matching this project's error-handling convention for user-triggered operations. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 0c4e9ec commit a917622

13 files changed

Lines changed: 428 additions & 29 deletions

File tree

‎src/__tests__/admin/data/dataCanvasRowCreation.test.tsx‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
import { afterEach, describe, expect, it, mock } from 'bun:test'
2+
import React from 'react'
3+
import { cleanup, fireEvent, render, screen } from '@testing-library/react'
4+
import { DataCanvas } from '@admin/pages/data/components/DataCanvas/DataCanvas'
5+
import type { DataRow, DataTable } from '@core/data/schemas'
6+
7+
afterEach(cleanup)
8+
9+
const now = '2026-07-15T10:00:00.000Z'
10+
11+
function makePagesTable(overrides: Partial<DataTable> = {}): DataTable {
12+
return {
13+
id: 'table-pages',
14+
name: 'pages',
15+
slug: 'pages',
16+
kind: 'page',
17+
singularLabel: 'Page',
18+
pluralLabel: 'Pages',
19+
routeBase: '',
20+
primaryFieldId: 'title',
21+
fields: [
22+
{ type: 'text', id: 'title', label: 'Title', required: true, builtIn: true },
23+
{ type: 'text', id: 'slug', label: 'Slug', required: true, builtIn: true },
24+
],
25+
system: true,
26+
createdByUserId: null,
27+
updatedByUserId: null,
28+
createdAt: now,
29+
updatedAt: now,
30+
...overrides,
31+
}
32+
}
33+
34+
function makeRow(overrides: Partial<DataRow> = {}): DataRow {
35+
return {
36+
id: 'row-home',
37+
tableId: 'table-pages',
38+
cells: { title: 'Home', slug: 'index' },
39+
slug: 'index',
40+
status: 'draft',
41+
authorUserId: null,
42+
createdByUserId: null,
43+
updatedByUserId: null,
44+
publishedByUserId: null,
45+
author: null,
46+
createdBy: null,
47+
updatedBy: null,
48+
publishedBy: null,
49+
createdAt: now,
50+
updatedAt: now,
51+
publishedAt: null,
52+
scheduledPublishAt: null,
53+
deletedAt: null,
54+
...overrides,
55+
}
56+
}
57+
58+
function renderCanvas(table: DataTable, rows: DataRow[], overrides: Partial<React.ComponentProps<typeof DataCanvas>> = {}) {
59+
return render(
60+
<DataCanvas
61+
table={table}
62+
tables={[table]}
63+
rows={rows}
64+
loading={false}
65+
loadingTables={false}
66+
error={null}
67+
selectedRowId={null}
68+
onSelectRow={() => {}}
69+
onAddRow={async () => {}}
70+
onDeleteRow={() => {}}
71+
onDuplicateRow={mock()}
72+
onEditInContent={() => {}}
73+
onOpenInSiteEditor={() => {}}
74+
onOpenRow={() => {}}
75+
onSetRowStatus={async (id, status) => makeRow({ id, status })}
76+
canCreate
77+
canEdit
78+
canDelete
79+
canExport={false}
80+
{...overrides}
81+
/>,
82+
)
83+
}
84+
85+
describe('DataCanvas — row creation on locked system tables', () => {
86+
it('hides "Add row" and "Duplicate row" when every field on the table is a locked built-in', () => {
87+
renderCanvas(makePagesTable(), [makeRow()])
88+
89+
expect(screen.queryByRole('button', { name: /add row/i })).toBeNull()
90+
91+
fireEvent.contextMenu(screen.getByText('Home').closest('[role="row"]')!, { clientX: 100, clientY: 100 })
92+
expect(screen.queryByRole('menuitem', { name: /duplicate row/i })).toBeNull()
93+
})
94+
95+
it('shows "Add row" and "Duplicate row" once the table has a custom (non-built-in) field', () => {
96+
const table = makePagesTable({
97+
fields: [
98+
{ type: 'text', id: 'title', label: 'Title', required: true, builtIn: true },
99+
{ type: 'text', id: 'slug', label: 'Slug', required: true, builtIn: true },
100+
{ type: 'text', id: 'custom-note', label: 'Note', required: false },
101+
],
102+
})
103+
renderCanvas(table, [makeRow()])
104+
105+
expect(screen.getByRole('button', { name: /add row/i })).toBeDefined()
106+
107+
fireEvent.contextMenu(screen.getByText('Home').closest('[role="row"]')!, { clientX: 100, clientY: 100 })
108+
expect(screen.getByRole('menuitem', { name: /duplicate row/i })).toBeDefined()
109+
})
110+
111+
it('still hides "Add row" for a locked table even when the empty state renders', () => {
112+
renderCanvas(makePagesTable(), [])
113+
114+
expect(screen.queryByRole('button', { name: /add row/i })).toBeNull()
115+
expect(screen.getByText(/no pages yet/i)).toBeDefined()
116+
})
117+
})

‎src/__tests__/site-explorer/siteExplorerPanel.test.tsx‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1004,6 +1004,42 @@ describe('SiteExplorerPanel', () => {
10041004
expect(screen.getByRole('textbox', { name: 'Rename Pricing' })).toBeDefined()
10051005
})
10061006

1007+
it('edits a page slug through the Page settings dialog', () => {
1008+
loadSite()
1009+
render(<SiteExplorerPanel sectionGroup="site" />)
1010+
1011+
fireEvent.contextMenu(screen.getByRole('button', { name: /open page pricing/i }), {
1012+
clientX: 120,
1013+
clientY: 140,
1014+
})
1015+
fireEvent.click(screen.getByRole('menuitem', { name: /page settings/i }))
1016+
1017+
const slugInput = screen.getByLabelText('Slug') as HTMLInputElement
1018+
expect(slugInput.value).toBe('pricing')
1019+
1020+
fireEvent.change(slugInput, { target: { value: 'plans-and-pricing' } })
1021+
fireEvent.click(screen.getByRole('button', { name: 'Save' }))
1022+
1023+
const updated = useEditorStore.getState().site?.pages.find((page) => page.id === 'page-pricing')
1024+
expect(updated?.slug).toBe('plans-and-pricing')
1025+
expect(updated?.title).toBe('Pricing')
1026+
})
1027+
1028+
it('locks slug editing for the homepage in the Page settings dialog', () => {
1029+
loadSite()
1030+
render(<SiteExplorerPanel sectionGroup="site" />)
1031+
1032+
fireEvent.contextMenu(screen.getByRole('button', { name: /open page home/i }), {
1033+
clientX: 120,
1034+
clientY: 140,
1035+
})
1036+
fireEvent.click(screen.getByRole('menuitem', { name: /page settings/i }))
1037+
1038+
const slugInput = screen.getByLabelText('Slug') as HTMLInputElement
1039+
expect(slugInput.disabled).toBe(true)
1040+
expect(slugInput.value).toBe('index')
1041+
})
1042+
10071043
it('renames and deletes components from the site row context menu', () => {
10081044
loadSite()
10091045
render(<SiteExplorerPanel sectionGroup="site" />)

‎src/admin/pages/data/DataPage.tsx‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ import {
2323
canManageTable,
2424
} from '@admin/access'
2525
import type { DataRow, DataRowCells, DataRowStatus } from '@core/data/schemas'
26+
import { pushToast } from '@ui/components/Toast'
27+
import { getErrorMessage } from '@core/utils/errorMessage'
2628
import { useDataWorkspace } from './hooks/useDataWorkspace'
2729
import { DataSidebar } from './components/DataSidebar/DataSidebar'
2830
import { DataCanvas } from './components/DataCanvas/DataCanvas'
@@ -97,6 +99,7 @@ export function DataPage() {
9799
workspace.selectRow(newRow.id)
98100
} catch (err) {
99101
console.error('[DataPage] Add row failed:', err)
102+
pushToast({ kind: 'error', title: 'Could not add row', body: getErrorMessage(err, 'Unknown error') })
100103
}
101104
}
102105

@@ -105,6 +108,7 @@ export function DataPage() {
105108
await workspace.duplicateRow(row)
106109
} catch (err) {
107110
console.error('[DataPage] Duplicate row failed:', err)
111+
pushToast({ kind: 'error', title: 'Could not duplicate row', body: getErrorMessage(err, 'Unknown error') })
108112
}
109113
}
110114

‎src/admin/pages/data/components/DataCanvas/DataCanvas.tsx‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { EmptyState } from '@ui/components/EmptyState'
1515
import { DataGrid } from '../DataGrid/DataGrid'
1616
import { DataGridSkeleton } from '../DataGrid/DataGridSkeleton'
1717
import type { DataRow, DataRowStatus, DataTable } from '@core/data/schemas'
18+
import { tableHasEditableFields } from '@core/data/systemTableGuard'
1819
// Reuse the site canvas surface token so the Data page matches
1920
// Site / Content / Media visual language.
2021
import canvasStyles from '@site/canvas/CanvasRoot.module.css'
@@ -112,6 +113,13 @@ export function DataCanvas({
112113
)
113114
}
114115

116+
// `pages` / `components` / `layouts` start out with every field built-in
117+
// and value-locked — a generic "Add row" / "Duplicate row" through the Data
118+
// grid has nothing it could actually fill in, and would only ever bounce
119+
// off the server's `lockedBuiltInCellKey` rejection. Hide both affordances
120+
// for those tables rather than offer an action guaranteed to fail.
121+
const rowCreationSupported = tableHasEditableFields(table)
122+
115123
return (
116124
<section className={`${canvasStyles.canvas} ${styles.canvas}`} aria-label={`${table.pluralLabel} data grid`}>
117125
<DataGrid
@@ -123,9 +131,9 @@ export function DataCanvas({
123131
error={error}
124132
readOnly={!canEdit}
125133
onSelectRow={onSelectRow}
126-
onAddRow={onAddRow}
134+
onAddRow={rowCreationSupported ? onAddRow : undefined}
127135
onEditInContent={onEditInContent}
128-
onDuplicateRow={canCreate ? onDuplicateRow : undefined}
136+
onDuplicateRow={canCreate && rowCreationSupported ? onDuplicateRow : undefined}
129137
onOpenInSiteEditor={onOpenInSiteEditor}
130138
onOpenRow={onOpenRow}
131139
onDeleteRow={canDelete ? onDeleteRow : undefined}

‎src/admin/pages/data/components/DataGrid/DataGrid.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ interface DataGridProps {
6161
readOnly?: boolean
6262
/** Click on a row — typically opens the inspector. */
6363
onSelectRow: (rowId: string | null) => void
64-
onAddRow: () => Promise<void> | void
64+
/** Create a new row. Omit to hide the "Add row" toolbar button + empty-state CTA. */
65+
onAddRow?: () => Promise<void> | void
6566
/** Delete a row by id. Omit to hide per-row + bulk delete. */
6667
onDeleteRow?: (rowId: string) => void
6768
/** Duplicate a row. Omit to hide the duplicate action. */

‎src/admin/pages/data/components/DataGrid/DataGridEmptyState.tsx‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ interface DataGridEmptyStateProps {
1818
/** True when a search query or non-'all' status filter is narrowing the view. */
1919
filtered: boolean
2020
readOnly: boolean
21-
onAddRow: () => Promise<void> | void
21+
onAddRow?: () => Promise<void> | void
2222
}
2323

2424
export function DataGridEmptyState({
@@ -28,20 +28,21 @@ export function DataGridEmptyState({
2828
onAddRow,
2929
}: DataGridEmptyStateProps): ReactElement {
3030
const noun = table.pluralLabel.toLowerCase()
31+
const canAdd = !readOnly && onAddRow != null
3132
return (
3233
<div className={styles.emptyStateSpan}>
3334
<EmptyState
3435
plain
3536
title={filtered ? `No ${noun} match this view` : `No ${noun} yet`}
3637
description={
37-
readOnly
38+
!canAdd
3839
? undefined
3940
: filtered
4041
? 'Try clearing the search or switching views.'
4142
: 'Add the first row to get started.'
4243
}
4344
action={
44-
!readOnly && !filtered ? (
45+
canAdd && !filtered ? (
4546
<Button variant="secondary" size="sm" onClick={() => { void onAddRow() }}>
4647
<PlusIcon size={12} aria-hidden="true" />
4748
Add row

‎src/admin/pages/data/components/DataGrid/DataGridToolbar.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ interface DataGridToolbarProps {
2929
readOnly: boolean
3030
query: string
3131
onQueryChange: (q: string) => void
32-
onAddRow: () => Promise<void> | void
32+
onAddRow?: () => Promise<void> | void
3333
hasPublishWorkflow: boolean
3434
statusViewOrder: StatusViewChip[]
3535
statusFilter: StatusFilter
@@ -88,7 +88,7 @@ export function DataGridToolbar({
8888
/>
8989
</div>
9090

91-
{!readOnly && (
91+
{!readOnly && onAddRow != null && (
9292
<Button variant="primary" size="sm" onClick={() => { void onAddRow() }}>
9393
<PlusIcon size={12} aria-hidden="true" />
9494
Add row

‎src/admin/pages/site/panels/SiteExplorerPanel/SiteExplorerPanel.tsx‎

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,10 @@ import { PaintBucketSolidIcon } from 'pixel-art-icons/icons/paint-bucket-solid'
1212
import { CodeIcon } from 'pixel-art-icons/icons/code'
1313
import { ExternalLinkSolidIcon } from 'pixel-art-icons/icons/external-link-solid'
1414
import { GlobeSolidIcon } from 'pixel-art-icons/icons/globe-solid'
15+
import { Settings2SolidIcon } from 'pixel-art-icons/icons/settings-2-solid'
1516
import { SiteCreateDialog, buildScriptPath, buildStylePath, slugifySiteItemName, type SiteCreatePayload, type SiteCreateKind } from '@admin/shared/dialogs/SiteCreateDialog'
1617
import type { ExplorerContextMenuItem } from '@site/explorer-actions'
17-
import { TemplateSettingsDialog, type TemplateSettingsPayload } from '@admin/shared/dialogs/TemplateSettingsDialog'
18+
import { usePageSettingsDialogs } from './usePageSettingsDialogs'
1819
import { useVCDeletionConfirm } from '@admin/shared/dialogs/VCDeletionConfirmDialog'
1920
import { useConfirmDelete } from '@admin/shared/dialogs/ConfirmDeleteDialog'
2021
import {
@@ -121,7 +122,6 @@ export function SiteExplorerPanel({
121122
const [createKind, setCreateKind] = useState<SiteCreateKind | null>(null)
122123
const [contextMenu, setContextMenu] = useState<ContextMenuState | null>(null)
123124
const [inlineRenameTarget, setInlineRenameTarget] = useState<SiteExplorerContextTarget | null>(null)
124-
const [templateSettingsTarget, setTemplateSettingsTarget] = useState<Page | null>(null)
125125
const [pathConfirmPlan, setPathConfirmPlan] = useState<ExplorerPathChangePlan | null>(null)
126126
const explorerSelection = useSiteExplorerSelection<SiteExplorerContextTarget>()
127127

@@ -152,6 +152,12 @@ export function SiteExplorerPanel({
152152
}
153153

154154
const pages = site?.pages ?? []
155+
const { openTemplateSettings, openPageSettings, dialogs: pageSettingsDialogs } = usePageSettingsDialogs({
156+
pages,
157+
renamePage,
158+
convertPageToTemplate,
159+
openPageInCanvas,
160+
})
155161
const normalPages = pages.filter((page) => !page.template)
156162
const templatePages = pages.filter((page) => page.template)
157163
const components = site?.visualComponents ?? []
@@ -350,15 +356,7 @@ export function SiteExplorerPanel({
350356
const slug = createUniquePageSlug('Post Template', pages)
351357
const page = addPage('Post Template', slug)
352358
openPageInCanvas(page.id)
353-
setTemplateSettingsTarget(page)
354-
}
355-
356-
function handleSaveTemplateSettings(payload: TemplateSettingsPayload) {
357-
if (!templateSettingsTarget) return
358-
renamePage(templateSettingsTarget.id, payload.title, payload.slug)
359-
convertPageToTemplate(templateSettingsTarget.id, payload.template)
360-
setTemplateSettingsTarget(null)
361-
openPageInCanvas(templateSettingsTarget.id)
359+
openTemplateSettings(page)
362360
}
363361

364362
function templateMenuItems(target: SiteExplorerContextTarget) {
@@ -371,7 +369,7 @@ export function SiteExplorerPanel({
371369
label: 'Template settings',
372370
icon: <FileTextSolidIcon size={13} />,
373371
action: () => {
374-
setTemplateSettingsTarget(page)
372+
openTemplateSettings(page)
375373
setContextMenu(null)
376374
},
377375
},
@@ -390,7 +388,7 @@ export function SiteExplorerPanel({
390388
label: 'Use as template',
391389
icon: <FileTextSolidIcon size={13} />,
392390
action: () => {
393-
setTemplateSettingsTarget(page)
391+
openTemplateSettings(page)
394392
setContextMenu(null)
395393
},
396394
}]
@@ -417,6 +415,16 @@ export function SiteExplorerPanel({
417415
setContextMenu(null)
418416
},
419417
},
418+
// Templates get title+slug through "Template settings" already — avoid
419+
// a second, redundant slug editor for the same page.
420+
...(!page.template ? [{
421+
label: 'Page settings',
422+
icon: <Settings2SolidIcon size={13} />,
423+
action: () => {
424+
openPageSettings(page)
425+
setContextMenu(null)
426+
},
427+
}] : []),
420428
...templateMenuItems(target),
421429
]
422430
}
@@ -652,14 +660,7 @@ export function SiteExplorerPanel({
652660
onDelete={() => handleDeleteContext(contextMenu)}
653661
/>
654662
)}
655-
{templateSettingsTarget && (
656-
<TemplateSettingsDialog
657-
page={templateSettingsTarget}
658-
pages={pages}
659-
onCancel={() => setTemplateSettingsTarget(null)}
660-
onSave={handleSaveTemplateSettings}
661-
/>
662-
)}
663+
{pageSettingsDialogs}
663664
{pathConfirmPlan && (
664665
<SiteExplorerPathConfirmDialog
665666
plan={pathConfirmPlan}

0 commit comments

Comments
 (0)