codeant-ai-for-open-source[bot] commented on code in PR #43979:
URL: https://github.com/apache/superset/pull/43979#discussion_r3954585537
##########
superset/utils/screenshot_utils.py:
##########
@@ -437,6 +437,67 @@ def _unready_chart_holders_js_body(*, viewport_only: bool)
-> str:
}}
"""
+# Content that is fully present in the DOM but visually clipped by a fixed
+# height + internal scrollbar -- e.g. a table taller than the space its
+# dashboard tile gives it. These mirror the selectors the client-side
+# "download as image" export unrolls before rasterizing
+# (superset-frontend/src/utils/downloadAsImage.tsx) so both capture paths
+# treat scrollable widgets the same way (#38090).
+SCROLLABLE_CONTENT_SELECTORS = [
+ ".ant-table-body",
+ ".table-container",
+ ".ant-table-container",
+ ".table-wrapper",
+ ".virtual-table",
+]
+
+# ag-Grid virtualizes rows for performance, so a plain height/overflow reset
+# would still leave off-screen rows unrendered. `domLayout: "print"` is
+# ag-Grid's own "render every row into the DOM" mode -- the same mode the
+# client-side image export switches to via the GridApi that
+# ThemedAgGridReact (superset-ui-core) stashes on the grid's host element
+# specifically so screenshot/export code can reach it.
+#
+# `page.screenshot(full_page=True)` already expands the outer dashboard
+# scroll to include every below-the-fold chart (#31158); it has no effect on
+# a chart's own internal scroll container, which is what this JS unrolls
+# in-place before the page is captured.
+EXPAND_SCROLLABLE_CONTENT_JS = f"""
+async () => {{
+ const agGrids = Array.from(
+ document.querySelectorAll('{AG_GRID_HOST_SELECTOR}')
+ );
+ await Promise.all(agGrids.map(async (grid) => {{
+ const api = grid._agGridApi;
+ if (!api) {{ return; }}
+ api.setGridOption('domLayout', 'print');
+ if (api.resetRowHeights) {{ api.resetRowHeights(); }}
+ let lastHeight = grid.scrollHeight;
+ let stableCount = 0;
+ const deadline = Date.now() + 5000;
+ while (Date.now() < deadline && stableCount < 5) {{
+ await new Promise((resolve) => setTimeout(resolve, 100));
Review Comment:
**Suggestion:** The stabilization loop can block for five seconds without
checking the report deadline, consuming time reserved for readiness, capture,
and cleanup. [performance]
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=0b3e6f97d3f843c8ac1cedfcdb4bbafd&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=0b3e6f97d3f843c8ac1cedfcdb4bbafd&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/utils/screenshot_utils.py
**Line:** 477:479
**Comment:**
*Performance: The stabilization loop can block for five seconds without
checking the report deadline, consuming time reserved for readiness, capture,
and cleanup.
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
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43979&comment_hash=feb2f23c62156347ec552c50a1912e54b1d403b7457918047c966f0c4ed3f9f3&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43979&comment_hash=feb2f23c62156347ec552c50a1912e54b1d403b7457918047c966f0c4ed3f9f3&reaction=dislike'>๐</a>
##########
superset/utils/screenshot_utils.py:
##########
@@ -437,6 +437,67 @@ def _unready_chart_holders_js_body(*, viewport_only: bool)
-> str:
}}
"""
+# Content that is fully present in the DOM but visually clipped by a fixed
+# height + internal scrollbar -- e.g. a table taller than the space its
+# dashboard tile gives it. These mirror the selectors the client-side
+# "download as image" export unrolls before rasterizing
+# (superset-frontend/src/utils/downloadAsImage.tsx) so both capture paths
+# treat scrollable widgets the same way (#38090).
+SCROLLABLE_CONTENT_SELECTORS = [
+ ".ant-table-body",
+ ".table-container",
+ ".ant-table-container",
+ ".table-wrapper",
+ ".virtual-table",
+]
+
+# ag-Grid virtualizes rows for performance, so a plain height/overflow reset
+# would still leave off-screen rows unrendered. `domLayout: "print"` is
+# ag-Grid's own "render every row into the DOM" mode -- the same mode the
+# client-side image export switches to via the GridApi that
+# ThemedAgGridReact (superset-ui-core) stashes on the grid's host element
+# specifically so screenshot/export code can reach it.
+#
+# `page.screenshot(full_page=True)` already expands the outer dashboard
+# scroll to include every below-the-fold chart (#31158); it has no effect on
+# a chart's own internal scroll container, which is what this JS unrolls
+# in-place before the page is captured.
+EXPAND_SCROLLABLE_CONTENT_JS = f"""
+async () => {{
+ const agGrids = Array.from(
+ document.querySelectorAll('{AG_GRID_HOST_SELECTOR}')
+ );
+ await Promise.all(agGrids.map(async (grid) => {{
+ const api = grid._agGridApi;
+ if (!api) {{ return; }}
+ api.setGridOption('domLayout', 'print');
+ if (api.resetRowHeights) {{ api.resetRowHeights(); }}
+ let lastHeight = grid.scrollHeight;
+ let stableCount = 0;
+ const deadline = Date.now() + 5000;
+ while (Date.now() < deadline && stableCount < 5) {{
+ await new Promise((resolve) => setTimeout(resolve, 100));
+ const height = grid.scrollHeight;
+ if (height === lastHeight) {{
+ stableCount += 1;
+ }} else {{
+ stableCount = 0;
+ lastHeight = height;
+ }}
+ }}
+ }}));
+
+ const scrollableSelectors = {SCROLLABLE_CONTENT_SELECTORS!r};
+ scrollableSelectors.forEach((selector) => {{
+ document.querySelectorAll(selector).forEach((el) => {{
+ el.style.overflow = 'visible';
+ el.style.height = 'auto';
+ el.style.maxHeight = 'none';
+ }});
Review Comment:
**Suggestion:** Changing descendant heights does not enlarge the fixed chart
element's bounding box, so `element.screenshot()` can still crop the expanded
table in single-chart exports. [api mismatch]
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f434f7436766477798caf5fe25942920&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=f434f7436766477798caf5fe25942920&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/utils/screenshot_utils.py
**Line:** 493:496
**Comment:**
*Api Mismatch: Changing descendant heights does not enlarge the fixed
chart element's bounding box, so `element.screenshot()` can still crop the
expanded table in single-chart exports.
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
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43979&comment_hash=b8a13d240e3ab2dc7766d4d3dfc5129698b63b426757dc9be0ba1c06a8b738ec&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43979&comment_hash=b8a13d240e3ab2dc7766d4d3dfc5129698b63b426757dc9be0ba1c06a8b738ec&reaction=dislike'>๐</a>
--
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]
