Skip to content

Commit fe76390

Browse files
committed
fix(tables): open the add-row form for required columns and gate Save on required fields
- New row, Shift+Enter, and Insert row open the Add Row form at their position when updates are locked or any column is required - Add Row and Update Row stay disabled until every required field has a value - Add mode accepts an insert position; Shift+Enter anchors to the neighbor row id
1 parent 1dbc256 commit fe76390

6 files changed

Lines changed: 150 additions & 29 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/context-menu/context-menu.tsx‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,7 @@ interface ContextMenuProps {
6565
disableInsert?: boolean
6666
/**
6767
* Duplicate is a one-shot insert carrying the copied row's data, so it needs
68-
* only the insert lock — unlike the blank-row inserts above it, which also
69-
* need the update lock to be fillable.
68+
* only the insert lock.
7069
*/
7170
disableDuplicate?: boolean
7271
disableDelete?: boolean

‎apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.test.tsx‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,75 @@ describe('RowModal add mode', () => {
154154
act(() => root.unmount())
155155
container.remove()
156156
})
157+
158+
it('inserts the row at the requested position', async () => {
159+
const container = document.createElement('div')
160+
document.body.appendChild(container)
161+
const root = createRoot(container)
162+
const props = {
163+
mode: 'add' as const,
164+
isOpen: true,
165+
onClose: vi.fn(),
166+
table: {
167+
id: 'table-3',
168+
name: 'People',
169+
schema: {
170+
columns: [{ id: 'col_name', name: 'Name', type: 'string' as const, required: true }],
171+
},
172+
},
173+
insertAt: { afterRowId: 'row-1' },
174+
onSuccess: vi.fn(),
175+
}
176+
177+
act(() => root.render(createElement(RowModal, props)))
178+
179+
const nameInput = container.querySelector<HTMLInputElement>('[data-testid="modal-input"]')
180+
act(() => changeInput(nameInput as HTMLInputElement, 'Ada'))
181+
const submit = container.querySelector<HTMLButtonElement>('[data-testid="submit"]')
182+
await act(async () => submit?.click())
183+
184+
expect(mockCreateRow).toHaveBeenCalledWith({ data: { col_name: 'Ada' }, afterRowId: 'row-1' })
185+
act(() => root.unmount())
186+
container.remove()
187+
})
188+
189+
it('keeps Add Row disabled until every required field has a value', () => {
190+
const container = document.createElement('div')
191+
document.body.appendChild(container)
192+
const root = createRoot(container)
193+
const props = {
194+
mode: 'add' as const,
195+
isOpen: true,
196+
onClose: vi.fn(),
197+
table: {
198+
id: 'table-3',
199+
name: 'People',
200+
schema: {
201+
columns: [
202+
{ id: 'col_name', name: 'Name', type: 'string' as const, required: true },
203+
{ id: 'col_notes', name: 'Notes', type: 'string' as const },
204+
{ id: 'col_active', name: 'Active', type: 'boolean' as const, required: true },
205+
],
206+
},
207+
},
208+
onSuccess: vi.fn(),
209+
}
210+
211+
act(() => root.render(createElement(RowModal, props)))
212+
213+
const submit = () => container.querySelector<HTMLButtonElement>('[data-testid="submit"]')
214+
const nameInput = container.querySelectorAll<HTMLInputElement>('[data-testid="modal-input"]')[0]
215+
expect(submit()?.disabled).toBe(true)
216+
217+
act(() => changeInput(nameInput, 'Ada'))
218+
expect(submit()?.disabled).toBe(false)
219+
220+
act(() => changeInput(nameInput, ''))
221+
expect(submit()?.disabled).toBe(true)
222+
223+
act(() => root.unmount())
224+
container.remove()
225+
})
157226
})
158227

