Repository navigation
fix(SqlLab): improve SQL diff modal — responsive width, padding, tabs, and copy button - #39246
Conversation
The Source SQL / Executed SQL modal in SQL Lab used the default 600px width, making long SQL queries hard to read. Adding the `responsive` prop makes the modal scale up to 900px and adapt to viewport size. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The modal body content had a negative margin (-sizeUnit*6 on all sides) that was pulling content outside the scrollable area (breaking overflow) and stripping padding from the Source/Executed SQL titles. Removing it lets the modal body's built-in padding apply naturally. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ngfully When the only difference between Source SQL and Executed SQL is a trailing LIMIT clause (applied by row limit enforcement), show a single block labeled "Source SQL" — they're effectively the same query. When the queries genuinely differ, show them in two tabs defaulting to "Executed SQL" (what actually ran), with "Source SQL" as the second tab. This avoids stacking two large code blocks vertically and makes the relationship between the two clearer. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add a `showCopyButton` prop (default true) to CodeSyntaxHighlighter that overlays a copy icon in the top-right corner of every code block. Uses navigator.clipboard directly with a brief checkmark feedback state — no external dependencies, fully self-contained in the package. Disable the button on the HighlightedSql trigger node (the inline truncated preview) where it makes no sense. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Guard navigator.clipboard with optional chaining to avoid crashes in environments where the Clipboard API is unavailable (Jest, HTTP iframes) - Remove trailing-LIMIT stripping from the diff comparison: if a row limit was applied, users should see it in the Executed SQL tab Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
dosubot
Bot
added
change:frontend
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
❌ Your project status has failed because the head coverage (99.81%) is below the target coverage (100.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #39246 +/- ##
=======================================
Coverage 64.37% 64.38%
=======================================
Files 2550 2550
Lines 132180 132205 +25
Branches 30661 30672 +11
=======================================
+ Hits 85096 85119 +23
- Misses 45599 45601 +2
Partials 1485 1485
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Agent Run #9f1ec8
Actionable Suggestions - 3
-
superset-frontend/src/SqlLab/components/HighlightedSql/HighlightedSql.test.tsx - 1
- Missing test assertion for Source SQL tab · Line 50-63
-
superset-frontend/packages/superset-ui-core/src/components/CodeSyntaxHighlighter/index.tsx - 2
- Missing clipboard API fallback · Line 109-109
- Missing i18n for button labels · Line 130-130
Review Details
-
Files reviewed - 3 · Commit Range:
dfd6044..1590929- superset-frontend/packages/superset-ui-core/src/components/CodeSyntaxHighlighter/index.tsx
- superset-frontend/src/SqlLab/components/HighlightedSql/HighlightedSql.test.tsx
- superset-frontend/src/SqlLab/components/HighlightedSql/index.tsx
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
…utton, test coverage
- Fix clipboard guard: chain ?.then() so undefined clipboard doesn't throw
- SyntaxHighlighterCopy already has its own copy button (with toasts via
src/utils/copy); pass showCopyButton={false} to suppress the built-in
one and avoid duplicate controls in query preview modals
- Add CodeSyntaxHighlighter tests: copy button shown/hidden, no-throw
when clipboard API is unavailable
- Add HighlightedSql test: rawSql === sql shows single block with no tabs
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Code Review Agent Run #0e7731Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
sadpandajoe
added
the
🎪 ⚡ showtime-trigger-start
github-actions
Bot
added
🎪 b7ca630 🚦 building
and removed
🎪 ⚡ showtime-trigger-start
github-actions
Bot
added
🎪 ⌛ 48h
|
🎪 Showtime deployed environment on GHA for 34f3b7b • Environment: http://54.188.176.204:8080 (admin/admin) |
|
🎪 Showtime deployed environment on GHA for 366fe18 • Environment: http://44.255.92.3:8080 (admin/admin) |
There was a problem hiding this comment.
Code Review Agent Run #11699d
Actionable Suggestions - 1
-
superset-frontend/packages/superset-ui-core/src/utils/copy.ts - 1
- Safari ClipboardItem API Misuse · Line 36-38
Additional Suggestions - 4
-
superset-frontend/packages/superset-ui-core/src/utils/copy.ts - 4
-
Missing Explicit Return Type · Line 26-26Add explicit return type : Promise to copyTextWithClipboardApi for consistency with codebase standards.
Code suggestion
diff --git a/superset-frontend/packages/superset-ui-core/src/utils/copy.ts b/superset-frontend/packages/superset-ui-core/src/utils/copy.ts index 0000000..0000000 100644 --- a/superset-frontend/packages/superset-ui-core/src/utils/copy.ts +++ b/superset-frontend/packages/superset-ui-core/src/utils/copy.ts @@ -26,1 +26,1 @@ -const copyTextWithClipboardApi = async (getText: () => Promise<string>) => { +const copyTextWithClipboardApi = async (getText: () => Promise<string>): Promise<void> => {
-
Broad Exception Catch · Line 40-40Replace bare exception catch with specific catch (error: unknown) to avoid masking unexpected errors, per codebase standards.
Code suggestion
diff --git a/superset-frontend/packages/superset-ui-core/src/utils/copy.ts b/superset-frontend/packages/superset-ui-core/src/utils/copy.ts index 0000000..0000000 100644 --- a/superset-frontend/packages/superset-ui-core/src/utils/copy.ts +++ b/superset-frontend/packages/superset-ui-core/src/utils/copy.ts @@ -40,1 +40,1 @@ - } catch { + } catch (error: unknown) {
-
Missing Explicit Return Type · Line 55-55Add explicit return type : Promise to copyTextToClipboard for consistency with codebase standards.
Code suggestion
diff --git a/superset-frontend/packages/superset-ui-core/src/utils/copy.ts b/superset-frontend/packages/superset-ui-core/src/utils/copy.ts index 0000000..0000000 100644 --- a/superset-frontend/packages/superset-ui-core/src/utils/copy.ts +++ b/superset-frontend/packages/superset-ui-core/src/utils/copy.ts @@ -55,1 +55,1 @@ -const copyTextToClipboard = (getText: () => Promise<string>) => +const copyTextToClipboard = (getText: () => Promise<string>): Promise<void> =>
-
Broad Exception Catch · Line 81-81Replace catch (err) with catch (error: unknown) in execCommand fallback to follow specific exception handling standards.
Code suggestion
diff --git a/superset-frontend/packages/superset-ui-core/src/utils/copy.ts b/superset-frontend/packages/superset-ui-core/src/utils/copy.ts index 0000000..0000000 100644 --- a/superset-frontend/packages/superset-ui-core/src/utils/copy.ts +++ b/superset-frontend/packages/superset-ui-core/src/utils/copy.ts @@ -81,1 +81,1 @@ - } catch (err) { + } catch (error: unknown) {
-
Review Details
-
Files reviewed - 6 · Commit Range:
b7ca630..34f3b7b- superset-frontend/packages/superset-ui-core/src/components/CodeSyntaxHighlighter/index.test.tsx
- superset-frontend/packages/superset-ui-core/src/components/CodeSyntaxHighlighter/index.tsx
- superset-frontend/packages/superset-ui-core/src/utils/copy.ts
- superset-frontend/packages/superset-ui-core/src/utils/index.ts
- superset-frontend/src/utils/copy.ts
- superset-frontend/src/SqlLab/components/HighlightedSql/HighlightedSql.test.tsx
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- Eslint (Linter) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
| const clipboardItem = new ClipboardItem({ | ||
| 'text/plain': getText(), | ||
| }); |
There was a problem hiding this comment.
For Safari, await getText() and pass its result as a Blob to ClipboardItem instead of passing the Promise directly.
Code Review Run #11699d
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
Live binding re-exports (export { default } from '...') create
non-configurable properties that jest.spyOn cannot redefine. Use
import + export default instead so the binding is writable.
Fixes DataTablesPane test: 'Should copy data table content correctly'
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Code Review Agent Run #6ea9e3Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
aminghadersohi
left a comment
aminghadersohi
left a comment
There was a problem hiding this comment.
tested it. works.
SUMMARY
Fixes several UX issues with the Source SQL vs Executed SQL modal in SQL Lab (sc-102563):
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before: Fixed 600px wide, content overflowing vertically, "Source SQL" / "Executed SQL" labels flush against the edge, both blocks always stacked vertically.
After: Responsive width, proper padding, tabs for diff view, copy button on each code block.
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION