ShauryaaIBM commented on code in PR #43762:
URL: https://github.com/apache/superset/pull/43762#discussion_r3986753684
##########
superset/utils/screenshot_utils.py:
##########
@@ -437,6 +437,1069 @@ def _unready_chart_holders_js_body(*, viewport_only:
bool) -> str:
}}
"""
+# Like REPORT_ALL_CHART_HOLDERS_READY_JS, but returns True immediately when
+# there are no chart holders (markdown-only dashboards). REPORT_*_READY_JS
+# requires holders.length > 0 to distinguish "still loading" from "no charts";
+# the print path must never block on an empty dashboard, so the gate is
omitted.
+# Reuses UNREADY_ALL_CHART_HOLDERS_JS_BODY which already excludes ECharts hosts
+# that have not yet fired their ``finished`` event, preventing blank-canvas
+# captures on dashboards with ECharts vizzes.
+PRINT_ALL_CHART_HOLDERS_READY_JS = (
Review Comment:
Fixed in commit `7ce60553`.
The predicate now distinguishes the pre-mount window from a confirmed
chart-free dashboard:
```javascript
if (holders.length === 0) {
const columns = document.querySelectorAll('.dragdroppable-column');
if (columns.length > 0) {
return false; // pre-mount: grid shell exists, chart components not
yet
}
}
return unready.length === 0;
```
`.dragdroppable-column` is only rendered by the Column and Tab grid
components — never by Markdown, Header, or Divider — so its presence reliably
signals that chart holders are expected but haven't mounted yet.
A truly chart-free (markdown-only) dashboard has no `.dragdroppable-column`
elements and falls through immediately.
Three unit tests in `TestPrintAllChartHoldersReadyJS` cover:
1. The guard is present.
2. The chart-free path falls through correctly.
3. The predicate uses the all-holders body (viewport-independent, matching
the full-page PDF path).
##########
superset-frontend/src/constants.ts:
##########
@@ -155,7 +175,6 @@ export const DEFAULT_COMMON_BOOTSTRAP_DATA:
CommonBootstrapData = {
},
extra_categorical_color_schemes: [],
extra_sequential_color_schemes: [],
- extra_theme_tokens: [],
theme: {
Review Comment:
Fixed in commit 7ce60553.
extra_theme_tokens: [] has been restored to DEFAULT_COMMON_BOOTSTRAP_DATA.
The field was accidentally dropped, making the typed initializer fail TS2322 on
any typecheck that compiled this constant (CommonBootstrapData declares
extra_theme_tokens: string[] as a required field with no default). The one-line
restore is the complete fix.
--
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]
