You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by jl...@apache.org on 2026/09/18 05:35:32 UTC
(superset) branch ultraset-fix-40428 updated (c003968bbc6 -> 9715057f3a1)
This is an automated email from the ASF dual-hosted git repository.
sadpandajoe pushed a change to branch ultraset-fix-40428
in repository https://gitbox.apache.org/repos/asf/superset.git
discard c003968bbc6 fix(dashboard): stop table charts bleeding over neighbours in image export
new 9715057f3a1 fix(dashboard): stop table charts bleeding over neighbours in image export
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (c003968bbc6)
\
N -- N -- N refs/heads/ultraset-fix-40428 (9715057f3a1)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../src/utils/downloadAsImage.test.ts | 25 +++++++++++++++++++---
superset-frontend/src/utils/downloadAsImage.tsx | 20 +++++++++++------
2 files changed, 35 insertions(+), 10 deletions(-)
(superset) 01/01: fix(dashboard): stop table charts bleeding over neighbours in image export
Posted by jl...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sadpandajoe pushed a commit to branch ultraset-fix-40428
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 9715057f3a14371619e7f8629e5d06aa356cd068
Author: sadpandajoe <47...@users.noreply.github.com>
AuthorDate: Fri Sep 18 05:10:39 2026 +0000
fix(dashboard): stop table charts bleeding over neighbours in image export
The clone-based image export copies every computed style inline, including
`overflow`, so `processCloneForVisibility`'s `[style*="overflow"]` selector
matches effectively every node in the clone rather than just scroll
containers. Each match had its `overflow` shorthand set to `visible`, which
unclipped both axes.
Unclipping the vertical axis is what makes the whole table paint (#34233):
heights become `auto`, the chart holder grows and the dashboard grid reflows
around it. The horizontal axis is different - the clone keeps every element's
on-screen pixel width - so a table that scrolls sideways painted its off-slot
columns straight across the charts sitting next to it in the same dashboard
row.
Keep the horizontal clip for dashboard captures, and only rewrite `overflow`
on elements that actually clip, so elements that never clipped on screen are
not newly clipped. Single-chart captures have no neighbours to overlap and
keep expanding on both axes.
Fixes #40428
Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
---
.../src/utils/downloadAsImage.test.ts | 79 +++++++++++++++++++++-
superset-frontend/src/utils/downloadAsImage.tsx | 40 ++++++++++-
2 files changed, 114 insertions(+), 5 deletions(-)
diff --git a/superset-frontend/src/utils/downloadAsImage.test.ts b/superset-frontend/src/utils/downloadAsImage.test.ts
index 979a867a895..d4e3593b3e2 100644
--- a/superset-frontend/src/utils/downloadAsImage.test.ts
+++ b/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 () => {
diff --git a/superset-frontend/src/utils/downloadAsImage.tsx b/superset-frontend/src/utils/downloadAsImage.tsx
index 61c7d34c512..d0cb07fb61d 100644
--- a/superset-frontend/src/utils/downloadAsImage.tsx
+++ b/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;