feat: pretty-print JSON and add copy button in cell popup#3801
Open
Arunkoo wants to merge 4 commits intoydb-platform:mainfrom
Open
feat: pretty-print JSON and add copy button in cell popup#3801Arunkoo wants to merge 4 commits intoydb-platform:mainfrom
Arunkoo wants to merge 4 commits intoydb-platform:mainfrom
Conversation
| <div className={b('cell-popup')}>{value}</div> | ||
| <div className={b('cell-popup')}> | ||
| {isJson ? <pre className={b('cell-popup-json')}>{formatted}</pre> : formatted} | ||
| <ClipboardButton text={formatted} size="s" className={b('cell-popup-copy')} /> |
Contributor
There was a problem hiding this comment.
Copy button copies pretty-printed form, not original value
The PR description says "copy the cell value," but text={formatted} copies the indented multi-line JSON (with extra whitespace) rather than the original compact string from the query result. Users pasting into code or a query editor will get the reformatted version instead of the raw value. Use text={value} to copy the original.
Suggested change
| <ClipboardButton text={formatted} size="s" className={b('cell-popup-copy')} /> | |
| <ClipboardButton text={value} size="s" className={b('cell-popup-copy')} /> |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/QueryResultTable/Cell/Cell.tsx
Line: 48
Comment:
**Copy button copies pretty-printed form, not original value**
The PR description says "copy the cell value," but `text={formatted}` copies the indented multi-line JSON (with extra whitespace) rather than the original compact string from the query result. Users pasting into code or a query editor will get the reformatted version instead of the raw value. Use `text={value}` to copy the original.
```suggestion
<ClipboardButton text={value} size="s" className={b('cell-popup-copy')} />
```
How can I resolve this? If you propose a fix, please make it concise.
Contributor
Author
|
hi @astandrik , @Raubzeug Gentle ping — happy to address any feedback when you get a chance. |
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
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.
Problem
When a query result cell contains a JSON value, it is displayed as a minified string in both the cell and the popup tooltip, making it hard to read — especially for large JSON payloads.
Closes #3594
Changes
Cell.tsx(primitives like42,true,nullare intentionally excluded)JSON.stringify(parsed, null, 2)inside a<pre>block in the popupClipboardButtonin the popup to copy the formatted JSON valueQueryResultTable.scssBefore
Popup shows:

After
Popup shows:

Testing
Greptile Summary
This PR adds JSON pretty-printing and a copy button to the cell popup in
QueryResultTable. When a cell value is a valid JSON object or array, the popup renders it formatted in a<pre>block; primitives (null, numbers, booleans) are correctly excluded. The primitive guard anduseMemomemoization are well-implemented, and the previous concern about primitive false-positives is resolved.Confidence Score: 5/5
Safe to merge — all remaining findings are minor style suggestions with no impact on correctness or runtime behavior.
The core logic is sound: JSON parsing is safe, primitive values are correctly excluded, and memoization is applied. The two open findings (copy button visibility scope and display: flex on the button) are P2 style concerns that do not affect functionality.
No files require special attention.
Important Files Changed
tryFormatJson), pretty-print rendering in a<pre>block, and aClipboardButton; primitive guard and memoization are correct; copy button is rendered unconditionally for all cell types.display: flexon the copy button may stretch it to full popup width.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Cell receives string value] --> B[tryFormatJson called via useMemo] B --> C{JSON.parse succeeds?} C -- No --> D[formatted = value\nisJson = false] C -- Yes --> E{typeof parsed === 'object'\nAND parsed !== null?} E -- No primitive --> D E -- Yes object or array --> F[formatted = JSON.stringify parsed null 2\nisJson = true] D --> G[Popup renders plain text\n+ ClipboardButton copies value] F --> H[Popup renders pre block\nwith pretty JSON\n+ ClipboardButton copies formatted]Prompt To Fix All With AI
Reviews (3): Last reviewed commit: "Merge remote-tracking branch 'upstream/m..." | Re-trigger Greptile