Skip to content

feat(QueryResultTable): add per-row clipboard copy button to query re…#3741

Open
Arunkoo wants to merge 4 commits intoydb-platform:mainfrom
Arunkoo:feature/add-copy-button
Open

feat(QueryResultTable): add per-row clipboard copy button to query re…#3741
Arunkoo wants to merge 4 commits intoydb-platform:mainfrom
Arunkoo:feature/add-copy-button

Conversation

@Arunkoo
Copy link
Copy Markdown
Contributor

@Arunkoo Arunkoo commented Apr 2, 2026

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
copy button for each row result

Changes

File Change
src/components/QueryResultTable/QueryResultTable.tsx Added rowToTsv helper, copyColumn definition, dynamic name collision check in preparedColumns useMemo
src/components/QueryResultTable/i18n/en.json Added action.copy-row key
src/components/QueryResultTable/i18n/ru.json Added action.copy-row key (machine translated)

Key decisions

  • Reuses buildTsvBlobParts for TSV serialization so escaping behaviour
    is identical to the existing full result copy — no duplicated logic
  • Copy column name is generated dynamically at runtime by checking against
    actual result column names, guaranteeing no collision even if the user
    queries a column named 'copy'
  • ClipboardButton from @gravity-ui/uikit handles copy, success and error
    states internally — no extra state management needed

Greptile Summary

This PR adds a per-row copy-to-clipboard button to the QueryResultTable component, serializing each row as a tab-separated string using the existing buildTsvBlobParts utility for consistent formatting with the full-table export path.

Previous review concerns (variable shadowing on data/rowData, hardcoded English strings, duplicate copyColumn definition, and 'copy' name collisions) have all been addressed in the current revision. Two minor style-level issues remain:

  • The buildTsvBlobParts utility is imported from src/containers/Tenant/Query/utils/, creating a cross-layer dependency from a shared components/ file into a feature-specific containers/ path. Moving the utility to a neutral shared location would remove this coupling.
  • rowToTsv uses .slice(2) as a magic index into buildTsvBlobParts'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

Filename Overview
src/components/QueryResultTable/QueryResultTable.tsx Adds per-row clipboard button using a module-level copyColumn constant with dynamic name-collision resolution. Previously flagged bugs are all fixed; two P2 style concerns remain: a cross-layer import from containers/ and a magic-number index into buildTsvBlobParts.
src/components/QueryResultTable/i18n/en.json Adds action.copy-row i18n key 'Copy row' — correct and consistent with existing key naming convention.
src/components/QueryResultTable/i18n/ru.json Adds action.copy-row i18n key 'Скопировать строку' — mirrors the English key correctly.

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 clipboard
Loading

Comments Outside Diff (2)

  1. src/components/QueryResultTable/QueryResultTable.tsx, line 141-143 (link)

    P1 "Table is empty" message is now unreachable

    preparedColumns always contains at least the copy column, so !preparedColumns.length is permanently false. When a query returns zero rows (generic path with no columns prop), 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.length guard on line 141 fires correctly on empty results.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: src/components/QueryResultTable/QueryResultTable.tsx
    Line: 141-143
    
    Comment:
    **"Table is empty" message is now unreachable**
    
    `preparedColumns` always contains at least the copy column, so `!preparedColumns.length` is permanently `false`. When a query returns zero rows (generic path with no `columns` prop), 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.length` guard on line 141 fires correctly on empty results.
    
    How can I resolve this? If you propose a fix, please make it concise.
  2. src/components/QueryResultTable/QueryResultTable.tsx, line 34 (link)

    Copied text includes column headers, not just cell values

    buildTsvBlobParts([row]).join('') produces two lines:

    col1\tcol2\tcol3
    val1\tval2\tval3
    

    The first line is the header row (column names), derived from Object.keys(data[0]) inside buildTsvBlobParts. 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.:

    const rowToTsv = (row: KeyValueRow) => {
        const values = Object.values(row).map((v) =>
            typeof v === 'object' ? JSON.stringify(v) : `${v}`,
        );
        return values.join('\t');
    };

    (This would also need the same escapeValue logic used by buildTsvBlobParts for correctness.)

Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/components/QueryResultTable/QueryResultTable.tsx
Line: 34

Comment:
**Fragile magic-number index into `buildTsvBlobParts` output**

`buildTsvBlobParts([row]).slice(2)` depends on the exact internal structure of `buildTsvBlobParts`:
- index 0 → header string
- index 1 → `'\n'` divider
- index 2 → row values

If `buildTsvBlobParts` ever gains a BOM, a preamble part, or restructures its output, `rowToTsv` will silently copy the wrong data (e.g., headers instead of values) with no compile-time or runtime warning.

A more resilient approach is to expose a dedicated single-row serializer from `getPreparedResult.ts`, or at minimum document the assumption with a named constant:

```ts
// buildTsvBlobParts returns [headers, ROW_DIVIDER, ...rowParts]
const HEADER_PARTS_COUNT = 2; // skip header line + leading newline
const rowToTsv = (row: KeyValueRow) =>
    buildTsvBlobParts([row]).slice(HEADER_PARTS_COUNT).join('');
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/components/QueryResultTable/QueryResultTable.tsx
Line: 7

Comment:
**Cross-layer import: `components/` depending on `containers/`**

`buildTsvBlobParts` is imported from `src/containers/Tenant/Query/utils/getPreparedResult`, a feature-specific utility path. The `QueryResultTable` component lives in `src/components/` — a shared layer that should not depend on a specific container feature's internals.

If this component is ever reused outside the `Tenant/Query` context, or if the `Tenant/Query` module is restructured, this import will break or force an awkward refactor.

Consider moving `buildTsvBlobParts` (and its helper `escapeValue`) to a more neutral shared location such as `src/utils/tsv.ts` or `src/utils/query.ts`, so it can be imported cleanly from both the component and the container.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (5): Last reviewed commit: "fix(QueryResultTable): exclude header ro..." | Re-trigger Greptile

…olumn name, i18n tooltip, deduplicate copyColumn
@Arunkoo Arunkoo force-pushed the feature/add-copy-button branch from 81e7e61 to 29408c4 Compare April 2, 2026 17:35
@Arunkoo
Copy link
Copy Markdown
Contributor Author

Arunkoo commented Apr 2, 2026

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> = {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

@Arunkoo Arunkoo requested a review from astandrik April 3, 2026 17:14
@astandrik
Copy link
Copy Markdown
Collaborator

@Arunkoo

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.
If row-copy - maybe its better to make floating icon that appears on row hover?

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

@Arunkoo
Copy link
Copy Markdown
Contributor Author

Arunkoo commented Apr 4, 2026

@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,
I'm happy to wait for product/design input before proceeding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

add copy button to query results

2 participants