fix(grid): make inline cell editing usable — stop row-nav on edit, type-aware editors, default persist#2021
Merged
Merged
Conversation
…pe-aware editors, default persist Inline editing was opt-in and MVP-level; turning it on for a 报工 grid surfaced three problems. This fixes all three. A) Bug: clicking an editable cell also opened the record-detail drawer. The cell onClick/onDoubleClick started edit mode but didn't stop propagation, so the row onClick still ran onRowClick. The row's "ignore interactive elements" heuristic missed it because the <input> only renders on the next frame (the target is still a <td> at click time). Now stopPropagation() is called when entering edit mode, so an editable cell only edits. Non-editable cells still navigate. B) Unfinished: the editor was a hardcoded text <Input> for every type. Added a small, readable, type-aware switch keyed on col.type: date → <input type="date">, datetime → <input type="datetime-local"> (with local⇄ISO conversion), number/currency/percent → number input, else the original text input. ObjectGrid now forwards the resolved column type onto every data-table column build path so the editor can read it. select/boolean left as documented extension points. C) Missing default: edits didn't persist for declarative editable views. ObjectGrid passed onRowSave/onBatchSave straight through with no default, so a declarative editable:true view saved to nothing. Added dataSource-backed defaults (dataSource.update per row, keyed on _id ?? id, refresh after) installed only when the consumer omits the prop. They throw on failure so DataTable keeps pending changes instead of silently clearing them.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
…ell, type-aware editors)
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
Inline cell editing in the grid was opt-in (
editable: schema.editable ?? false) and shipped at MVP level. Turning it on for a 报工 grid surfaced three problems. This PR fixes all three, keeping the diff focused on those fixes.A — Bug: clicking an editable cell also opened the record-detail drawer
Root cause: The
<TableCell>onClick/onDoubleClickstarted edit mode but did not stop propagation. The<TableRow>onClickthen firedonRowClick(row)→ navigation drawer. The row has a heuristic that ignores clicks oninput/button/menu/…, but at click time the cell is still a<td>(the<input>only renders on the next frame), so the heuristic missed it and navigation fired anyway.Fix: In the cell
onClick/onDoubleClick, calle.stopPropagation()when actually entering edit mode. Clicking an editable cell now only enters edit mode (no drawer). Non-editable cells in an editable grid still navigate as before (covered by the existingdata-table-row-click"plain text cell still calls onRowClick" test).B — Unfinished: the editor was a hardcoded text
<Input>for every field typeRoot cause: The
isEditingblock rendered a single text<Input>regardless of field type — no date picker, no number input. Also,col.typewas not forwarded fromObjectGridonto the data-table columns, so the editor had nothing to switch on.Fix:
ObjectGridnow forwards the resolved column type onto every column-build path (accessorKey pass-through, ListColumn, string-array, inline-data, and objectSchema-generated columns) astype.date→<input type="date">(native picker), value normalized toyyyy-MM-dd; a bareyyyy-MM-ddis passed through verbatim to avoid a UTC-parse day shift.datetime/datetime-local→<input type="datetime-local">, converting local ⇄ ISO so display/format code (formatCellValue) stays consistent.number/currency/percent(+ int/float aliases) →<Input type="number">.<Input>(unchanged fallback).select/booleanare intentionally left as documented extension points (the data-table column does not yet carry option metadata). Keyboard handling (handleEditKeyDown: Enter saves, Esc cancels) is preserved; native date inputs commit on change.C — Missing default: edits didn't persist for declarative editable views
Root cause:
onRowSave/onBatchSavewere passed straight through as props with no default, so a declaratively-configurededitable: trueview (no React host wiring) saved to nothing — "Save All" just cleared pending changes.Fix:
ObjectGridnow supplies dataSource-backed defaults, installed only when the consumer omits the prop (onRowSave={onRowSave ?? defaultRowSave}):defaultRowSave→dataSource.update(objectName, id, changes)whereid = row._id ?? row.id, then refresh.defaultBatchSave→ oneupdateper modified row (thebulk/bulkUpdateprimitives apply a single uniform patch and don't fit per-row edits), then refresh.saveRow/saveBatchkeep pending changes instead of silently clearing them.Files
packages/components/src/renderers/complex/data-table.tsx— stopPropagation on edit entry; type-aware editor switch + date/datetime conversion helpers.packages/plugin-grid/src/ObjectGrid.tsx— forwardcol.typeon all column-build paths; defaultonRowSave/onBatchSaveviadataSource.Verification
pnpm --filter @object-ui/components type-check→ clean (0 errors).pnpm --filter @object-ui/plugin-grid build→ built (dist/index.js); no new type errors from these changes (two pre-existing i18nt()Yes/No overload warnings at ObjectGrid.tsx:685-686 are untouched).data-table-row-click,data-table,data-table-manual-pagination) → 13 passed, incl. the row-click regression suite.no-explicit-any/react-hooks patterns consistent with the surrounding files).Behavioral verification (jsdom, real patched component)
A live console screenshot wasn't feasible in the sandbox (console needs a backend + auth), so the two fixes were verified by rendering the actual patched
data-tablein jsdom and asserting the exact behaviors from the bug reports. Added as a regression test (data-table-inline-edit.test.tsx, commit133fad634) — 7/7 pass:onRowClick(no detail drawer)editable:false) cell still firesonRowClick(no regression)type:'date'column renders a native date picker (<input type="date">), not free texttype:'number'column renders a numeric inputRendered editor markup produced by the patch: