fix(dashboard): stop wide table charts bleeding over neighbours in dashboard image export

superset-frontend/src/utils/downloadAsImage.test.ts

@@ -644,9 +644,41 @@ test('falls through to clone path for dashboard export with multiple ag-grid cha
   document.body.removeChild(dashboard);
 });
 
-test('captures JPEG for non-ag-grid elements via the clone path', async () => {
+// A chart holding a table that scrolls in both axes, in a slot that never clips
+function buildScrollableChartElement(rootClassName?: string) {
   const container = document.createElement('div');
+  if (rootClassName) {
+    container.className = rootClassName;
+  }
+  const slot = document.createElement('div');
+  slot.className = 'chart-slot';
+  slot.style.overflow = 'visible';
+  slot.style.width = '100px';
+  const scrollContainer = document.createElement('div');
+  scrollContainer.className = 'scroll-container';
+  scrollContainer.style.overflow = 'auto';
+  scrollContainer.style.width = '100px';
+  scrollContainer.style.height = '50px';
+  const wideContent = document.createElement('div');
+  wideContent.style.width = '400px';
+  wideContent.style.height = '300px';
+  wideContent.textContent = 'wide table content';
+  scrollContainer.appendChild(wideContent);
+  // A sibling that clips via `overflow: clip` rather than a scrolling value
+  const clipContainer = document.createElement('div');
+  clipContainer.className = 'clip-container';
+  clipContainer.style.overflow = 'clip';
+  clipContainer.style.width = '100px';
+  clipContainer.style.height = '50px';
+  slot.appendChild(scrollContainer);
+  slot.appendChild(clipContainer);
+  container.appendChild(slot);
   document.body.appendChild(container);
+  return { container, cleanup: () => document.body.removeChild(container) };
+}
+
+test('captures JPEG for non-ag-grid elements via the clone path', async () => {
+  const { container, cleanup } = buildScrollableChartElement();
 
   const handler = downloadAsImageOptimized('div', 'Bar Chart');
   await handler(syntheticEventFor(container));
@@ -655,9 +687,52 @@ test('captures JPEG for non-ag-grid elements via the clone path', async () => {
     expect.any(HTMLElement),
     expect.objectContaining({ quality: 0.95 }),
   );
+  // A single chart has no neighbours to overlap, so its scrollable content is
+  // expanded on both axes and the whole table makes it into the image
+  const clone = mockToJpeg.mock.calls[0][0] as HTMLElement;
+  const clonedScrollContainer = clone.querySelector(
+    '.scroll-container',
+  ) as HTMLElement;
+  expect(clonedScrollContainer.style.height).toBe('auto');
+  expect(clonedScrollContainer.style.overflow).toBe('visible');
+  expect(
+    (clone.querySelector('.clip-container') as HTMLElement).style.overflow,
+  ).toBe('visible');
   expect(mockAddWarningToast).not.toHaveBeenCalled();
 
-  document.body.removeChild(container);
+  cleanup();
+});
+
+test('dashboard clone path expands scrollable content vertically without unclipping it horizontally', async () => {
+  const { container, cleanup } = buildScrollableChartElement('dashboard');
+
+  const handler = downloadAsImageOptimized('div', 'My Dashboard');
+  await handler(syntheticEventFor(container));
+
+  const clone = mockToJpeg.mock.calls[0][0] as HTMLElement;
+  const clonedScrollContainer = clone.querySelector(
+    '.scroll-container',
+  ) as HTMLElement;
+  // Vertical: grows so every row is captured and the grid reflows around it
+  expect(clonedScrollContainer.style.height).toBe('auto');
+  expect(clonedScrollContainer.style.overflowY).toBe('visible');
+  // Horizontal: stays clipped, otherwise a sideways-scrolling table paints its
+  // off-slot columns over the charts next to it in the same dashboard row.
+  // `clip`, not `hidden`, so the `visible` above is not promoted to `auto`
+  expect(clonedScrollContainer.style.overflowX).toBe('clip');
+  // The slot never clipped on screen, so the export must not start clipping it
+  const clonedSlot = clone.querySelector('.chart-slot') as HTMLElement;
+  expect(clonedSlot.style.overflowX).toBe('');
+  expect(clonedSlot.style.overflow).toBe('visible');
+  // `overflow: clip` clips just as much as `auto`/`hidden` do, so it is treated
+  // the same way rather than left alone
+  const clonedClipContainer = clone.querySelector(
+    '.clip-container',
+  ) as HTMLElement;
+  expect(clonedClipContainer.style.overflowX).toBe('clip');
+  expect(clonedClipContainer.style.overflowY).toBe('visible');
+
+  cleanup();
 });
 
 test('shows warning toast when clone capture throws', async () => {

superset-frontend/src/utils/downloadAsImage.tsx

@@ -159,7 +159,23 @@ const copyAllComputedStyles = (
   }
 };
 
-const processCloneForVisibility = (clone: HTMLElement) => {
+// True when the element clips its content on at least one axis. `copyAllComputedStyles`
+// writes the computed `overflow` inline on the whole tree, so `[style*="overflow"]` below
+// matches nearly every node; only the ones that actually clip should have their overflow
+// rewritten. Both the shorthand and the longhands are read because engines disagree on
+// which of them a computed style resolves: browsers resolve the longhands, jsdom only
+// echoes back whichever form was specified.
+const clipsOverflow = (element: HTMLElement) => {
+  const computed = window.getComputedStyle(element);
+  return /auto|scroll|hidden|clip/.test(
+    `${computed.overflow} ${computed.overflowX} ${computed.overflowY}`,
+  );
+};
+
+const processCloneForVisibility = (
+  clone: HTMLElement,
+  clipHorizontalOverflow = false,
+) => {
   const cloneStyle = clone.style;
   cloneStyle.height = 'auto';
   cloneStyle.maxHeight = 'none';
@@ -183,9 +199,25 @@ const processCloneForVisibility = (clone: HTMLElement) => {
   scrollableSelectors.forEach(selector => {
     clone.querySelectorAll(selector).forEach(el => {
       const element = el as HTMLElement;
-      element.style.overflow = 'visible';
       element.style.height = 'auto';
       element.style.maxHeight = 'none';
+      if (!clipsOverflow(element)) return;
+      if (clipHorizontalOverflow) {
+        // The vertical axis stays unclipped so the whole chart paints while the
+        // grid reflows around its now auto-height slot. The horizontal axis keeps
+        // its clip: the clone preserves every element's on-screen pixel width, so
+        // a table that scrolls sideways would otherwise paint its off-slot columns
+        // across the charts sitting next to it in the same dashboard row.
+        // `clip` rather than `hidden`, because `hidden` paired with `visible` is
+        // not a legal computed combination: the engine promotes the `visible` axis
+        // to `auto` and clips that one too. `clip` paired with `visible` clips a
+        // single axis and leaves the element a non-scroll-container, so nothing
+        // but the sideways bleed changes.
+        element.style.overflowX = 'clip';
+        element.style.overflowY = 'visible';
+      } else {
+        element.style.overflow = 'visible';
+      }
     });
   });
 
@@ -302,6 +334,7 @@ const createEnhancedClone = (
   originalElement: Element,
   theme?: SupersetTheme,
   getInstanceByDom?: EChartsGetInstanceByDom,
+  clipHorizontalOverflow = false,
 ): { clone: HTMLElement; cleanup: () => void } => {
   const clone = originalElement.cloneNode(true) as HTMLElement;
   copyAllComputedStyles(originalElement, clone, theme);
@@ -319,7 +352,7 @@ const createEnhancedClone = (
   tempContainer.appendChild(clone);
   document.body.appendChild(tempContainer);
 
-  processCloneForVisibility(clone);
+  processCloneForVisibility(clone, clipHorizontalOverflow);
 
   const cleanup = () => {
     if (tempContainer.parentElement) {
@@ -583,6 +616,7 @@ export default function downloadAsImageOptimized(
         elementToPrint,
         theme,
         getInstanceByDom,
+        isDashboardCapture,
       );
       cleanup = cleanupFn;