diff --git a/apps/client/src/components/CardLayoutBuilder.tsx b/apps/client/src/components/CardLayoutBuilder.tsx index 1c0fe28..3324ffe 100644 --- a/apps/client/src/components/CardLayoutBuilder.tsx +++ b/apps/client/src/components/CardLayoutBuilder.tsx @@ -27,6 +27,11 @@ import { LoadingSpinner } from './LoadingSpinner'; interface CardLayoutBuilderProps { tableId: string; fields: BuilderField[]; + /** + * Reports whether the working layout differs from the selected view's saved config, so + * the route can guard navigation. Must be referentially stable. + */ + onDirtyChange?: (isDirty: boolean) => void; } interface View { @@ -59,7 +64,32 @@ function DroppableColumn({ id, children, className, style }: DroppableColumnProp ); } -export function CardLayoutBuilder({ tableId, fields }: CardLayoutBuilderProps) { +/** Flatten the drag-and-drop containers into the column array the API stores. */ +function columnsOf(items: Record, columnCount: number): string[][] { + return Array.from({ length: columnCount }).map((_, idx) => items[`column-${idx}`] || []); +} + +/** + * Serialize the parts of the layout that `handleSave` persists, so the working copy can be + * compared against what the selected view holds server-side. Fields left unassigned are + * excluded because their order is not part of the saved config. + */ +function snapshotLayout( + columnCount: number, + columns: string[][], + hiddenLabels: Record +): string { + return JSON.stringify({ + columnCount, + columns, + // Toggling a label off leaves an explicit `false`, which is equivalent to absent. + hiddenLabels: Object.keys(hiddenLabels) + .filter((id) => hiddenLabels[id]) + .sort(), + }); +} + +export function CardLayoutBuilder({ tableId, fields, onDirtyChange }: CardLayoutBuilderProps) { const { t } = useI18n(); const queryClient = useQueryClient(); @@ -74,6 +104,10 @@ export function CardLayoutBuilder({ tableId, fields }: CardLayoutBuilderProps) { const [hiddenLabels, setHiddenLabels] = useState>({}); + // Snapshot of the selected view's saved config, compared against the working layout to + // know whether leaving the page would discard work. + const [savedLayout, setSavedLayout] = useState(() => snapshotLayout(1, [[]], {})); + // Dragging states const [activeId, setActiveId] = useState(null); const [toastMessage, setToastMessage] = useState(null); @@ -102,25 +136,19 @@ export function CardLayoutBuilder({ tableId, fields }: CardLayoutBuilderProps) { // Sync state with selected view layout config useEffect(() => { - if (!selectedView) { - setColumnCount(1); - setLayoutItems({ - unassigned: [], - 'column-0': fields.map((f) => f.id), - }); - setHiddenLabels({}); - return; - } + // Fallback for a view with no usable config: every field stacked in one column. + let nextCount = 1; + let nextItems: Record = { + unassigned: [], + 'column-0': fields.map((f) => f.id), + }; + let nextLabels: Record = {}; - const fieldIdsMap = new Set(fields.map((f) => f.id)); - const config = selectedView.config; + const config = selectedView?.config; if (config && config.columns && typeof config.columnCount === 'number') { - setColumnCount(config.columnCount); - - const newItems: Record = { - unassigned: [], - }; + const fieldIdsMap = new Set(fields.map((f) => f.id)); + nextCount = config.columnCount; // Load columns const loadedColumns = config.columns.map((col) => col.filter((id) => fieldIdsMap.has(id))); @@ -133,26 +161,45 @@ export function CardLayoutBuilder({ tableId, fields }: CardLayoutBuilderProps) { loadedColumns.splice(config.columnCount); } + nextItems = { unassigned: [] }; loadedColumns.forEach((col, idx) => { - newItems[`column-${idx}`] = col; + nextItems[`column-${idx}`] = col; }); // Find any fields in table that are not placed in columns const placedIds = new Set(loadedColumns.flat()); - newItems.unassigned = fields.map((f) => f.id).filter((id) => !placedIds.has(id)); + nextItems.unassigned = fields.map((f) => f.id).filter((id) => !placedIds.has(id)); - setLayoutItems(newItems); - setHiddenLabels(config.hiddenLabels ?? {}); - } else { - setColumnCount(1); - setLayoutItems({ - unassigned: [], - 'column-0': fields.map((f) => f.id), - }); - setHiddenLabels({}); + nextLabels = config.hiddenLabels ?? {}; } + + setColumnCount(nextCount); + setLayoutItems(nextItems); + setHiddenLabels(nextLabels); + // Rebase the baseline onto what was just loaded, normalized the same way the working + // copy is, so dropped fields and padded columns do not read as user edits. + setSavedLayout(snapshotLayout(nextCount, columnsOf(nextItems, nextCount), nextLabels)); }, [selectedView, fields]); + const isLayoutDirty = useMemo(() => { + // Without a selected view there is nothing to save to, so nothing can be lost. + if (!selectedViewId) return false; + return ( + snapshotLayout(columnCount, columnsOf(layoutItems, columnCount), hiddenLabels) !== savedLayout + ); + }, [selectedViewId, columnCount, layoutItems, hiddenLabels, savedLayout]); + + // Report upward so the route-level blocker guards this tab too. + useEffect(() => { + onDirtyChange?.(isLayoutDirty); + }, [isLayoutDirty, onDirtyChange]); + + // The working layout lives in this component: once it unmounts the edits are already + // gone, so there is nothing left for the route to warn about. + useEffect(() => { + return () => onDirtyChange?.(false); + }, [onDirtyChange]); + // Change columns count handler const handleColumnCountChange = (newCount: number) => { setColumnCount(newCount); @@ -217,8 +264,11 @@ export function CardLayoutBuilder({ tableId, fields }: CardLayoutBuilderProps) { config, }); }, - onSuccess: () => { + onSuccess: (_data, config) => { queryClient.invalidateQueries({ queryKey: ['views', tableId] }); + // Rebase immediately rather than waiting for the refetch, so the guard stops firing + // as soon as the save lands. + setSavedLayout(snapshotLayout(config.columnCount, config.columns, config.hiddenLabels ?? {})); showToast(t('layout.saveSuccess')); }, }); @@ -350,12 +400,9 @@ export function CardLayoutBuilder({ tableId, fields }: CardLayoutBuilderProps) { }; const handleSave = () => { - const colsArray = Array.from({ length: columnCount }).map( - (_, idx) => layoutItems[`column-${idx}`] || [] - ); saveLayoutMutation.mutate({ columnCount, - columns: colsArray, + columns: columnsOf(layoutItems, columnCount), hiddenLabels, }); }; @@ -380,9 +427,7 @@ export function CardLayoutBuilder({ tableId, fields }: CardLayoutBuilderProps) { const activeField = activeId ? fieldMap.get(activeId) : null; const unassigned = layoutItems.unassigned || []; - const columns = Array.from({ length: columnCount }).map( - (_, idx) => layoutItems[`column-${idx}`] || [] - ); + const columns = columnsOf(layoutItems, columnCount); if (viewsQuery.isLoading) { return ( diff --git a/apps/client/src/components/__tests__/CardLayoutBuilder.test.tsx b/apps/client/src/components/__tests__/CardLayoutBuilder.test.tsx index c1d08ea..9c30ae3 100644 --- a/apps/client/src/components/__tests__/CardLayoutBuilder.test.tsx +++ b/apps/client/src/components/__tests__/CardLayoutBuilder.test.tsx @@ -1,4 +1,4 @@ -import { describe, it, expect, vi } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { screen, fireEvent, waitFor } from '@testing-library/react'; import { CardLayoutBuilder } from '../CardLayoutBuilder'; import { type BuilderField } from '../../lib/fieldTypes'; @@ -71,3 +71,108 @@ describe('CardLayoutBuilder', () => { }); }); }); + +describe('CardLayoutBuilder unsaved-changes signal', () => { + function renderBuilder(onDirtyChange: (isDirty: boolean) => void) { + // A fresh client per test, so a cached view list cannot leak between them. + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + return render( + + + + ); + } + + /** Wait for the saved view to load, then report the last dirty value seen. */ + async function lastDirty(onDirtyChange: ReturnType) { + await waitFor(() => { + expect(screen.getAllByTitle('Retirer')).toHaveLength(2); + }); + return onDirtyChange.mock.calls.at(-1)?.[0]; + } + + beforeEach(() => { + vi.spyOn(apiClient, 'get').mockImplementation((url) => { + if (url.startsWith('/views')) { + return Promise.resolve(mockViews); + } + return Promise.reject(new Error('Unexpected URL')); + }); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('reports clean while the working layout still matches the saved view', async () => { + const onDirtyChange = vi.fn(); + renderBuilder(onDirtyChange); + + expect(await lastDirty(onDirtyChange)).toBe(false); + }); + + it('reports dirty once a field is moved out of a column', async () => { + const onDirtyChange = vi.fn(); + renderBuilder(onDirtyChange); + + expect(await lastDirty(onDirtyChange)).toBe(false); + + fireEvent.click(screen.getAllByTitle('Retirer')[0]); + + await waitFor(() => { + expect(onDirtyChange.mock.calls.at(-1)?.[0]).toBe(true); + }); + }); + + it('reports dirty once the column count changes', async () => { + const onDirtyChange = vi.fn(); + renderBuilder(onDirtyChange); + + expect(await lastDirty(onDirtyChange)).toBe(false); + + fireEvent.click(screen.getByRole('button', { name: '3 columns' })); + + await waitFor(() => { + expect(onDirtyChange.mock.calls.at(-1)?.[0]).toBe(true); + }); + }); + + it('reports clean again once the layout is saved', async () => { + const put = vi.spyOn(apiClient, 'put').mockResolvedValue({}); + const onDirtyChange = vi.fn(); + renderBuilder(onDirtyChange); + + expect(await lastDirty(onDirtyChange)).toBe(false); + + fireEvent.click(screen.getAllByTitle('Retirer')[0]); + await waitFor(() => { + expect(onDirtyChange.mock.calls.at(-1)?.[0]).toBe(true); + }); + + fireEvent.click(screen.getByRole('button', { name: /Save Layout/ })); + + await waitFor(() => { + expect(put).toHaveBeenCalledWith('/views/view-1', { + config: { columnCount: 2, columns: [[], ['f2']], hiddenLabels: {} }, + }); + }); + await waitFor(() => { + expect(onDirtyChange.mock.calls.at(-1)?.[0]).toBe(false); + }); + }); + + it('reports clean on unmount, since the working layout goes with it', async () => { + const onDirtyChange = vi.fn(); + const { unmount } = renderBuilder(onDirtyChange); + + expect(await lastDirty(onDirtyChange)).toBe(false); + + fireEvent.click(screen.getAllByTitle('Retirer')[0]); + await waitFor(() => { + expect(onDirtyChange.mock.calls.at(-1)?.[0]).toBe(true); + }); + + unmount(); + expect(onDirtyChange.mock.calls.at(-1)?.[0]).toBe(false); + }); +}); diff --git a/apps/client/src/routes/__tests__/builder-unsaved-guard.test.tsx b/apps/client/src/routes/__tests__/builder-unsaved-guard.test.tsx index 9c88f37..c7a4cad 100644 --- a/apps/client/src/routes/__tests__/builder-unsaved-guard.test.tsx +++ b/apps/client/src/routes/__tests__/builder-unsaved-guard.test.tsx @@ -37,6 +37,18 @@ const mockFields = [ }, ]; +// The layout builder keys its columns by draft field id, which the page derives from the +// persisted id. +const mockViews = [ + { + id: 'view-1', + table_id: 'tbl-1', + name: 'Fiche', + type: 'card', + config: { columnCount: 1, columns: [['draft-f1']] }, + }, +]; + function renderBuilder() { const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); return render( @@ -55,6 +67,7 @@ describe('schema builder unsaved-changes guard', () => { if (url.startsWith('/tables/')) return Promise.resolve(mockTable) as any; if (url.startsWith('/fields?')) return Promise.resolve(mockFields) as any; if (url.startsWith('/tables?')) return Promise.resolve([mockTable]) as any; + if (url.startsWith('/views')) return Promise.resolve(mockViews) as any; return Promise.resolve([]) as any; }); }); @@ -83,6 +96,25 @@ describe('schema builder unsaved-changes guard', () => { expect(blockerOpts?.enableBeforeUnload()).toBe(true); }); + it('blocks navigation once the layout tab holds unsaved edits', async () => { + const user = userEvent.setup(); + renderBuilder(); + + await user.click(await screen.findByTestId('layout-tab')); + + // The saved layout is loaded as-is, so nothing is at risk yet. + const removeButton = await screen.findByTitle('Retirer'); + expect(blockerOpts?.shouldBlockFn()).toBe(false); + + // Pull the field out of its column without saving. + await user.click(removeButton); + + await waitFor(() => { + expect(blockerOpts?.shouldBlockFn()).toBe(true); + }); + expect(blockerOpts?.enableBeforeUnload()).toBe(true); + }); + it('offers to stay or leave when the router reports a blocked navigation', async () => { const user = userEvent.setup(); const proceed = vi.fn(); diff --git a/apps/client/src/routes/builder.$databaseId.$tableId.tsx b/apps/client/src/routes/builder.$databaseId.$tableId.tsx index 9a197e0..727f878 100644 --- a/apps/client/src/routes/builder.$databaseId.$tableId.tsx +++ b/apps/client/src/routes/builder.$databaseId.$tableId.tsx @@ -109,6 +109,9 @@ function StructureBuilder() { // Snapshot of what the server holds, keyed by draft field id. Compared against the // live draft to know whether leaving the page would discard work. const [savedSnapshot, setSavedSnapshot] = useState>({}); + // The layout tab keeps its own working state; `CardLayoutBuilder` reports whether it + // holds edits that the explicit Save has not persisted yet. + const [isLayoutDirty, setIsLayoutDirty] = useState(false); const [deleteError, setDeleteError] = useState(null); const [pendingAction, setPendingAction] = useState<{ fieldId: string; @@ -410,7 +413,7 @@ function StructureBuilder() { }); }, [fields, queryClient, tableId, saveFieldMutation]); - const isDirty = useMemo(() => { + const isStructureDirty = useMemo(() => { const current = snapshotFields(fields); const currentIds = Object.keys(current); const savedIds = Object.keys(savedSnapshot); @@ -419,6 +422,9 @@ function StructureBuilder() { return currentIds.some((id) => current[id] !== savedSnapshot[id]); }, [fields, savedSnapshot]); + // Either tab holds work behind an explicit Save, so either can be lost on navigation. + const isDirty = isStructureDirty || isLayoutDirty; + // Covers in-app navigation; `enableBeforeUnload` covers tab close and reload. const blocker = useBlocker({ shouldBlockFn: () => isDirty, @@ -527,7 +533,7 @@ function StructureBuilder() { selectedId={selectedFieldId} onSelect={setSelectedFieldId} onRemove={handleRemove} - isDirty={isDirty} + isDirty={isStructureDirty} /> @@ -548,7 +554,7 @@ function StructureBuilder() { ) : ( - + )} {pendingAction && (