feat(datagrid): paste cells from clipboard (Cmd/Ctrl+V) - #612
Conversation
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (18 files)
Previous Review Summary (commit 65e772b)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 65e772b)Status: No Issues Found | Recommendation: Merge Files Reviewed (18 files)
Reviewed by glm-5.2 · Input: 53.5K · Output: 8.5K · Cached: 553.1K |
|
Hey @ymadd, thanks for this one, really solid work. I went through it locally today: checked out the branch, ran the full suite (3552 tests green, including your 23 new paste cases), tsc and eslint clean, and tested the paste flows by hand against a SQLite db (range paste, header round-trip, keyless table, alias columns, quoted CSV, and the no-op detection when you paste the original value back on its own cell). Everything behaved exactly as described in the PR. The functional update fix on A few things before merging:
Rebase it and I'm happy to approve. Points 2 and 3 can be follow-ups. |
handlePendingChange / handlePendingInsertionChange computed the next pending state from a tabsRef snapshot taken before the update. When many cells are staged in the same tick (e.g. a multi-cell clipboard paste), every call reads the same stale snapshot and React batches the setState calls, so only the last cell's change survives. updateTab now also accepts an updater function ((tab) => partial) and both handlers compute the next pending state inside it, so rapid successive updates compose instead of clobbering each other.
Copying cells has been supported for a while, but there was no way to paste back into the grid. This adds spreadsheet-style paste as staged edits (pending changes), applied via the existing commit/rollback flow: - Cmd/Ctrl+V pastes at the selection: the cell range's top-left, the top of the row selection, or the focused cell. A context-menu Paste entry pastes at the right-clicked cell. - Tab-separated cells win (spreadsheet convention). Multi-line text without tabs is parsed as CSV with double-quote escaping, preferring the configured CSV delimiter, so the grid's own copy formats (comma/semicolon/pipe) round-trip. A single line without tabs is always one value, so free text like "hello, world" lands in one cell. - A leading header row is dropped when every cell matches a column name (round-trip of the "export column names" option). - A single copied value fills the whole selected range / selected rows. - The paste matrix is clipped at the grid edges; existing rows require an identifiable key (insertion rows accept a paste regardless). - Pasting a cell's original value back clears its pending change, same as inline editing.
- Exclude primary key columns from the single-value row-selection fill: a whole-row fill overwriting row identities is never what the user meant. Explicitly selected cells (range / focused cell) still accept PK values. - Skip database-generated columns on paste, matching the inline-editing guard that main gained since this branch was cut. - Surface clipboard read failures as an error toast instead of only console.error.
65e772b to
ef57700
Compare
|
Thanks for the thorough review @debba — much appreciated, especially the hands-on testing! All addressed and pushed:
Re-verified after the rebase: full suite green (the 3 suites that fail to load do so on main as well), |
|
@ymadd Do you think it would make sense to support pasting as new rows as well? One possible approach would be to route the same Paste command based on the clipboard shape: partial matrices would keep updating existing cells, while full-width rows could become staged insertions. Alternatively, a separate explicit “Paste as new rows” action would avoid ambiguous cases. Would you prefer to explore this within the current implementation or in a follow-up PR? If a follow-up sounds better, I’m happy to merge this PR as it is in the meantime. |
|
Great idea — I'd take that on, but as a follow-up PR with an explicit "Paste as new rows" action in the context menu, rather than routing Cmd/Ctrl+V by clipboard shape. Shape alone isn't a reliable signal of intent: a full-width matrix can still be meant to update existing rows, while a perfectly valid insertion may omit generated, auto-increment, or defaulted columns. Keeping Cmd/Ctrl+V as an update operation keeps it predictable, and the explicit action gives the append case a clear home — still staged through the pending-changes flow. It also crosses the current DataGrid/Editor boundary: the grid can edit existing pending-insertion rows and trigger the creation of a single one via So yes please — happy to have this merged as is. I'll file the follow-up issues right after (paste-as-new-rows, plus the BLOB-column guard you flagged) and pick both up. |
|
Merging it right now . |
2 participants
Closes #611
Summary
The data grid supports copying cells/rows/ranges but not pasting. This PR adds spreadsheet-style paste as staged edits — pasted values go through the existing pending-changes flow (apply/rollback), never directly to the database.
Behavior
hello, worldlands in one cell.Prerequisite fix (first commit)
handlePendingChange/handlePendingInsertionChangecomputed the next pending state from atabsRefsnapshot; with N staged cells in one tick, React batches the updates and only the last cell survived.updateTabnow also accepts an updater function and both handlers compute inside it. Multi-cell paste is the first caller that hits this, but it hardens every rapid-succession staging path.Known limitations
rowToCSV), so values containing the delimiter don't survive a copy→paste round-trip. Robust CSV serialization is follow-up material.NULLcopies as the literal stringnull, so pasting it into another cell stages that string, not SQL NULL.Testing
tests/utils/dataGrid.test.ts: 23 new cases forparsePasteMatrix/stripHeaderRow/computePasteTargets(TSV/CSV/quotes/delimiter detection/positional header stripping/range fill/clipping).tsc --noEmit,eslint, and the vitest suite are green (the 3 failing files onmain— ThemeProvider/SettingsProvider/useSidebarResize — fail identically without this change).i18n
3 new keys (
pasteCells,pastedCells,pasteNotEditable) added to all 11 locales.