(superset) 01/01: fix(sqllab): clip multi-line result values to their row

[jl...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

sadpandajoe pushed a commit to branch ultraset-sqllab-multiline-cell-values-overlap-rows
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 49463d978a387679563abdd21ca9b2f788167471
Author: Joe Li <jo...@preset.io>
AuthorDate: Mon Oct 19 19:51:36 2026 +0000

    fix(sqllab): clip multi-line result values to their row
    
    Since #41135 set `white-space: pre` on `.ag-cell-value`, a value with an
    embedded newline lays out on several lines. The value span grows to fit
    them and neither ag-grid's `.ag-cell` nor `.ag-cell-wrapper` clips
    overflow, so the extra lines painted over the rows below. Clip at
    `.ag-cell`, which has the fixed row height.
    
    Fixes #45464
    
    Co-Authored-By: Claude Opus 5.5 <no...@anthropic.com>
---
 .../src/components/GridTable/GridTable.test.tsx     | 21 +++++++++++++++++++++
 .../src/components/GridTable/index.tsx              | 10 ++++++++--
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/superset-frontend/src/components/GridTable/GridTable.test.tsx b/superset-frontend/src/components/GridTable/GridTable.test.tsx
index 75993d4283b..0b6de658d6b 100644
--- a/superset-frontend/src/components/GridTable/GridTable.test.tsx
+++ b/superset-frontend/src/components/GridTable/GridTable.test.tsx
@@ -20,6 +20,13 @@ import { render } from 'spec/helpers/testing-library';
 import { setupAGGridModules } from '@superset-ui/core/components/ThemedAgGridReact';
 import { GridTable } from '.';
 
+// Jest renders the `css` prop as a plain attribute, so record the styles
+// GridTable builds instead of reading them from the DOM.
+jest.mock('@apache-superset/core/theme', () => {
+  const actual = jest.requireActual('@apache-superset/core/theme');
+  return { ...actual, css: jest.fn(actual.css) };
+});
+
 const mockedProps = {
   queryId: 'abc',
   columns: ['a', 'b', 'c'].map(key => ({
@@ -64,3 +71,17 @@ test('sorts strings correctly', () => {
   // Original order
   expect(container).toHaveTextContent(['Bravo', 'Alpha', 'Charlie'].join(''));
 });
+
+test('clips multi-line cell values at the cell so they do not overlap the rows below', () => {
+  const { css } = jest.requireMock('@apache-superset/core/theme');
+  css.mockClear();
+  render(<GridTable {...mockedProps} />);
+
+  const gridStyles = css.mock.results
+    .map(({ value }: { value: { styles: string } }) => value.styles)
+    .find((styles: string) => styles.includes('.ag-cell-value'));
+
+  // The cell has the fixed row height; the inner value span grows to fit its
+  // lines, so the clip has to be on the cell itself.
+  expect(gridStyles).toMatch(/\.ag-cell\s*\{\s*overflow:\s*hidden;/);
+});
diff --git a/superset-frontend/src/components/GridTable/index.tsx b/superset-frontend/src/components/GridTable/index.tsx
index 04ae59fb015..0ce3e3b57d0 100644
--- a/superset-frontend/src/components/GridTable/index.tsx
+++ b/superset-frontend/src/components/GridTable/index.tsx
@@ -182,10 +182,16 @@ export function GridTable<RecordType extends object>({
         'pre' is a deliberate trade-off over 'pre-wrap': it keeps values on a
         single line so row heights and column sizing stay unchanged. CSS has no
         value that preserves spaces while collapsing newlines, so an embedded
-        newline renders on multiple lines and is clipped by the fixed row
-        height; truncating such values to the visible row is acceptable here.
+        newline renders on multiple lines; truncating such values to the
+        visible row is acceptable here. The value element grows to fit its
+        lines, so the clip is applied on .ag-cell, which has the fixed row
+        height; otherwise the extra lines paint over the rows below.
         overflow/text-overflow keep over-long single-line values clipped with
         an ellipsis rather than overflowing the cell. */
+        .ag-cell {
+          overflow: hidden;
+        }
+
         .ag-cell-value {
           white-space: pre;
           overflow: hidden;