gabotorresruiz commented on code in PR #44313:
URL: https://github.com/apache/superset/pull/44313#discussion_r4029926320
##########
superset-frontend/src/embedded/api.tsx:
##########
@@ -46,10 +46,55 @@ type EmbeddedSupersetApi = {
setDataMask: ({ dataMask }: { dataMask: DataMaskStateWithId }) => void;
};
-const getScrollSize = (): Size => ({
- width: document.body.scrollWidth,
- height: document.body.scrollHeight,
-});
+// Hosts size the iframe from this value, so it has to describe the content and
+// not the frame the host already set. Lift the fill-the-frame constraints,
read,
+// restore, all in one synchronous task. The styles are set inline so they win
+// the cascade outright: a rule that silently loses it turns this back into a
+// measurement of the frame.
+const getScrollSize = (): Size => {
+ const root = document.documentElement;
+ const { body } = document;
+ const app = document.getElementById('app');
+ const content = document.querySelector('#app .dashboard');
+
+ // Nothing has laid out yet, so report the viewport. A host applying a near
+ // zero height would collapse the frame, and charts render only once they are
+ // in view, so the embed could not recover.
+ if (!content || content.getBoundingClientRect().height === 0) {
Review Comment:
Good catch — fixed in 1cae80c1ac. The guard now gates on the store having
hydrated (`dashboardInfo.id`) plus `.dashboard-grid` being in the DOM, rather
than on the container having any height. The grid’s rows take their heights
from the layout, not from chart data, so once it is mounted the measurement is
complete even while charts are still loading, and the partial-layout window you
describe no longer passes the guard. Two unit tests cover it, both using a
120px chrome-only height that the old check would have let through: one for
“not hydrated”, one for “hydrated but grid not mounted”.
##########
superset-frontend/src/embedded/api.tsx:
##########
@@ -46,10 +46,55 @@ type EmbeddedSupersetApi = {
setDataMask: ({ dataMask }: { dataMask: DataMaskStateWithId }) => void;
};
-const getScrollSize = (): Size => ({
- width: document.body.scrollWidth,
- height: document.body.scrollHeight,
-});
+// Hosts size the iframe from this value, so it has to describe the content and
+// not the frame the host already set. Lift the fill-the-frame constraints,
read,
+// restore, all in one synchronous task. The styles are set inline so they win
+// the cascade outright: a rule that silently loses it turns this back into a
+// measurement of the frame.
+const getScrollSize = (): Size => {
+ const root = document.documentElement;
+ const { body } = document;
+ const app = document.getElementById('app');
+ const content = document.querySelector('#app .dashboard');
+
+ // Nothing has laid out yet, so report the viewport. A host applying a near
+ // zero height would collapse the frame, and charts render only once they are
+ // in view, so the embed could not recover.
+ if (!content || content.getBoundingClientRect().height === 0) {
+ return { width: body.scrollWidth, height: root.clientHeight };
+ }
+
+ // The bounded filter bar is capped to the frame as well, so the cap comes
off
+ // too or a tall filter list reports as frame-high.
+ const bar = document.querySelector<HTMLElement>('.filter-bar-bounded');
+ const scroller = bar?.querySelector<HTMLElement>('.filter-bar-scroll');
Review Comment:
Fixed in 1cae80c1ac. The class names now live in
`src/dashboard/util/embeddedLayout.ts` (`DASHBOARD_GRID_CLASS`,
`FILTER_BAR_BOUNDED_CLASS`, `FILTER_BAR_SCROLL_CLASS`) and are imported by
`Vertical.tsx`, `DashboardGrid.tsx`, `api.tsx` and the `api.test.ts` fixture,
so a rename in the component now breaks the unit test instead of silently
regressing production. The Playwright spec keeps its own literals on purpose:
it runs against the real DOM, so it fails loudly either way.
--
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]