159228
describe('RowModal column ids', () => {

‎apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/row-modal/row-modal.tsx‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@ import type { ColumnDefinition, TableInfo, TableRow } from '@/lib/table'
2222
import { getColumnId } from '@/lib/table/column-keys'
2323
import { columnTypeOf } from '@/lib/table/column-types'
2424
import { resolveCurrencyCode } from '@/lib/table/currency'
25+
import { isEmptyCellValue } from '@/lib/table/deps'
2526
import { todayAtTtlOffset, ttlValueFromPicker, ttlValueToPickerParts } from '@/lib/table/ttl-values'
2627
import { getTimezoneEditBlockedMessage } from '@/app/workspace/[workspaceId]/tables/[tableId]/components/timezone-editing'
28+
import type { RowInsertTarget } from '@/app/workspace/[workspaceId]/tables/[tableId]/types'
2729
import { type TimezoneState, useTimezoneState } from '@/hooks/queries/general-settings'
2830
import {
2931
useCreateTableRow,
@@ -50,6 +52,8 @@ export interface RowModalProps {
5052
table: TableInfo
5153
row?: TableRow
5254
rowIds?: string[]
55+
/** Where add mode inserts the row; appends when omitted. */
56+
insertAt?: RowInsertTarget
5357
onSuccess: () => void
5458
}
5559

@@ -87,7 +91,16 @@ function cleanRowData(
8791
* call-site ever keeps it mounted across target-row changes, it must supply a `key`
8892
* prop (e.g. the row id) so React remounts with the new row's values.
8993
*/
90-
export function RowModal({ mode, isOpen, onClose, table, row, rowIds, onSuccess }: RowModalProps) {
94+
export function RowModal({
95+
mode,
96+
isOpen,
97+
onClose,
98+
table,
99+
row,
100+
rowIds,
101+
insertAt,
102+
onSuccess,
103+
}: RowModalProps) {
91104
const params = useParams()
92105
const workspaceId = params.workspaceId as string
93106
const tableId = table.id
@@ -121,17 +134,25 @@ export function RowModal({ mode, isOpen, onClose, table, row, rowIds, onSuccess
121134
const hasEditableColumn = columns.some(
122135
(column) => columnTypeOf(column).editor !== 'date' || dateEditorsReady
123136
)
137+
/** Toggles always save a boolean, so only other required columns can be left empty. */
138+
const missingRequiredValue = columns.some(
139+
(column) =>
140+
column.required &&
141+
columnTypeOf(column).editor !== 'toggle' &&
142+
isEmptyCellValue(rowData[getColumnId(column)])
143+
)
144+
const canSubmit = hasEditableColumn && !missingRequiredValue
124145

125146
const handleFormSubmit = async (e?: React.FormEvent) => {
126147
e?.preventDefault()
127148
setError(null)
128-
if (!hasEditableColumn) return
149+
if (!canSubmit) return
129150

130151
try {
131152
const cleanData = cleanRowData(columns, rowData, timeZone, dateEditorsReady)
132153

133154
if (isAddMode) {
134-
await createRowMutation.mutateAsync({ data: cleanData })
155+
await createRowMutation.mutateAsync({ data: cleanData, ...insertAt })
135156
} else if (row) {
136157
await updateRowMutation.mutateAsync({ rowId: row.id, data: cleanData })
137158
}
@@ -211,7 +232,7 @@ export function RowModal({ mode, isOpen, onClose, table, row, rowIds, onSuccess
211232
{isAddMode ? 'Fill in values for' : 'Update values for'} {table?.name ?? 'table'}
212233
</p>
213234
<form onSubmit={handleFormSubmit} className='contents'>
214-
<button type='submit' hidden disabled={isSubmitting || !hasEditableColumn} />
235+
<button type='submit' hidden disabled={isSubmitting || !canSubmit} />
215236
{columns.map((column) =>
216237
columnTypeOf(column).editor === 'date' && !dateEditorsReady ? (
217238
<TimezoneBlockedColumnField
@@ -250,7 +271,7 @@ export function RowModal({ mode, isOpen, onClose, table, row, rowIds, onSuccess
250271
? 'Updating...'
251272
: 'Update Row',
252273
onClick: () => handleFormSubmit(),
253-
disabled: isSubmitting || !hasEditableColumn,
274+
disabled: isSubmitting || !canSubmit,
254275
}}
255276
/>
256277
</ChipModal>

‎apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx‎

Lines changed: 40 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ import { extractCreatedRowId, useTableUndo } from '@/hooks/use-table-undo'
5656
import type { ChatContext } from '@/stores/panel'
5757
import type { DeletedRowSnapshot } from '@/stores/table/types'
5858
import { useContextMenu, useTable } from '../../hooks'
59-
import type { EditingCell, QueryOptions, SaveReason } from '../../types'
59+
import type { EditingCell, QueryOptions, RowInsertTarget, SaveReason } from '../../types'
6060
import { cleanCellValue, generateColumnName as sharedGenerateColumnName } from '../../utils'
6161
import type { ColumnConfig } from '../column-config-sidebar'
6262
import { ColumnDropdown } from '../column-dropdown'
@@ -209,8 +209,8 @@ interface TableGridProps {
209209
onOpenEnrichmentDetails: (rowId: string, groupId: string) => void
210210
/** Open the row-edit modal for `row`. Wrapper renders the modal. */
211211
onOpenRowModal: (row: TableRowType) => void
212-
/** Opens the add-row form, which inserts a complete row in one request. */
213-
onOpenAddRowModal: () => void
212+
/** Opens the add-row form, which inserts a complete row at `insertAt` (appends when omitted). */
213+
onOpenAddRowModal: (insertAt?: RowInsertTarget) => void
214214
/** Open the row-delete modal for `snapshots`. Wrapper renders the modal. */
215215
onRequestDeleteRows: (snapshots: DeletedRowSnapshot[]) => void
216216
/**
@@ -373,6 +373,15 @@ function writeLoadedRowsWithChip(opts: {
373373
return true
374374
}
375375

376+
/**
377+
* Whether new rows must go through the add-row form instead of a blank grid row.
378+
* A blank row only works when the grid can fill it in afterwards: typing into it
379+
* is an update, and the server rejects an empty row when any column is required.
380+
*/
381+
function needsAddRowForm(updateLocked: boolean | undefined, columns: ColumnDefinition[]): boolean {
382+
return Boolean(updateLocked) || columns.some((column) => column.required)
383+
}
384+
376385
/**
377386
* Value-equality for a cell's stored value vs a pending edit. Primitives compare
378387
* with `===`; arrays/objects (multiselect id arrays, json) compare structurally
@@ -703,15 +712,14 @@ export function TableGrid({
703712
// requires the delete lock clear too — mirror that here or the affordance
704713
// stays live on an append-only table and only fails on click.
705714
const canDestroyColumn = canMutateSchema && !locks?.deleteLocked
706-
// Duplicate inserts a full copied row in one shot, so unlike the blank-row
707-
// paths it needs the insert lock only — it is valid on an append-only table.
715+
/**
716+
* Inserts that carry the whole row in one request (Duplicate, paste-append, the
717+
* add-row form) need only the insert lock, so they stay valid on an append-only
718+
* table. New row, Shift+Enter, and Insert row fall back to that form whenever
719+
* `needsAddRowForm` says a blank row can't work.
720+
*/
708721
const canInsertFullRow = userPermissions.canEdit && !locks?.insertLocked
709-
// Manual grid entry is "add an empty row, then type into its cells" — the
710-
// typing is an update. So a *useful* manual add needs BOTH insert and update
711-
// unlocked; on an append-only table (update locked) it would leave a blank
712-
// row the user can't fill, so New row opens the add-row form instead, which
713-
// inserts the complete row in one request. Full-row inserts (the form, CSV
714-
// import, API, blocks, Mothership) need only the insert lock off server-side.
722+
/** A blank grid row is filled in by typing, which is an update, so it needs both locks off. */
715723
const canManualAddRow = userPermissions.canEdit && !locks?.insertLocked && !locks?.updateLocked
716724
const canEditCellRef = useRef(canEditCell)
717725
canEditCellRef.current = canEditCell
@@ -1623,6 +1631,11 @@ export function TableGrid({
16231631
const anchorId = contextMenu.row.id
16241632
// Fractional ordering: express intent by neighbor id, not integer position.
16251633
const intent = offset === 0 ? { beforeRowId: anchorId } : { afterRowId: anchorId }
1634+
if (needsAddRowForm(updateLockedRef.current, schemaColumnsRef.current)) {
1635+
closeContextMenu()
1636+
onOpenAddRowModalRef.current(intent)
1637+
return
1638+
}
16261639
createRef.current(
16271640
{ data: {}, ...intent },
16281641
{
@@ -1762,7 +1775,10 @@ export function TableGrid({
17621775
// Stable identity so <AddRowButton>'s React.memo still bails out; lock state
17631776
// is read from refs instead of being closed over.
17641777
const handleAddRowClick = useCallback(() => {
1765-
if (canInsertFullRowRef.current && updateLockedRef.current) {
1778+
if (
1779+
canInsertFullRowRef.current &&
1780+
needsAddRowForm(updateLockedRef.current, schemaColumnsRef.current)
1781+
) {
17661782
onOpenAddRowModalRef.current()
17671783
return
17681784
}
@@ -2977,14 +2993,22 @@ export function TableGrid({
29772993

29782994
if (e.shiftKey && e.key === 'Enter') {
29792995
if (!canEditRef.current) return
2980-
// Same manual-add path as the Add row button, so it owes the same
2981-
// explanation rather than silently doing nothing on a locked table.
2996+
const row = currentRows[anchor.rowIndex]
2997+
// Mirrors handleAddRowClick; keep the two new-row paths in sync.
2998+
if (
2999+
row &&
3000+
canInsertFullRowRef.current &&
3001+
needsAddRowForm(updateLockedRef.current, schemaColumnsRef.current)
3002+
) {
3003+
e.preventDefault()
3004+
onOpenAddRowModalRef.current({ afterRowId: row.id })
3005+
return
3006+
}
29823007
if (!canManualAddRowRef.current) {
29833008
e.preventDefault()
29843009
onBlockedActionRef.current('add-row')
29853010
return
29863011
}
2987-
const row = currentRows[anchor.rowIndex]
29883012
if (!row) return
29893013
e.preventDefault()
29903014
const position = row.position + 1
@@ -5119,7 +5143,7 @@ export function TableGrid({
51195143
hasWorkflowColumns={hasWorkflowColumns}
51205144
workflowCellScoped={Boolean(contextMenuGroupId)}
51215145
disableEdit={!canEditCell}
5122-
disableInsert={!canManualAddRow}
5146+
disableInsert={!canInsertFullRow}
51235147
disableDuplicate={!canInsertFullRow}
51245148
disableDelete={!canDeleteRow}
51255149
onAddToChat={addToChatRowIds.length > 0 ? handleAddSelectionToChat : undefined}

‎apps/sim/app/workspace/[workspaceId]/tables/[tableId]/table.tsx‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ import {
100100
tableDetailParsers,
101101
tableDetailUrlKeys,
102102
} from './search-params'
103-
import type { QueryOptions } from './types'
103+
import type { QueryOptions, RowInsertTarget } from './types'
104104
import { generateColumnName } from './utils'
105105

106106
const logger = createLogger('Table')
@@ -226,7 +226,7 @@ export function Table({
226226
const blockedToastIdRef = useRef<string | null>(null)
227227
const [isImportCsvOpen, setIsImportCsvOpen] = useState(false)
228228
const [editingRow, setEditingRow] = useState<TableRowType | null>(null)
229-
const [isAddingRow, setIsAddingRow] = useState(false)
229+
const [addRowTarget, setAddRowTarget] = useState<RowInsertTarget | null>(null)
230230
const [deletingRows, setDeletingRows] = useState<DeletedRowSnapshot[]>([])
231231
const [deletingAll, setDeletingAll] = useState<{
232232
excludeRowIds: string[]
@@ -296,7 +296,7 @@ export function Table({
296296
}, [])
297297
const onCloseSlideout = () => dispatch({ type: 'CLOSE' })
298298
const onOpenRowModal = (row: TableRowType) => setEditingRow(row)
299-
const onOpenAddRowModal = () => setIsAddingRow(true)
299+
const onOpenAddRowModal = (insertAt: RowInsertTarget = {}) => setAddRowTarget(insertAt)
300300
// useCallback because <Resource.Header> is memo-wrapped — these flow into
301301
// the breadcrumbs / headerActions memos, whose identity drives that re-render.
302302
const onRequestDeleteTable = useCallback(() => setShowDeleteTableConfirm(true), [])
@@ -1756,13 +1756,14 @@ export function Table({
17561756
table={tableData}
17571757
/>
17581758
)}
1759-
{isAddingRow && tableData && (
1759+
{addRowTarget && tableData && (
17601760
<RowModal
17611761
mode='add'
17621762
isOpen={true}
1763-
onClose={() => setIsAddingRow(false)}
1763+
onClose={() => setAddRowTarget(null)}
17641764
table={tableData}
1765-
onSuccess={() => setIsAddingRow(false)}
1765+
insertAt={addRowTarget}
1766+
onSuccess={() => setAddRowTarget(null)}
17661767
/>
17671768
)}
17681769
{editingRow && tableData && (

‎apps/sim/app/workspace/[workspaceId]/tables/[tableId]/types.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import type { InsertTableRowBodyInput } from '@/lib/api/contracts/tables'
12
import type { SortSpec, TablePredicate, TableRow } from '@/lib/table'
23

34
/**
@@ -36,3 +37,9 @@ export interface EditingCell {
3637
columnName: string
3738
columnKey?: string
3839
}
40+
41+
/** Where a new row goes; an empty target appends it to the end of the table. */
42+
export type RowInsertTarget = Pick<
43+
InsertTableRowBodyInput,
44+
'position' | 'afterRowId' | 'beforeRowId'
45+
>

0 commit comments

Comments
 (0)