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

   ### SUMMARY
   Follow-up to #38090 / #43979. After that PR merged, testing against the 
**legacy (non-ag-Grid) `plugin-chart-table`** table type showed rows were still 
clipped in dashboard/chart PDF and PNG exports.
   
   Root cause: `plugin-chart-table`'s sticky table 
(`DataTable/hooks/useSticky.tsx`) wraps its scrollable body in an **outer 
`role="table"` div that has its own fixed pixel height + `overflow: hidden`**, 
around an **inner scroll body div** that also has a fixed height + `overflow: 
auto`. `EXPAND_SCROLLABLE_CONTENT_JS`'s generic-descendant reset (`#43979`) is 
gated on `el.scrollHeight > el.clientHeight`, and does a single 
`querySelectorAll` pass. Before any mutation, the *outer* wrapper's height was 
computed as the sum of its *already-clipped* children, so its `scrollHeight` 
already equals its `clientHeight` — the gate skips it. Only *after* the inner 
scroll body gets reset (later in that same pass) does the outer wrapper's 
content actually overflow it. One pass is one pass too few.
   
   ### FIX
   Repeat the generic-descendant reset to a fixed point (bounded to 5 passes) 
instead of a single `forEach`, so a wrapper that only becomes detectably 
clipped after its child expands still gets caught on a later pass.
   
   ### VERIFICATION
   No app/browser environment available here, so I verified this directly 
against a **real headless Chromium via Playwright**, reproducing the exact 
nested shape from `useSticky.tsx` (outer fixed-height `overflow:hidden` wrapper 
around an inner fixed-height `overflow:auto` scroll body containing 50 rows):
   
   - Against the pre-fix single-pass code: the fixture's last row stays outside 
the `.chart-container` bounding box that a locator-scoped 
`element.screenshot()` would capture — reproducing the report.
   - Against the fixed-point loop: `.chart-container` grows from 300px to 
1638px and every row is inside the captured bounding box.
   
   Before/after screenshots (not embedded here, happy to attach on request):
   
   | | height captured | last row visible |
   |---|---|---|
   | before | 300px | ❌ |
   | after | 1638px | ✅ (all 50 rows) |
   
   This browser-driven test is now 
`test_expand_scrollable_content_resolves_nested_ancestor_clip` in 
`test_screenshot_utils.py`, skipping cleanly (`pytest.importorskip`) when 
Playwright/Chromium aren't available — **note this repo's own `Python-Unit` CI 
job does not currently install either**, so today this is a strong local/dev 
regression aid rather than an enforced CI gate. Also added a plain 
string-content assertion for the loop construct, consistent with the rest of 
this file's existing tests.
   
   `pytest tests/unit_tests/utils/test_screenshot_utils.py 
tests/unit_tests/utils/webdriver_test.py` → 130 passed. `pre-commit` clean.
   
   ### SCOPE
   This addresses one of three distinct table/pivot export-completeness gaps 
identified while investigating the original report:
   1. **Row-height clipping for the classic (non-ag-Grid) table** — this PR.
   2. Column-width clipping (wide tables) — not addressed here, out of scope 
for a CSS-level fix.
   3. Row-dropping under `server_pagination` (rows never fetched at all, not 
just visually clipped) — a different, larger-scope problem (would need a 
bounded re-fetch, not a DOM reset).
   
   ### ADDITIONAL INFORMATION
   - [x] Has associated issue: follow-up to #38090, #43979
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration
   - [ ] 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