sadpandajoe opened a new pull request, #45056:
URL: https://github.com/apache/superset/pull/45056
### SUMMARY
Root cause: #31590 (commit dd129fa403, the Ant Design v5 overhaul) replaced
the Superset 5 shell — `body { min-height: 100vh; display: flex;
flex-direction: column }`, `#app { flex: 1 1 auto; position: relative;
display: flex; flex-direction: column }` (from
superset-frontend/src/assets/stylesheets/less/index.less) — with a fixed
`html, body, #app { height: 100%; }` model in
superset-frontend/packages/superset-core/src/theme/GlobalStyles.tsx. When a
host page embeds Superset in the same document and injects content above
`#app` (the reporter embeds it under a geOrchestra menu), `#app` still
claims a full viewport instead of shrinking for that sibling, and its
bottom gets pushed off-screen. On Explore, `body { height: 100vh;
max-height: 100vh; overflow: hidden }` turns that overflow into something
genuinely unreachable.
The fix has two halves. `GlobalStyles.tsx`: `body` is a `min-height: 100vh`
flex column again, and `#app` has no explicit height, so it flex-grows into
whatever space its siblings leave it. `App.tsx`: `pageScrollShellCss` — the
`<Flex>` that is `#app`'s only in-flow child — drops its own `min-height:
100vh` for `flex: 1 1 auto; min-height: 0`, so it sizes to the space `#app`
grants it rather than re-claiming a full viewport on its own. Shrinking
`#app` alone isn't enough: without this, the shell would overflow `#app`
again and `#app`'s `overflow: hidden` would clip it right back.
Content taller than `#app` still grows the shell, so window page scrolling
and hide-navbar-on-scroll behavior are unchanged.
`lockedShellCss` (the chat-panel-open shell) is deliberately untouched: with
a host-injected header, that shell flex-shrinks to fit `#app` on Explore the
same way, and on page-scroll routes it still needs a page scroll equal to
the injected height — unchanged from before this fix.
Explore's own `#app` override (`ExploreViewContainer/index.tsx`) needs no
change — it already carries `height: 100%` from before #31590, and a
Chromium ablation confirms that property, `min-height: 0`, no property at
all, and even `height: 100vh` all produce byte-identical geometry here,
since `flex-basis: 100%` already overrides height for flex-basis purposes
and `overflow: hidden` already zeroes the item's automatic minimum size.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
No screenshots attached. A live Explore preview with a 60px host banner was
captured before and after, but the two deploys loaded different default
charts, so the images are not a like-for-like comparison. Evidence instead:
a Chromium (Playwright) layout harness, 1000x800 viewport, with a 60px
`<div>`
injected as a sibling before `#app`, reproducing the real
`body → #app → shell → Layout/Layout.Content → portal` chain and antd's own
Layout defaults. On Explore, the bottom control's bottom edge sits at y=850
(clipped, and unreachable even after simulated wheel-scroll input) before
this fix, and at y=790 (fully visible, viewport height 800) after. The
no-header case is identical before and after. SQL Lab, a generic tall
page-scroll route, and the chat-locked shell measure identically with and
without the injected header.
### TESTING INSTRUCTIONS
1. Add a 60px `<div>` immediately before `<div id="app">` in
superset/templates/superset/spa.html (simulating a host page's injected
banner/menu).
2. Open Explore for any chart — the bottom chart-action controls should be
fully reachable (not clipped, no scrolling needed).
3. Remove the injected `<div>` and confirm Explore, SQL Lab, a dashboard,
and the welcome page all look visually unchanged; long pages should
still page-scroll with the navbar hiding on scroll, and the chat panel
(if the extension is enabled) should still lock the shell as before.
The two added tests (App.test.tsx, GlobalStyles.test.tsx) assert the CSS
contract directly (the actual emotion-injected rules), not rendered
geometry — jsdom doesn't compute layout, so they don't substitute for the
manual check above.
### ADDITIONAL INFORMATION
- [x] Has associated issue: Fixes #44867
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in SIP-59)
- [ ] 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
--
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]
