[sadpandajoe]

SUMMARY

Downloading a dashboard as an image renders a DOM clone through dom-to-image-more. copyAllComputedStyles inlines the computed overflow onto essentially every node in that clone, so processCloneForVisibility's [style*="overflow"] selector matched nearly the whole tree (232/232 nodes in a measured fixture) and set overflow: visible on both axes everywhere — not just on real scroll containers.

Unclipping vertically is deliberate (#34233): it lets a chart holder grow so the full table paints instead of only the rows in the viewport. Unclipping horizontally is the bug in #40428: a table that scrolls sideways on screen paints its full unclipped width into the export and runs across the charts sitting next to it in the same dashboard row.

This change:

• adds a clipsOverflow() guard so only elements that genuinely clip on screen get their overflow rewritten at all. It reads both the overflow shorthand and the overflow-x/overflow-y longhands, because browsers resolve the longhands while jsdom only echoes back whichever form was specified.
• for dashboard captures, rewrites those elements to overflow-x: clip; overflow-y: visible instead of a blanket overflow: visible. clip rather than hidden matters: per CSS Overflow 3, hidden paired with visible is not a legal computed combination — the engine would promote the visible axis to auto and clip that one too, defeating fix(chart-download): ensure full table or handlebar chart is captured in image export #34233. clip/visible clips exactly one axis and leaves the element a non-scroll-container.
• leaves the single-chart export path on the previous overflow: visible behaviour (it has no neighbours to overlap), and leaves height: auto / max-height: none applied unconditionally on both paths, so the fix(chart-download): ensure full table or handlebar chart is captured in image export #34233 vertical reflow is untouched.

PDF export is unaffected — it doesn't clone. The ag-grid single-chart print-layout branch is untouched.

Note the intended trade-off: a dashboard image now shows a sideways-scrolling table clipped to its slot, matching what's on screen, rather than spilling its extra columns over the chart beside it.

BEFORE/AFTER SCREENSHOTS

TESTING INSTRUCTIONS

1. Build a dashboard with a table chart wide enough to scroll horizontally, placed next to another chart in the same row.
2. Download the dashboard as an image.
3. The table is clipped to its own slot; the neighbouring chart renders unobstructed. Vertically the table still shows all its rows, not just the visible ones.
4. Download the table chart on its own — unchanged, still expands on both axes.

Unit tests: cd superset-frontend && NODE_ENV=test npx jest src/utils/downloadAsImage.test.ts (34/34).

ADDITIONAL INFORMATION

• Has associated issue: Fixes Other charts are overlapping with Table chart when downloading dashboard as image using "Download as Image" option #40428
• Required feature flags: None
• Changes UI
• Introduces new feature or API
• Removes existing feature or API

[codeant-ai-for-open-source]

Suggestion: Dashboard exports make every matching clipped element vertically visible, including chart header titles that rely on hidden overflow for ellipsis, allowing labels to bleed outside their chart.

Assessment: 🟠 Major · 🔁 Occurrence: Often · 🏷️ Logic error

Prompt for AI Agent 🤖

This is a comment left during a code review.

**Path:** superset-frontend/src/utils/downloadAsImage.tsx
**Line:** 217:217
**Comment:**
	*Logic Error: Dashboard exports make every matching clipped element vertically visible, including chart header titles that rely on hidden overflow for ellipsis, allowing labels to bleed outside their chart.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

👍 | 👎

[bito-code-review]

The flagged issue is correct. The current implementation of processCloneForVisibility in superset-frontend/src/utils/downloadAsImage.tsx unconditionally sets overflow to visible for all scrollable elements, which causes elements relying on hidden overflow (like ellipsis in chart headers) to bleed outside their containers during dashboard exports.

The fix implemented in the PR (which you can see in the diff) introduces a clipsOverflow check and conditionally applies overflowX: 'clip' and overflowY: 'visible' when clipHorizontalOverflow is true (used for dashboard captures). This ensures vertical content is fully captured while maintaining horizontal clipping to prevent layout bleeding.

Would you like me to fetch all other comments on this PR to validate and implement fixes for them as well?

superset-frontend/src/utils/downloadAsImage.tsx

const clipsOverflow = (element: HTMLElement) => {
  const computed = window.getComputedStyle(element);
  return /auto|hidden|clip/.test(
    `${computed.overflow} ${computed.overflowX} ${computed.overflowY}`,
  );
};

[bito-code-review]

Code Review Agent Run #2e3788

Actionable Suggestions - 1

• superset-frontend/src/utils/downloadAsImage.tsx - 1

  • getComputedStyle in loop · Line 168-173

Additional Suggestions - 1

• superset-frontend/src/utils/downloadAsImage.tsx - 1

  • Horizontal clip over-applied · Line 204-217
    `clipsOverflow` returns true when ANY axis clips, but the `clipHorizontalOverflow` branch then unconditionally sets `overflowX='clip'`. An element that clips only vertically (`overflow-y:auto; overflow-x:visible`) gets its horizontal axis newly clipped, hiding content that was visible on screen — contradicting the comment 'The horizontal axis keeps its clip'. Test the horizontal axis before forcing `clip`.

Review Details

• Files reviewed - 2 · Commit Range: 9715057..9715057

  • superset-frontend/src/utils/downloadAsImage.test.ts
  • superset-frontend/src/utils/downloadAsImage.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 an incremental AI Review.

• /review full - 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

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[bito-code-review]

getComputedStyle in loop

clipsOverflow calls window.getComputedStyle(element) for every node matching the scrollable selectors — including [style*="overflow"], which the comment notes matches nearly every node. Each call forces a style/layout recalc, adding O(n) forced layouts on the dashboard export path where the previous code set overflow directly. Reuse the computed styles already cached by copyAllComputedStyles.

Code Review Run #2e3788

---

Should Bito avoid suggestions like this for future reviews? (Manage Rules)

• Yes, avoid them

[codecov]

Codecov Report

❌ Patch coverage is 85.71429% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.56%. Comparing base (394bca5) to head (9715057).

Files with missing lines	Patch %	Lines
superset-frontend/src/utils/downloadAsImage.tsx	85.71%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #44414   +/-   ##
=======================================
  Coverage   80.56%   80.56%           
=======================================
  Files        2938     2938           
  Lines      175246   175257   +11     
  Branches    40680    40685    +5     
=======================================
+ Hits       141184   141197   +13     
+ Misses      31396    31394    -2     
  Partials     2666     2666           

Flag	Coverage Δ
javascript	76.04% <85.71%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[gabotorresruiz]

LGTM.

I checked the branch out and ran the real downloadAsImageOptimized in headless Chromium at both 394bca5 and 9715057, against a fixture built from the sticky header/body structure in plugins/plugin-chart-table/src/DataTable/hooks/useSticky.tsx: a 12 column table in a 484px slot, next to another chart in the same dashboard row.

• On the parent commit the clone comes out 1598px wide on a 1000px dashboard and the table paints all 12 columns straight across the neighbour's slot, which reproduces the shape reported in #40428.
• On this branch the clone is 1000px and the table stops at its own slot, while clone.scrollHeight is identical on both (756px at 40 rows, 7236px at 400 rows), so #34233 is untouched.
• The live DOM is untouched either way: the serialized inline styles of the whole .dashboard subtree are identical before and after the capture, and no temp container is left behind.
• src/utils/downloadAsImage.test.ts is 34/34 here, and running the same file against the parent's downloadAsImage.tsx gives 1 failed / 33 passed, so the new test really does pin the fix.

The clip versus hidden reasoning in the comment holds up in the engine, I probed it directly: overflow-x: clip; overflow-y: visible stays clip / visible and clips exactly one axis, while overflow-x: hidden; overflow-y: visible computes to hidden / auto and clips vertically too, which would have undone #34233. Good that it is written down.

One thing worth saying out loud, since this is a behaviour change and not only a rendering fix: a dashboard image now contains only the columns that fit the slot. Before, the off slot columns were painted, partly buried under whatever chart sat beside them. Downloading the table on its own still expands both axes so the full table is still reachable, but this is a real change for anyone who used the dashboard image to read a wide table. Your description calls it out, I just want it visible to whoever backports this.

The two bot comments above look wrong to me

Flagging so they do not cause churn:

• "horizontal clip over applied": in Chromium overflow-x: visible paired with auto, hidden or scroll on the other axis computes to overflow-x: auto, so an element that clips only vertically while still showing horizontally can only exist as a literal overflow-y: clip with overflow-x: visible, and there is none in src, packages, plugins, antd or ag-grid-community.
• "header labels bleed": clipsOverflow narrows the rewrite to elements that clip on screen, 3 of 5250 nodes in my fixture where [style*="overflow"] matched all 5250, and overflow-x: clip restores the ellipsis clipping that overflow: visible used to remove. This branch makes those labels more faithful, not less.

The server side screenshot still unclips both axes

Not a blocker and not this PR's job, but EXPAND_SCROLLABLE_CONTENT_JS in superset/utils/screenshot_utils.py (line 709) still does el.style.overflow = 'visible' whenever scrollHeight > clientHeight, so a table that scrolls on both axes loses its horizontal clip on the reports, alerts, thumbnails and Excel export paths in the same way. Worth a follow up issue?

Last one: the BEFORE/AFTER SCREENSHOTS section is empty. For a fix whose whole payload is visual, a pair of images would help a lot.

[rusackas]

LGTM, thanks for the rigorous before/after verification here.