feat(QueryResultTable): add per-row clipboard copy button to query re…#3741
feat(QueryResultTable): add per-row clipboard copy button to query re…#3741Arunkoo wants to merge 4 commits intoydb-platform:mainfrom
Conversation
…olumn name, i18n tooltip, deduplicate copyColumn
81e7e61 to
29408c4
Compare
|
Since I’m not familiar with Russian, I used Google Translate for the changes in ru.json. Please feel free to suggest corrections. |
| .map((v) => (typeof v === 'object' ? JSON.stringify(v) : String(v))) | ||
| .join('\t'); | ||
|
|
||
| const copyColumn: Column<KeyValueRow> = { |
There was a problem hiding this comment.
Avoid a fixed synthetic column name for the copy action
This action column uses a hard-coded name, copy_action, but query results can legally contain a column with the same alias. In that case QueryResultTable passes duplicate column names to react-data-table, which uses column names as accessors and state keys, so a query like SELECT 1 AS "copy_action" can make the real result column ambiguous and lead to the wrong data being extracted or rendered for one of the columns.
There was a problem hiding this comment.
@astandrik The fixed copy column name is now generated dynamically inside useMemo by checking against actual column names in the result. Starts with 'copy', and if that already exists as a query result column it wraps to '_copy_', then '__copy__' and so on until a unique name is found. This guarantees no collision with any real query column name regardless of what the user queries.
… collision with query result columns
…o preserve empty state message
…lean up default column name
|
Thanks for the pull request But actually I carefully reread parent ticket and its unclear what was the main @adameat's idea - to copy the whole result? To copy row as its done in this pr? Or per-cell copy. The main ticket lacks explanation of what is intended to be as a result and design :_( Our solution is merged to main ydb backend as a fronted view and then installed as internal/cloud and provided as on-prem solution to big companies - so its crucial to make weighted and designed solutions very sorry for that poor description of parent ticket(( this ticket really needs product work and design cc @Raubzeug |
|
@astandrik Thank you for the detailed feedback and for taking the time to review! I completely understand since the ticket lacks a clear design spec, |
Closes #3691
Summary
Adds a copy button to each row in the query results table, allowing users to copy an individual row's data to the clipboard as a tab-separated string with a single click.
Problem
Previously the only clipboard option was the global copy button in the toolbar which copies the entire result set. There was no way to quickly extract a single row without manually selecting text across multiple cells.
Solution
Appended a fixed-width action column containing a ClipboardButton to every row in the query results table. The button serializes the row using the existing buildTsvBlobParts utility ensuring the copied format is consistent with the full result set copy path.
###Screenshot

Changes
Key decisions
is identical to the existing full result copy — no duplicated logic
actual result column names, guaranteeing no collision even if the user
queries a column named 'copy'
states internally — no extra state management needed
Greptile Summary
This PR adds a per-row copy-to-clipboard button to the
QueryResultTablecomponent, serializing each row as a tab-separated string using the existingbuildTsvBlobPartsutility for consistent formatting with the full-table export path.Previous review concerns (variable shadowing on
data/rowData, hardcoded English strings, duplicatecopyColumndefinition, and'copy'name collisions) have all been addressed in the current revision. Two minor style-level issues remain:buildTsvBlobPartsutility is imported fromsrc/containers/Tenant/Query/utils/, creating a cross-layer dependency from a sharedcomponents/file into a feature-specificcontainers/path. Moving the utility to a neutral shared location would remove this coupling.rowToTsvuses.slice(2)as a magic index intobuildTsvBlobParts's internal output structure, which is silently fragile if that function's output format ever changes.Confidence Score: 5/5
Safe to merge — all previously flagged bugs are resolved and only P2 style suggestions remain.
All P0/P1 issues from earlier review rounds (variable shadowing, hardcoded strings, duplicate column definition, name collision) are fixed. The two remaining findings are purely P2 style concerns (cross-layer import and a magic-number slice index) that do not affect correctness or user-facing behavior.
src/components/QueryResultTable/QueryResultTable.tsx — cross-layer import and slice(2) fragility are worth a follow-up, but neither blocks this merge.
Important Files Changed
Sequence Diagram
sequenceDiagram participant User participant ClipboardButton participant rowToTsv participant buildTsvBlobParts User->>ClipboardButton: click copy icon on a row ClipboardButton->>rowToTsv: rowToTsv(row) rowToTsv->>buildTsvBlobParts: buildTsvBlobParts([row]) buildTsvBlobParts-->>rowToTsv: [headers, "\n", rowValues] rowToTsv-->>ClipboardButton: rowValues string (.slice(2).join("")) ClipboardButton->>User: writes rowValues to clipboardComments Outside Diff (2)
src/components/QueryResultTable/QueryResultTable.tsx, line 141-143 (link)preparedColumnsalways contains at least the copy column, so!preparedColumns.lengthis permanentlyfalse. When a query returns zero rows (generic path with nocolumnsprop), instead of the"Table is empty"message, users see a blank table with an invisible header — a noticeable UX regression.The fix is to base the empty check on the data columns, not the final column list:
With this approach the copy column is only appended when there are real data columns, so the
!preparedColumns.lengthguard on line 141 fires correctly on empty results.Prompt To Fix With AI
src/components/QueryResultTable/QueryResultTable.tsx, line 34 (link)Copied text includes column headers, not just cell values
buildTsvBlobParts([row]).join('')produces two lines:The first line is the header row (column names), derived from
Object.keys(data[0])insidebuildTsvBlobParts. For a "Copy row" action, users almost certainly expect just the cell values — not a header+data pair. Pasting the two-line result into, e.g., a spreadsheet cell or an SQL statement will include the unwanted header line.If column names in the copied output are desirable, this should be documented/communicated to users. Otherwise, serialise only the values directly, e.g.:
(This would also need the same
escapeValuelogic used bybuildTsvBlobPartsfor correctness.)Prompt To Fix All With AI
Reviews (5): Last reviewed commit: "fix(QueryResultTable): exclude header ro..." | Re-trigger Greptile