gabotorresruiz opened a new pull request, #44313:
URL: https://github.com/apache/superset/pull/44313
### SUMMARY
A host embedding a dashboard cannot know how tall the content is, so the
pattern our embedding docs publish asks the dashboard via `getScrollSize()` and
sizes the iframe from the answer.
That loop only terminates if the answer is independent of the iframe height,
and it was not.
Two things made the reported height a function of the frame:
- The app fills its frame by design, so measuring
`document.body.scrollHeight` returned the height the host had just set.
- The vertical filter bar sized its scroll area from `calc(100vh - offset)`,
which inside an iframe is the iframe's own height, and the Apply and Clear
buttons sat **after** that scroll area rather than inside it.
The reported height was therefore always `frame + constant`, so a host
applying it grew the iframe on every poll, without bound and without decay.
The same root cause has a second, user visible symptom: because the panel
stretched to the full frame, the filter list stopped scrolling inside its own
panel and the action buttons followed the frame to the bottom, far outside what
the viewer could see.
#### Approach
- `getScrollSize()` lifts the fill the frame constraint for a single
synchronous read, so it describes the content rather than handing the frame
back. Inline styles are used deliberately: a stylesheet rule has to win a
specificity contest, and losing it silently turns the call back into a
measurement of the frame.
- Before the dashboard lays out there is nothing to measure, so it reports
the viewport. A host applying a near zero height would collapse the embed, and
charts only render once they are in view, so it could not recover.
- In an embed the filter bar becomes a bounded flex column, as tall as its
content but never taller than the frame. The filter list is the only part that
scrolls, so the header and the buttons stay inside the panel.
- `VersionHistoryColumn` no longer renders in embeds. It is sized from
`100vh`, renders empty for guests, and pinned the document height to the frame.
- Separately, the dashboard header was capped with `max-width: calc(100vw -
filterBarWidth)`. Viewport units include the scrollbar gutter, so with classic
scrollbars the header ran past the visible edge and **every** dashboard with
the filter bar open scrolled horizontally by the width of the scrollbar. The
grid track already knows its own width.
#### Known limitation
With a frame larger than the content the buttons are no longer pinned to the
visible area, because a guest document cannot know which part of itself the
host has on screen.
Their distance is now bounded by the content height rather than the iframe
height, which is the part that used to scale without limit.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Measured with a 746px browser window on a dashboard with 12 native filters
and a vertical filter bar.
| | before | after |
|---|---|---|
| Reported height at an 800px frame | 1665 | 1665 |
| Reported height at a 2400px frame | 2400 | **1665** |
| Documented resize loop, per tick delta | `+40` forever, no decay | `0`
after two ticks |
| Apply filters in a 4000px frame | 3908px down | **1045px** down |
| Apply filters in a 6000px frame | tracks the frame | **1045px** down,
capped |
| Fit to window, document vs frame | 785 vs 767 | 767 vs 767 |
| Horizontal overflow, filter bar open | 15px | 0 |
Reported height is identical at ten frame sizes from 400 to 6000.
### TESTING INSTRUCTIONS
Automated, added in this PR:
```bash
cd superset-frontend
INCLUDE_EMBEDDED=true npx playwright test
playwright/tests/embedded/embedded-iframe-sizing.spec.ts
--project=chromium-embedded
```
Four tests, all of which run against the existing embedded test app:
1. `reported content height does not depend on the iframe height`
2. `the documented resize loop settles instead of growing`
3. `measuring the content leaves no inline styles behind`
4. `a frame taller than the window keeps the content at the top`
On `master` the first two fail with exactly the reported signature:
```
reported content height does not depend on the iframe height
Expected: 1665
Received: 2400
the documented resize loop settles instead of growing
Expected [0, 0, 0]
Received [40, 40, 40]
```
Note that the second one reproduces on the `world_health` example dashboard,
which has **no** vertical filter bar.
The loop runs away purely because the reported height follows the frame and
hosts add headroom of their own.
Manual:
1. Embed a dashboard with a vertical filter bar and several filters.
2. Poll `handle.getScrollSize()` on an interval and apply the returned
height to the iframe. The iframe should settle within a couple of ticks and
stay there. Leave it running for two minutes to be sure.
3. Set the iframe to a fixed 4000px. Apply filters should sit just below the
last filter rather than at the bottom of the frame.
4. Bind the iframe to the browser window. The filter list should scroll
inside its own panel with the buttons pinned at the bottom of the panel.
5. Open any dashboard normally with the filter bar open and confirm there is
no horizontal scrollbar.
Verified in Chromium, WebKit and Firefox, on dashboards with 12 filters, 2
filters and a horizontal filter bar, in edit mode, in standalone modes 1, 2 and
3, and at phone width with mobile consumption mode enabled.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
#### Review guidance
The riskiest hunk is the `StyledHeader` change in `DashboardBuilder.tsx`,
because it is the only one that affects **every** dashboard rather than embeds
alone.
Everything else is gated on `isEmbedded()`, so a non embedded dashboard
renders byte identical CSS.
Suggested reading order:
1. `src/embedded/api.tsx`, the measurement itself.
2. `FilterBar/Vertical.tsx`, the bounded bar.
3. `DashboardBuilder.tsx`, the version history guard, the column separator,
and the header cap.
4. The new spec.
--
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]
