diff --git a/README.md b/README.md index 59d2090..e31f8b5 100644 --- a/README.md +++ b/README.md @@ -96,6 +96,7 @@ const table = useSpreadSheetTable({ initialData, // Row[] - initial row data rowKey, // keyof Row - unique identifier field onChange, // (changes: ChangeInfo[]) => void + onRowChangeCommitted, // (commit: RowChangeCommit) => void onReorder, // (newData: Row[]) => void validate, // (row: Row, key: string, value: unknown) => ValidationResult | undefined frozenColumns, // number - columns to freeze from the left diff --git a/apps/docs/e2e/row-change.test.ts b/apps/docs/e2e/row-change.test.ts index 564aa83..a4c1500 100644 --- a/apps/docs/e2e/row-change.test.ts +++ b/apps/docs/e2e/row-change.test.ts @@ -24,6 +24,41 @@ test.describe('行変更の確定前処理', () => { await expect(getCell(page, 0, 2)).toHaveText('10:00') }) + test('編集の確定内容が補正後の値を含めて通知される', async ({ page }) => { + await goToRowChangeDemo(page) + const demo = getDemoContainer(page) + + await getCell(page, 0, 1).dblclick() + const input = demo.locator('input[type="time"]').first() + await input.fill('10:00') + await input.press('Enter') + + await expect(page.getByTestId('committed-message')).toHaveText( + 'edit: #0 startTime 09:00→10:00, endTime 10:00→11:00', + ) + await expect(page.getByTestId('commit-count')).toHaveText('1') + }) + + test('ペーストは1操作としてまとめて通知される', async ({ page }) => { + await goToRowChangeDemo(page) + const mod = modKey(page) + + await page.evaluate(() => navigator.clipboard.writeText('10:00\t11:00\n13:00\t14:00')) + await clickCell(page, 0, 1) + await page.keyboard.press(`${mod}+KeyV`) + + await expect(page.getByTestId('committed-message')).toHaveText( + 'paste: #0 startTime 09:00→10:00, endTime 10:00→11:00 / #1 startTime 11:00→13:00, endTime 12:00→14:00', + ) + // 2行分の変更でも通知は1回のみ + await expect(page.getByTestId('commit-count')).toHaveText('1') + + await expect(getCell(page, 0, 1)).toHaveText('10:00') + await expect(getCell(page, 0, 2)).toHaveText('11:00') + await expect(getCell(page, 1, 1)).toHaveText('13:00') + await expect(getCell(page, 1, 2)).toHaveText('14:00') + }) + test('拒否された編集は反映されず、拒否理由が通知される', async ({ page }) => { await goToRowChangeDemo(page) const demo = getDemoContainer(page) @@ -36,6 +71,8 @@ test.describe('行変更の確定前処理', () => { await expect(getCell(page, 0, 2)).toHaveText('10:00') await expect(page.getByTestId('rejected-message')).toHaveText('終了は開始より後にしてください') + // 拒否された操作は確定通知されない + await expect(page.getByTestId('commit-count')).toHaveText('0') }) test('クリアはsourceがclearとして拒否される', async ({ page }) => { diff --git a/apps/docs/src/components/RowChangeDemo.tsx b/apps/docs/src/components/RowChangeDemo.tsx index 9ff10b1..cb232ae 100644 --- a/apps/docs/src/components/RowChangeDemo.tsx +++ b/apps/docs/src/components/RowChangeDemo.tsx @@ -89,14 +89,27 @@ const processRowChange: ProcessRowChange = (candidate, previous, context) export function RowChangeDemo() { const [rejected, setRejected] = useState(null) + const [committed, setCommitted] = useState(null) + const [commitCount, setCommitCount] = useState(0) const table = useSpreadSheetTable({ columns, initialData: shiftData, rowKey: 'id', processRowChange, - onRowChangeRejected: (errors) => { - setRejected(errors.map((error) => error.result.message).join(' / ')) + onRowChangeCommitted: (commit) => { + const rows = commit.rows.map((row) => { + const changes = row.changes + .map((change) => `${String(change.key)} ${change.previousValue}→${change.newValue}`) + .join(', ') + return `#${row.rowIndex} ${changes}` + }) + setCommitted(`${commit.source}: ${rows.join(' / ')}`) + setCommitCount((count) => count + 1) + }, + onRowChangeRejected: (rejection) => { + setRejected(rejection.errors.map((error) => error.result.message).join(' / ')) + setCommitted(null) }, onChange: () => setRejected(null), }) @@ -110,6 +123,12 @@ export function RowChangeDemo() { {rejected ?? ''} + + {committed ?? ''} + + + {commitCount} + diff --git a/apps/docs/src/content/docs/api/use-spread-sheet-table.mdx b/apps/docs/src/content/docs/api/use-spread-sheet-table.mdx index 7fb5769..5871741 100644 --- a/apps/docs/src/content/docs/api/use-spread-sheet-table.mdx +++ b/apps/docs/src/content/docs/api/use-spread-sheet-table.mdx @@ -24,12 +24,13 @@ const table = useSpreadSheetTable({ | `columns` | `ColumnDef[]` | (必須) | カラム定義の配列 | | `initialData` | `T[]` | (必須) | 初期データ | | `rowKey` | `keyof T` | (必須) | 行のユニークキー | -| `onChange` | `(rows: T[]) => void` | - | データ変更時のコールバック | +| `onChange` | `(rows: ChangeInfo[]) => void` | - | 編集基準から変更された行を通知 | +| `onRowChangeCommitted` | `(commit: RowChangeCommit) => void` | - | 今回の操作で確定した変更前後の行を通知 | | `onReorder` | `(rows: T[]) => void` | - | 行並び替え時のコールバック | | `validate` | `(value, row, key) => ValidationResult \| null` | - | カスタムバリデーション | | `onValidationError` | `(errors) => void` | - | バリデーションエラーのコールバック | | `processRowChange` | `(candidate, previous, context) => ProcessRowChangeResult` | - | 行変更を確定前に補正または拒否 | -| `onRowChangeRejected` | `(errors) => void` | - | 行変更が拒否されたときのコールバック | +| `onRowChangeRejected` | `(rejection: RowChangeRejection) => void` | - | 拒否された操作と行を通知 | | `readOnly` | `boolean` | `false` | テーブル全体を読み取り専用に | | `sortable` | `boolean` | `true` | ソート有効/無効 | | `filterable` | `boolean` | `true` | フィルター有効/無効 | @@ -94,8 +95,13 @@ const table = useSpreadSheetTable({ row: nextRow, } }, - onRowChangeRejected: (errors) => { - console.log('Rejected row change:', errors) + onRowChangeCommitted: (commit) => { + for (const { previousRow, row } of commit.rows) { + saveChangedRow(previousRow, row) + } + }, + onRowChangeRejected: (rejection) => { + console.log('Rejected row change:', rejection.rows) }, }) ``` @@ -105,3 +111,7 @@ const table = useSpreadSheetTable({ 補正した値は元の編集と同じ操作として記録されるため、1回のUndoでまとめて元に戻ります。 複数行のペーストでは、1行でも拒否されるとペースト全体を反映しません。 範囲を選択したDeleteとCutも同じ扱いで、1行でも拒否されると範囲全体をクリアしません。 + +`onChange`は編集基準からの累積変更を通知します。 +今回の操作だけをAPIへ送る場合は、`onRowChangeCommitted`の`previousRow`と`row`を使用してください。 +渡された行はテーブル内のスナップショットとして読み取り専用で扱います。 diff --git a/apps/docs/src/content/docs/playground/row-change.mdx b/apps/docs/src/content/docs/playground/row-change.mdx index 6da3c1a..4e0dc2a 100644 --- a/apps/docs/src/content/docs/playground/row-change.mdx +++ b/apps/docs/src/content/docs/playground/row-change.mdx @@ -11,6 +11,7 @@ import { RowChangeDemo } from '../../../components/RowChangeDemo' - 終了時刻が開始時刻以前になる変更は拒否されます - 担当者と開始の列は Delete / Cut でクリアできません (`context.source` が `clear` の場合の分岐) - 複数行のペーストは、1行でも拒否されると全体が反映されません +- 確定した変更は `onRowChangeCommitted` で操作単位に通知されます (補正後の値を含みます)
diff --git a/openspec/changes/add-change-transaction-context/design.md b/openspec/changes/add-change-transaction-context/design.md new file mode 100644 index 0000000..d8cb3c8 --- /dev/null +++ b/openspec/changes/add-change-transaction-context/design.md @@ -0,0 +1,97 @@ +## Context + +テーブルは非制御コンポーネントとして行データを内部に保持する。 + +現在の`onChange`第1引数は、編集基準から変更された行を返す。この情報は保存対象全体の取得には適しているが、今回の操作でどの状態からどの状態へ変わったかは表さない。 + +利用側が操作単位のAPI更新を作る場合、通知された行と比較するための行データをテーブルとは別に保持する必要がある。 + +## Goals / Non-Goals + +### Goals + +- 利用側が行データを二重管理せず、操作直前と確定後の行を取得できるようにする。 +- 補正された複数項目を含む最終行を、1操作の結果として通知する。 +- 拒否された変更候補と元の行を、行番号から引き直さず取得できるようにする。 +- 既存の`onChange`利用側との互換性を保つ。 + +### Non-Goals + +- APIリクエストや業務固有の更新イベントをライブラリ内で生成すること。 +- 非同期の行検証を追加すること。 +- 外部制御コンポーネントへ変更すること。 +- 累積変更を返す`getChangedRows()`の意味を変更すること。 + +## Decisions + +### 操作単位の確定通知を独立したコールバックとして追加する + +`onChange`は編集基準からの累積変更を通知する既存APIとして維持する。 + +操作単位の確定結果は、新しい`onRowChangeCommitted`で通知する。 + +既存コールバックへ引数を追加すると、引数を厳密に検証する利用側のテストへ影響する可能性がある。独立したコールバックにすることで、既存`onChange`の型と実行時の呼び出しを変更しない。 + +```ts +type CommittedRowChange = { + rowIndex: number + previousRow: T + row: T + changes: ReadonlyArray> +} + +type RowChangeCommit = { + source: 'edit' | 'paste' + rows: ReadonlyArray> +} + +type RejectedRowChange = { + rowIndex: number + previousRow: T + candidateRow: T + issues: ReadonlyArray> +} + +type RowChangeRejection = { + source: 'edit' | 'paste' + errors: ReadonlyArray + rows: ReadonlyArray> +} +``` + +行オブジェクトはテーブルが保持するスナップショットとして渡し、利用側は読み取り専用として扱う。 + +### 変更トランザクションは操作直前のStoreから作る + +編集確定直前に`store.getRows()`を取得し、確定後の行と比較してトランザクションを組み立てる。 + +これにより、`markAsSaved()`や編集基準からの累積変更に依存せず、今回の操作だけを表現できる。 + +`processRowChange`が補正した項目も、確定前後で値が異なる場合は`changes`へ含める。 + +### 拒否トランザクションは行変更準備中に保持する + +`prepareCellUpdates`は変更前の行、入力候補の行、`processRowChange`が返した拒否理由をすでに参照している。 + +拒否時にこの情報を破棄せず、セルエラーとともに`useSpreadSheetTable`へ返す。 + +複数行ペーストでは拒否された行をまとめ、ペースト全体が確定されなかったことを1回のコールバックで通知する。 + +## Risks / Trade-offs + +- 行オブジェクトをコールバックへ追加するため、通知データ量が増える。 + 対象は1操作で影響を受けた行だけに限定する。 +- `onChange`の累積変更と`onRowChangeCommitted`の操作変更は意味が異なる。 + コールバック名、型名、APIドキュメント、例で区別を明記する。 +- 行オブジェクトはStoreのスナップショットであり、利用側が変更すると混乱を招く。 + 公開型を`readonly`とし、ライブラリ内ではイミュータブルに扱う。 + +## Migration Plan + +1. 公開型とテストを追加する。 +2. 行変更準備処理から拒否コンテキストを返す。 +3. 確定前後の行から変更トランザクションを作る。 +4. 独立した確定コールバックと拒否コールバックから通知する。 +5. ドキュメントと利用例を更新する。 + +既存の`onChange`を変更しないため、利用側の一括移行は不要である。 diff --git a/openspec/changes/add-change-transaction-context/proposal.md b/openspec/changes/add-change-transaction-context/proposal.md new file mode 100644 index 0000000..03ee084 --- /dev/null +++ b/openspec/changes/add-change-transaction-context/proposal.md @@ -0,0 +1,22 @@ +# Change: 変更操作の行コンテキストを通知する + +## Why + +`onChange`は編集基準からの累積変更行だけを通知するため、利用側が操作直前と確定後の行を比較するには、テーブル内部と同じ行データを別途保持する必要がある。 + +`onRowChangeRejected`も行番号とセルエラーだけを通知するため、利用側は拒否された行の識別情報を自前の行データから引き直している。 + +テーブルがすでに保持している操作単位の行コンテキストを公開し、利用側による状態の二重管理を不要にする。 + +## What Changes + +- `onRowChangeCommitted`を追加し、操作種別と確定前後の行を操作単位で通知する。 +- `onRowChangeRejected`は、操作種別、セルエラー、変更前の行、変更候補の行、拒否理由を一つのオブジェクトで通知する。 +- 単一セル編集と複数セルペーストのどちらも、1操作につき1トランザクションを通知する。 +- 既存の`onChange`の型と呼び出し方は変更せず、既存利用側との互換性を保つ。 + +## Impact + +- Affected specs: `table-api` +- Affected code: public types、行変更準備、`useSpreadSheetTable`、APIドキュメント +- Consumer impact: 既存の`onChange`利用側は変更不要。操作単位の前後行が必要な利用側だけ新しいコールバックを使用する。 diff --git a/openspec/changes/add-change-transaction-context/specs/table-api/spec.md b/openspec/changes/add-change-transaction-context/specs/table-api/spec.md new file mode 100644 index 0000000..1bddb3b --- /dev/null +++ b/openspec/changes/add-change-transaction-context/specs/table-api/spec.md @@ -0,0 +1,69 @@ +## ADDED Requirements + +### Requirement: Row Change Commit Notification + +テーブルは、単一セル編集またはペーストによる変更を確定した際、`onRowChangeCommitted`へ今回の操作で確定した行変更を通知しなければならない（MUST）。 + +確定通知は操作種別、および影響を受けた各行の変更前の行、確定後の行、セル単位の変更詳細を含まなければならない（MUST）。 + +既存の`onChange`の型と呼び出し方を変更してはならない（MUST NOT）。 + +```typescript +type RowChangeCommit = { + source: 'edit' | 'paste' + rows: ReadonlyArray<{ + rowIndex: number + previousRow: T + row: T + changes: ReadonlyArray<{ + key: keyof T + previousValue: T[keyof T] + newValue: T[keyof T] + }> + }> +} +``` + +#### Scenario: Single cell edit notification + +- **WHEN** ユーザーが1つのセルを編集する +- **THEN** `onRowChangeCommitted`が1回だけ呼ばれる +- **AND** 操作種別`edit`と変更前後の行が渡される +- **AND** `changes`に今回変更したセルの変更前後の値が含まれる + +#### Scenario: Normalized row notification + +- **GIVEN** `processRowChange`が同じ行の別項目を補正する +- **WHEN** ユーザーがセル編集を確定する +- **THEN** 確定後の行に補正済みの値が含まれる +- **AND** `changes`に入力項目と補正項目が含まれる + +#### Scenario: Bulk paste notification + +- **WHEN** ユーザーが3行分のデータをペーストする +- **THEN** `onRowChangeCommitted`は1回だけ呼ばれる +- **AND** 操作種別は`paste`である +- **AND** 影響を受けた3行の変更前後の行が含まれる + +#### Scenario: Existing onChange compatibility + +- **GIVEN** 利用側が既存の`onChange`だけを指定している +- **WHEN** ユーザーがセルを編集する +- **THEN** `onChange`は従来と同じ1引数で呼ばれる + +### Requirement: Row Change Rejection Context + +`processRowChange`が行変更を拒否した場合、`onRowChangeRejected`は操作種別、セルエラー、および拒否された各行の変更前の行、変更候補の行、拒否理由を一つの拒否オブジェクトとして通知しなければならない（MUST）。 + +#### Scenario: Rejected single edit context + +- **WHEN** `processRowChange`が単一セル編集を拒否する +- **THEN** `onRowChangeRejected`が1回だけ呼ばれる +- **AND** 操作種別`edit`、セルエラー、変更前の行、変更候補の行、拒否理由が渡される + +#### Scenario: Rejected atomic paste context + +- **WHEN** 複数行ペーストの一部を`processRowChange`が拒否する +- **THEN** ペースト全体は確定されない +- **AND** `onRowChangeRejected`は1回だけ呼ばれる +- **AND** 拒否された各行の変更前の行、変更候補の行、拒否理由が含まれる diff --git a/openspec/changes/add-change-transaction-context/tasks.md b/openspec/changes/add-change-transaction-context/tasks.md new file mode 100644 index 0000000..d118860 --- /dev/null +++ b/openspec/changes/add-change-transaction-context/tasks.md @@ -0,0 +1,14 @@ +## 1. Implementation + +- [x] 1.1 単一編集の変更トランザクションをテストで固定する。 +- [x] 1.2 補正された複数項目と複数行ペーストのトランザクションをテストで固定する。 +- [x] 1.3 拒否トランザクションへ変更前の行と候補行が含まれることをテストで固定する。 +- [x] 1.4 公開型へ変更トランザクションと拒否トランザクションを追加する。 +- [x] 1.5 `useSpreadSheetTable`から独立した確定コールバックと拒否コールバックを通知する。 +- [x] 1.6 APIドキュメントと利用例を更新する。 +- [x] 1.7 typecheck、lint、test、buildを実行する。 + +## 2. Consumer Verification + +- [x] 2.1 利用側が行データの比較用コピーを保持せず、変更トランザクションからAPI更新を作れることを確認する。 +- [x] 2.2 利用側が拒否された行を行番号から引き直さず、検証エラーへ変換できることを確認する。 diff --git a/packages/core/src/core/types/index.ts b/packages/core/src/core/types/index.ts index 75ed4a7..18486d2 100644 --- a/packages/core/src/core/types/index.ts +++ b/packages/core/src/core/types/index.ts @@ -35,10 +35,14 @@ export type { CellChange, CellMeta, ChangeInfo, + CommittedRowChange, ProcessRowChange, ProcessRowChangeResult, + RejectedRowChange, + RowChangeCommit, RowChangeContext, RowChangeIssue, + RowChangeRejection, RowChangeSource, SpreadSheetTableProps, TableInstance, diff --git a/packages/core/src/core/types/table.ts b/packages/core/src/core/types/table.ts index 166565a..958dcd2 100644 --- a/packages/core/src/core/types/table.ts +++ b/packages/core/src/core/types/table.ts @@ -33,6 +33,31 @@ export type RowChangeIssue = { readonly result: ValidationResult } +export type CommittedRowChange = { + readonly rowIndex: number + readonly previousRow: T + readonly row: T + readonly changes: ReadonlyArray> +} + +export type RowChangeCommit = { + readonly source: RowChangeSource + readonly rows: ReadonlyArray> +} + +export type RejectedRowChange = { + readonly rowIndex: number + readonly previousRow: T + readonly candidateRow: T + readonly issues: ReadonlyArray> +} + +export type RowChangeRejection = { + readonly source: RowChangeSource + readonly errors: ReadonlyArray + readonly rows: ReadonlyArray> +} + export type ProcessRowChangeResult = | { readonly status: 'accepted'; readonly row: T } | { readonly status: 'rejected'; readonly issues: ReadonlyArray> } @@ -54,12 +79,13 @@ export type UseSpreadSheetTableOptions = { readonly sortable?: boolean readonly filterable?: boolean readonly onChange?: (changedRows: ReadonlyArray>) => void + readonly onRowChangeCommitted?: (commit: RowChangeCommit) => void readonly onSort?: (sortState: SortState) => void readonly onFilter?: (filterState: FilterState) => void readonly onValidationError?: (errors: ReadonlyArray) => void readonly validate?: (value: unknown, row: T, columnKey: keyof T) => ValidationResult | null readonly processRowChange?: ProcessRowChange - readonly onRowChangeRejected?: (errors: ReadonlyArray) => void + readonly onRowChangeRejected?: (rejection: RowChangeRejection) => void readonly reorderable?: boolean readonly onReorder?: (newData: ReadonlyArray) => void readonly resizable?: boolean diff --git a/packages/core/src/core/validation/row-change-processing.test.ts b/packages/core/src/core/validation/row-change-processing.test.ts index 3db98bf..5a6ddd9 100644 --- a/packages/core/src/core/validation/row-change-processing.test.ts +++ b/packages/core/src/core/validation/row-change-processing.test.ts @@ -78,6 +78,24 @@ describe('prepareCellUpdates', () => { result: { level: 'error', message: 'This shift overlaps another shift' }, }, ], + rejectedRows: [ + { + rowIndex: 1, + previousRow: rows[1], + candidateRow: { + id: 'shift-2', + startTime: '09:15', + endTime: '10:30', + note: '', + }, + issues: [ + { + columnKey: 'startTime', + result: { level: 'error', message: 'This shift overlaps another shift' }, + }, + ], + }, + ], }) }) @@ -101,4 +119,15 @@ describe('prepareCellUpdates', () => { note: '', }) }) + + it('rejects the transaction even when the processor returns no issues', () => { + const result = prepareCellUpdates( + rows, + [{ rowIndex: 0, columnKey: 'startTime', value: '09:15' }], + 'edit', + () => ({ status: 'rejected', issues: [] }), + ) + + expect(result.accepted).toBe(false) + }) }) diff --git a/packages/core/src/core/validation/row-change-processing.ts b/packages/core/src/core/validation/row-change-processing.ts index d629a4d..8f78c8a 100644 --- a/packages/core/src/core/validation/row-change-processing.ts +++ b/packages/core/src/core/validation/row-change-processing.ts @@ -1,9 +1,20 @@ -import type { CellChange, CellValidationError, ProcessRowChange, RowChangeSource } from '../types' +import type { + CellChange, + CellValidationError, + ProcessRowChange, + RejectedRowChange, + RowChangeCommit, + RowChangeSource, +} from '../types' import { applyCellUpdates, type CellUpdate } from './cell-update-validation' export type PreparedCellUpdates = | { readonly accepted: true; readonly updates: ReadonlyArray> } - | { readonly accepted: false; readonly errors: ReadonlyArray } + | { + readonly accepted: false + readonly errors: ReadonlyArray + readonly rejectedRows: ReadonlyArray> + } function getChangedKeys( previousRow: T, @@ -46,6 +57,7 @@ export function prepareCellUpdates( const affectedRowIndices = new Set(updates.map((update) => update.rowIndex)) const processedRows = new Map }>() const errors: CellValidationError[] = [] + const rejectedRows: RejectedRowChange[] = [] for (const rowIndex of affectedRowIndices) { const previousRow = rows[rowIndex] @@ -63,6 +75,7 @@ export function prepareCellUpdates( const result = processRowChange(candidateRow, previousRow, { rowIndex, source, changes }) if (result.status === 'rejected') { + rejectedRows.push({ rowIndex, previousRow, candidateRow, issues: result.issues }) for (const issue of result.issues) { errors.push({ rowIndex, @@ -76,7 +89,7 @@ export function prepareCellUpdates( processedRows.set(rowIndex, { row: result.row, changedKeys }) } - if (errors.length > 0) return { accepted: false, errors } + if (rejectedRows.length > 0) return { accepted: false, errors, rejectedRows } const preparedUpdates: CellUpdate[] = [] for (const [rowIndex, processed] of processedRows) { @@ -93,3 +106,31 @@ export function prepareCellUpdates( return { accepted: true, updates: preparedUpdates } } + +export function createRowChangeCommit( + rows: ReadonlyArray, + updates: ReadonlyArray>, + source: RowChangeSource, +): RowChangeCommit { + const committedRows = applyCellUpdates(rows, updates) + const affectedRowIndices = new Set(updates.map((update) => update.rowIndex)) + + return { + source, + rows: [...affectedRowIndices].flatMap((rowIndex) => { + const previousRow = rows[rowIndex] + const row = committedRows[rowIndex] + if (previousRow == null || row == null) return [] + + const changedKeys = getChangedKeys(previousRow, row, updates, rowIndex) + if (changedKeys.length === 0) return [] + + const changes: ReadonlyArray> = changedKeys.map((key) => ({ + key, + previousValue: previousRow[key], + newValue: row[key], + })) + return [{ rowIndex, previousRow, row, changes }] + }), + } +} diff --git a/packages/core/src/hooks/use-spread-sheet-table.test.tsx b/packages/core/src/hooks/use-spread-sheet-table.test.tsx index 5db3529..bf3a113 100644 --- a/packages/core/src/hooks/use-spread-sheet-table.test.tsx +++ b/packages/core/src/hooks/use-spread-sheet-table.test.tsx @@ -66,11 +66,13 @@ function renderTableHook(options: UseSpreadSheetTableOptions): { describe('useSpreadSheetTable row processing', () => { it('commits a normalized row as one undoable change', () => { const onChange = vi.fn() + const onRowChangeCommitted = vi.fn() const { getTable, root } = renderTableHook({ columns, initialData, rowKey: 'id', onChange, + onRowChangeCommitted, processRowChange: (candidate) => ({ status: 'accepted', row: { ...candidate, endTime: '10:00' }, @@ -85,6 +87,25 @@ describe('useSpreadSheetTable row processing', () => { endTime: '10:00', }) expect(onChange).toHaveBeenCalledOnce() + expect(onChange.mock.calls[0]).toHaveLength(1) + expect(onRowChangeCommitted).toHaveBeenCalledWith({ + source: 'edit', + rows: [ + { + rowIndex: 0, + previousRow: initialData[0], + row: { + id: 'shift-1', + startTime: '09:30', + endTime: '10:00', + }, + changes: [ + { key: 'startTime', previousValue: '09:00', newValue: '09:30' }, + { key: 'endTime', previousValue: '09:30', newValue: '10:00' }, + ], + }, + ], + }) act(() => getTable().undo()) expect(getTable().getData()[0]).toEqual(initialData[0]) @@ -116,13 +137,33 @@ describe('useSpreadSheetTable row processing', () => { expect(getTable().getData()).toEqual(initialData) expect(onChange).not.toHaveBeenCalled() - expect(onRowChangeRejected).toHaveBeenCalledWith([ - { - rowIndex: 0, - columnKey: 'startTime', - result: { level: 'error', message: 'This shift overlaps another shift' }, - }, - ]) + expect(onRowChangeRejected).toHaveBeenCalledWith({ + source: 'edit', + errors: [ + { + rowIndex: 0, + columnKey: 'startTime', + result: { level: 'error', message: 'This shift overlaps another shift' }, + }, + ], + rows: [ + { + rowIndex: 0, + previousRow: initialData[0], + candidateRow: { + id: 'shift-1', + startTime: '09:30', + endTime: '09:30', + }, + issues: [ + { + columnKey: 'startTime', + result: { level: 'error', message: 'This shift overlaps another shift' }, + }, + ], + }, + ], + }) act(() => root.unmount()) }) @@ -164,13 +205,103 @@ describe('useSpreadSheetTable row processing', () => { expect(result?.committed).toBe(false) expect(getTable().getData()).toEqual(initialData) expect(onChange).not.toHaveBeenCalled() - expect(onRowChangeRejected).toHaveBeenCalledWith([ - { - rowIndex: 1, - columnKey: 'startTime', - result: { level: 'error', message: 'This shift overlaps another shift' }, - }, - ]) + expect(onRowChangeRejected).toHaveBeenCalledWith({ + source: 'paste', + errors: [ + { + rowIndex: 1, + columnKey: 'startTime', + result: { level: 'error', message: 'This shift overlaps another shift' }, + }, + ], + rows: [ + { + rowIndex: 1, + previousRow: initialData[1], + candidateRow: { + id: 'shift-2', + startTime: '11:30', + endTime: '10:30', + }, + issues: [ + { + columnKey: 'startTime', + result: { level: 'error', message: 'This shift overlaps another shift' }, + }, + ], + }, + ], + }) + + act(() => root.unmount()) + }) + + it('reports a multi-row paste as one committed operation', () => { + const onChange = vi.fn() + const onRowChangeCommitted = vi.fn() + const { getTable, root } = renderTableHook({ + columns, + initialData, + rowKey: 'id', + onChange, + onRowChangeCommitted, + }) + + act(() => { + getTable().__handleBatchCellChanges([ + { rowIndex: 0, columnKey: 'startTime', value: '11:00' }, + { rowIndex: 1, columnKey: 'startTime', value: '11:30' }, + ]) + }) + + expect(onChange).toHaveBeenCalledOnce() + expect(onRowChangeCommitted).toHaveBeenCalledOnce() + expect(onRowChangeCommitted).toHaveBeenCalledWith({ + source: 'paste', + rows: [ + { + rowIndex: 0, + previousRow: initialData[0], + row: { ...initialData[0], startTime: '11:00' }, + changes: [{ key: 'startTime', previousValue: '09:00', newValue: '11:00' }], + }, + { + rowIndex: 1, + previousRow: initialData[1], + row: { ...initialData[1], startTime: '11:30' }, + changes: [{ key: 'startTime', previousValue: '10:00', newValue: '11:30' }], + }, + ], + }) + + act(() => root.unmount()) + }) + + it('reports a clear as a committed operation', () => { + const onRowChangeCommitted = vi.fn() + const { getTable, root } = renderTableHook({ + columns, + initialData, + rowKey: 'id', + onRowChangeCommitted, + }) + + act(() => { + getTable().__handleClearCells([{ rowIndex: 0, columnKey: 'startTime', value: '' }]) + }) + + expect(onRowChangeCommitted).toHaveBeenCalledOnce() + expect(onRowChangeCommitted).toHaveBeenCalledWith({ + source: 'clear', + rows: [ + { + rowIndex: 0, + previousRow: initialData[0], + row: { ...initialData[0], startTime: '' }, + changes: [{ key: 'startTime', previousValue: '09:00', newValue: '' }], + }, + ], + }) act(() => root.unmount()) }) @@ -178,6 +309,7 @@ describe('useSpreadSheetTable row processing', () => { it('rejects a multi-cell clear atomically', () => { const onChange = vi.fn() const onRowChangeRejected = vi.fn() + const onRowChangeCommitted = vi.fn() const processRowChange: ProcessRowChange = (candidate, _previous, context) => { if (context.rowIndex === 1) { return { @@ -198,6 +330,7 @@ describe('useSpreadSheetTable row processing', () => { rowKey: 'id', onChange, onRowChangeRejected, + onRowChangeCommitted, processRowChange, }) @@ -213,6 +346,7 @@ describe('useSpreadSheetTable row processing', () => { expect(getTable().getData()).toEqual(initialData) expect(onChange).not.toHaveBeenCalled() expect(onRowChangeRejected).toHaveBeenCalledOnce() + expect(onRowChangeCommitted).not.toHaveBeenCalled() act(() => root.unmount()) }) diff --git a/packages/core/src/hooks/use-spread-sheet-table.ts b/packages/core/src/hooks/use-spread-sheet-table.ts index 879395a..ca1a094 100644 --- a/packages/core/src/hooks/use-spread-sheet-table.ts +++ b/packages/core/src/hooks/use-spread-sheet-table.ts @@ -11,7 +11,7 @@ import type { } from '../core/types' import { isDataColumn } from '../core/types' import { type CellUpdate, validateCellUpdates } from '../core/validation/cell-update-validation' -import { prepareCellUpdates } from '../core/validation/row-change-processing' +import { createRowChangeCommit, prepareCellUpdates } from '../core/validation/row-change-processing' import { runValidation } from '../core/validation/validation-utils' export function useSpreadSheetTable(options: UseSpreadSheetTableOptions): TableInstance { @@ -50,14 +50,14 @@ export function useSpreadSheetTable(options: UseSpreadSheetTableOptions): source: RowChangeSource, validateBeforeCommit: boolean, ) => { - const prepared = prepareCellUpdates( - store.getRows(), - updates, - source, - options.processRowChange, - ) + const previousRows = store.getRows() + const prepared = prepareCellUpdates(previousRows, updates, source, options.processRowChange) if (!prepared.accepted) { - options.onRowChangeRejected?.(prepared.errors) + options.onRowChangeRejected?.({ + source, + errors: prepared.errors, + rows: prepared.rejectedRows, + }) return { committed: false as const, errors: prepared.errors } } @@ -68,12 +68,14 @@ export function useSpreadSheetTable(options: UseSpreadSheetTableOptions): return { committed: false as const, errors } } + const commit = createRowChangeCommit(previousRows, prepared.updates, source) store.beginBatch() for (const update of prepared.updates) { store.setCellValue(update.rowIndex, update.columnKey, update.value) } store.endBatch() if (prepared.updates.length > 0) options.onChange?.(store.getChangedRows()) + if (commit.rows.length > 0) options.onRowChangeCommitted?.(commit) return { committed: true as const, errors } }, @@ -81,6 +83,7 @@ export function useSpreadSheetTable(options: UseSpreadSheetTableOptions): store, options.columns, options.onChange, + options.onRowChangeCommitted, options.onRowChangeRejected, options.processRowChange, options.validate, diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 59e34f5..dbe4750 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -13,6 +13,7 @@ export type { CellValidationError, ChangeInfo, ColumnDef, + CommittedRowChange, DataColumnDef, DataColumnType, DateColumnDef, @@ -24,8 +25,11 @@ export type { NumberColumnDef, ProcessRowChange, ProcessRowChangeResult, + RejectedRowChange, + RowChangeCommit, RowChangeContext, RowChangeIssue, + RowChangeRejection, RowChangeSource, SelectionRange, SelectionState,