sadpandajoe opened a new pull request, #44414:
URL: https://github.com/apache/superset/pull/44414

   ### 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 #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 #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
   
   <!-- Before: table card overlapping the neighbouring Pie chart's slot. 
After: same dashboard, table clipped to its own slot. -->
   
   ### 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
   
   - [x] Has associated issue: Fixes #40428
   - [x] Required feature flags: None
   - [ ] Changes UI
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to