-
Notifications
You must be signed in to change notification settings - Fork 1
feat(data-table): inline cell editing #560
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
itsprade
wants to merge
12
commits into
main
Choose a base branch
from
feat/core/1750-datatable-inline-edit
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+3,964
−86
Open
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
6df0b3b
docs(data-table): propose inline cell editing
itsprade 2d2f81f
feat(data-table): add cell edit rules and keyboard navigation helpers
itsprade 3182fdb
feat(data-table): inline cell editing for text, number, money and lin…
itsprade cb1e072
docs(data-table): document inline editing and add a goods-receipt demo
itsprade 181871b
feat(data-table): dropdown, badge and date cell editors
itsprade 1408647
fix(data-table): address pre-push review of inline editing
itsprade 4e4f7f2
feat(data-table): spreadsheet-style editable cells
itsprade dd93733
fix(data-table): open an empty bounded calendar on a pickable month
itsprade 50d4729
docs(data-table): more inline-editing examples in the lab demo
itsprade 3185e7e
feat(data-table): keep a space for dropdown and date icons
itsprade e7b094f
fix(select): add inner padding to the dropdown
itsprade 40d8ebd
docs(data-table): note in the decision record that PR 2 is folded in
itsprade File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| --- | ||
| "@tailor-platform/app-shell": minor | ||
| --- | ||
|
|
||
| Add inline cell editing to `DataTable`. Give a typed column an `edit` config and users can change its values right in the list: `text`, `number` and `money` cells are typed into; `text` / `link` columns with `edit.options` and `badge` columns become dropdowns; `date` columns open a calendar. Rules (`min`, `max`, `maxDecimals`, `required`, `validate`), per-row control (`canEdit(row, { selected })`) and Enter / Tab keyboard entry come built in. The value reaches `edit.onCommit(row, value)` when the user leaves the cell or picks a choice; return a promise to autosave, and the cell reverts if it rejects. | ||
|
|
||
| ```tsx | ||
| column({ | ||
| id: "received", | ||
| label: "Received", | ||
| type: "number", | ||
| edit: { | ||
| canEdit: (row, { selected }) => selected, | ||
| min: 0, | ||
| maxDecimals: 0, // whole numbers only | ||
| onCommit: (row, value) => updateLine(row.id, { received: value }), | ||
| }, | ||
| }); | ||
|
|
||
| column({ | ||
| id: "supplierId", | ||
| label: "Supplier", | ||
| type: "text", | ||
| edit: { | ||
| options: suppliers.map((s) => ({ value: s.id, label: s.name })), | ||
| onCommit: (row, value) => updateLine(row.id, { supplierId: value }), | ||
| }, | ||
| }); | ||
| ``` | ||
|
|
||
| Also fixes date display: a date-only `"YYYY-MM-DD"` value now shows that day in every time zone instead of the day before west of UTC. A `number` / `money` column with `edit.maxDecimals` displays up to that many decimals, and a `number` column whose `maxDecimals` is below its `minDecimals` no longer throws. | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@tailor-platform/app-shell": patch | ||
| --- | ||
|
|
||
| `Select`'s dropdown now has a 4px inner gap, like `Menu`, `Combobox` and `Autocomplete`, so the highlighted option no longer touches the dropdown's edges. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,161 @@ | ||
| # Decision: inline cell editing in DataTable | ||
|
|
||
| > Status: **Open — for team input.** Every column type is implemented in this PR (text, number, money, link, dropdowns via `edit.options`, badge dropdowns and dates), so the proposal can be tried in the vite example (`/showcase/data-table-lab`) while the questions below are settled. | ||
| > | ||
| > Context: [platform-planning#1750](https://github.com/tailor-inc/platform-planning/issues/1750) (the UI Catalogue inline-edit pattern, left with @itsprade), the Larson IMS request in Slack (`#prj-larson-ims`), and [platform-planning#1428](https://github.com/tailor-inc/platform-planning/issues/1428). Related: [platform-planning#1115](https://github.com/tailor-inc/platform-planning/issues/1115) (optimistic rows) and [platform-planning#1161](https://github.com/tailor-inc/platform-planning/issues/1161) (LineItems). | ||
| > | ||
| > Scope: editing values in rows a DataTable already shows. Adding, removing or reordering rows is out of scope; that's document line items (#1161). | ||
|
|
||
| ## Problem | ||
|
|
||
| Two teams have asked for the same thing: | ||
|
|
||
| - **Larson IMS (Slack).** They're moving about a dozen AG-Grid tables to `DataTable`. Regular lists already work, including filtering, sorting, pagination and row actions. The blocked tables are the ones where people fill in quantities: purchase orders, invoices, receipts, credit notes and stock adjustments. They need two things: | ||
| 1. **Typing into a cell, with basic rules**, such as whole numbers only, no negatives, or at most 2 decimal places. | ||
| 2. **Some rows editable and others not**, decided by their own logic. For example, a row becomes editable once it's selected, or only while it meets a condition. | ||
| - **Opportunity tracker ([#1428](https://github.com/tailor-inc/platform-planning/issues/1428)).** They hand-built popover + save + refetch cells for text, enum and decimal values. They're asking for one standard version, so every app looks and saves the same way. | ||
|
|
||
| Today DataTable only displays data. An app that needs editing draws its own inputs in `render`. That loses DataTable's typed formatting and fights with its row click, cell context menu and truncation. | ||
|
|
||
| ## What exists today | ||
|
|
||
| - **DataTable is display-only.** Rows belong to the screen: DataTable renders `data.rows` as given and leaves sorting, filtering and paging to `CollectionControl`. There is no notion of cell focus or keyboard movement. | ||
| - **UI Catalogue pattern: "DataTable inline cell edit"** ([ui.tailor.tech](https://ui.tailor.tech/patterns/datatable-inline-edit), [#1750](https://github.com/tailor-inc/platform-planning/issues/1750)). It's built on DataTable `render`: | ||
| - Underlined values open a small popover with the input and Cancel / Save. | ||
| - Status is a badge plus a chevron menu; picking an option applies it immediately and shows a toast. | ||
| - A "Confirmed" flag commits through a checkbox. | ||
| - Its guidance: it suits a few edits on small lists. Use a Sheet or form for cross-field checks or large row counts. | ||
| - **The `list-dense-scan` pattern rules it out.** `docs-src/patterns/list-dense-scan.docs.outline.md` lines 32 and 89 say: "Inline editable cells — use pattern/detail or pattern/form/modal instead". | ||
| - **LineItems** ([#1161](https://github.com/tailor-inc/platform-planning/issues/1161), unmerged app-shell#238) is a document-line editor. It has cell editors but no validation rules, no per-row control, and no filtering or paging. That makes it a reference, not the base for list tables. | ||
| - **[#1115](https://github.com/tailor-inc/platform-planning/issues/1115)**: DataTable manages rendering state, and saving with optimistic updates and rollback belongs in a separate `useOptimisticRows` hook. The mutation callbacks were removed from `useDataTable` in app-shell#130. | ||
| - **`CsvImporter`**'s review grid, with `onCellEdit(row, columnKey, value)`, is the only inline editing in the package today. | ||
|
|
||
| ## Proposal | ||
|
|
||
| Build inline editing into DataTable and configure it per column. | ||
|
|
||
| 1. **Type straight into a cell.** Click or Tab into an editable cell and type. There's no popover and no form. This is where the proposal differs from the catalogue pattern (see [Alternatives](#alternatives-considered)). | ||
| 2. **Show what's editable — spreadsheet-style.** | ||
| - Editable cells look like the rest of the table: no input boxes, and the whole cell is the click target. | ||
| - The cursor tells cells apart: a text cursor for typing cells, a pointer for dropdown and date cells, and "not allowed" for cells that can't be edited. | ||
| - The cell being edited outlines its edges; dropdown and date icons appear on hover and focus, in a space kept free at the right edge so they never cover the value. | ||
| - Row height and column width don't shift when a row becomes editable. | ||
| 3. **Block impossible input as it's typed.** | ||
| - With "no negatives" there's no minus sign. With "whole numbers" there's no decimal point. With "2 decimals" there's no third decimal digit. | ||
| - Pasted text is cleaned up (thousands separators, full-width digits) and then checked. It is never silently rounded. | ||
| 4. **Explain the other rules.** | ||
| - A required value, a maximum, or the screen's own rule ("can't receive more than ordered") turns the cell red, with a tooltip saying why. | ||
| - Enter and Tab won't save until it's fixed, and Esc puts the old value back. | ||
| - Leaving the cell while it's still invalid reverts it. | ||
| 5. **Let the screen decide which rows are editable.** The per-row check also knows whether the row is selected, so "editable once ticked" and "only while Draft" are one line each. | ||
| 6. **Make keyboard data entry fast.** | ||
| - Enter saves and moves down to the same column. | ||
| - Tab saves and moves to the next editable cell, skipping read-only ones. | ||
| - Esc undoes. | ||
| - Japanese IME input is respected: pressing Enter to confirm a conversion doesn't save. | ||
| 7. **Autosave on leave.** | ||
| - A value saves when the user leaves the cell or presses Enter or Tab, and only if it actually changed: going from `10` to `10.00` is not a change. | ||
| - There is no saving indicator. | ||
| - If the screen's save fails, the cell goes back to the old value, and the screen can show its own toast. | ||
| - Screens that hold edits for a Save button work the same way; their save just never fails. | ||
| 8. **Leave everything else alone.** Filtering, sorting, paging, selection, row actions, pinned columns and column settings keep working. Existing tables don't change unless a column opts in. | ||
|
|
||
| ### Every column type | ||
|
|
||
| | Column type | How it's edited | What the screen receives | | ||
| | ---------------- | ------------------------------------------------------------ | ----------------------------------------------------------------- | | ||
| | `text` | Type in the cell | Text, or `null` when emptied | | ||
| | `number` | Type in the cell, with the rules above | A number, or `null` | | ||
| | `money` | Same as number; decimals follow the currency (USD 2, JPY 0) | A number, or `null` | | ||
| | `date` | Pick from a calendar or type; date-time columns add the time | `"YYYY-MM-DD"`, a UTC ISO string for date-time columns, or `null` | | ||
| | `badge` (status) | Pick from a list, either one value or several | The value (or list of values), or `null` | | ||
| | `link` | Edit the text; it shows as plain text while editable | Text, or `null` | | ||
|
|
||
| ## API sketch | ||
|
|
||
| This follows the shapes DataTable already has: | ||
|
|
||
| - **`edit` sits on the column next to `type` / `typeOptions`** and narrows by `type`, like `typeOptions` and `accessor` do today (`packages/core/src/components/data-table/types.ts`). That means the value handed to `onCommit` has the right type for each column type. | ||
| - **`canEdit(row, { selected })`** mirrors `rowExpansion.canExpand(row)` and `RowAction.isDisabled(row)`. | ||
| - **Nothing changes in the `useDataTable` options or `DataTableContextValue`.** | ||
|
|
||
| ```tsx | ||
| const { column } = createColumnHelper<ReceiptLine>(); | ||
|
|
||
| column({ | ||
| id: "received", | ||
| label: "Received", | ||
| type: "number", | ||
| edit: { | ||
| canEdit: (row, { selected }) => selected && row.status === "open", | ||
| min: 0, | ||
| maxDecimals: 0, // whole numbers only | ||
| required: true, | ||
| validate: (value, row) => | ||
| value !== null && value > row.ordered ? `Can't exceed ordered (${row.ordered})` : undefined, | ||
| // May return a promise; if it rejects, the cell reverts. | ||
| onCommit: (row, value) => saveReceivedQty(row.id, value), | ||
| }, | ||
| }); | ||
| ``` | ||
|
|
||
| - **The table never stores edits.** It calls `onCommit`, and the screen updates `data`: a local draft for a Save-button screen, or a mutation for autosave. While an `onCommit` promise is pending, the cell keeps showing the new value; if the promise rejects, the cell reverts. Richer optimistic behaviour stays with `useOptimisticRows` (#1115). | ||
| - **Rules by type:** | ||
| - `text` / `link`: `required` and `validate` only. | ||
| - `number` / `money`: `min`, `max` and `maxDecimals`. `maxDecimals` defaults to what the column displays; for money, the currency's decimals. | ||
| - `date`: `min` and `max`. | ||
| - `badge`: `options` (defaults to the column's enum filter options) and `multiple`. | ||
| - **Built from existing components:** `Input`, `DatePicker`, `Select`, `Tooltip` and `Badge`. There are no new dependencies. Rows need an `id` to be editable. | ||
|
|
||
| ## Scope contract | ||
|
|
||
| - **Required behaviour:** Larson IMS's received-quantity column works on a paginated, filtered DataTable. It must be integer, ≥ 0 and ≤ ordered, editable only when the row is selected, entered row by row with Enter, and autosaved with a revert on failure. | ||
| - **Compatibility:** | ||
| - No change to existing tables, the `useDataTable` options or the context. `edit` is optional on every column. | ||
| - One display fix comes with it: a number column shows as many decimals as its editor allows. Today an unconfigured number column rounds to whole numbers, so a typed 2.5 would render as "3". | ||
| - **Intentionally unsupported in v1:** | ||
| - yes/no flags (there's no boolean column type) | ||
| - a bring-your-own editor for custom `render` columns | ||
| - copying and pasting across several cells, fill-down, and undo | ||
| - moving between cells with the arrow keys | ||
| - a "changed" marker | ||
| - a saving indicator | ||
| - comma decimal separators (`1,5`) | ||
| - adding or removing rows | ||
| - **Validation plan:** | ||
| - Unit tests for parsing and rules. | ||
| - Interaction tests for each column type: commit, revert, blocked keys, keyboard flow, per-row control and no-op saves. | ||
| - Type tests for the value each column type receives. | ||
| - A goods-receipt demo in the vite example, checked in the browser, including that row height stays fixed. | ||
|
|
||
| ## Alternatives considered | ||
|
|
||
| - **Keep it a pattern (#1750 as it is).** No package change, but every app rebuilds the rules, per-row checks and keyboard flow, which is the per-app drift #1428 describes. | ||
| - **Popover editing, like the catalogue pattern.** This matches the existing pattern and suits occasional changes. But a click and a Save for every cell is too slow for filling in quantities down a column, which is what Larson IMS needs. | ||
| - **DataTable holds the drafts** (dirty tracking, "get changes"). This goes against #1115's direction that DataTable only renders. The screen already owns the rows. | ||
| - **Build on LineItems.** It's unmerged, it has no validation or per-row control, and it has no filtering or paging. | ||
|
|
||
| ## Delivery | ||
|
|
||
| - **PR 1, which unblocks Larson IMS:** | ||
| - text, number, money and link editing | ||
| - rules and errors | ||
| - per-row control | ||
| - keyboard entry | ||
| - autosave and revert | ||
| - docs (`docs-src/components/data-table.docs.outline.md`) | ||
| - pattern updates: rewrite `list-dense-scan`, and move the #1750 inline-edit pattern into `docs-src/patterns/` | ||
| - a vite example | ||
| - a `minor` changeset | ||
| - **PR 2 (folded in):** date and badge editing, plus dropdowns for `text` / `link` columns via `edit.options`, with the matching docs and example additions — built in the same PR so every column type can be reviewed together. | ||
|
|
||
| ## For the call | ||
|
|
||
| - **Naming.** `edit` / `canEdit` / `onCommit`, or Base UI's `onValueCommitted` wording? | ||
| - **Leaving an invalid cell reverts it.** This is AG-Grid's default, and it means the data never disagrees with the screen, but the typed value is lost. Keep it? The other option is keeping the red value until it's fixed, and giving screens a way to ask "any invalid cells?" before Save. | ||
| - **Enter moves down** to the same column, spreadsheet-style, instead of staying put. OK? | ||
| - **One PR.** Date and badge editing were planned as a follow-up but are folded in, so every column type is reviewed together. OK, or split them back out? | ||
| - **#1115.** Ship the simple pending/revert behaviour with PR 1 and let `useOptimisticRows` build on it later, or wait for #1115? | ||
| - **Catalogue pattern.** Move #1750 into `docs-src/patterns/`, rewritten around the built-in feature, and retire the popover version? | ||
| - **Bring-your-own editor** (for flags, product search and similar): leave it out until a team asks? | ||
| - **Out of scope here.** The two open notes in `docs-src/pages/document-detail.docs.outline.md`, "a shared line-items component" and "in-place editing versus an edit route", stay open. This proposal covers list tables only. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we avoid exposing
min,max,maxDecimals,required, and a separatevalidatecallback here?I would prefer
edit.schemato accept a Standard Schema-compatible validator, following the existingCsvImporterpattern. That gives consumers one validation and normalization point, rather than adding DataTable-specific validation rules as public API.For example, using Zod purely as a Standard Schema-compatible implementation (DataTable itself would not need a Zod dependency):
The same mechanism can normalize a value before it is committed:
I think the table should keep the raw draft untouched while the user is typing, then parse it for the column type and run the schema on Enter, Tab, or blur. On success, it should commit the schema output; on failure, it should leave the external value unchanged and surface the schema issue. In particular, we should not transform on every keystroke, since that tends to break IME composition and cursor position.
Before settling that contract, I think we should explicitly resolve the following:
onCommit/server failure means “the value could not be saved.” These should not be presented as the same state.row. If row-dependent rules remain supported, we need to decide how that context reaches the schema (for example,schema: (row) => schema).aria-invalid, and an associated accessible error description.This is a larger API decision than replacing
validate, but I think it produces a cleaner and more reusable public contract than adding the individual options.