You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ms...@apache.org on 2026/08/26 14:58:47 UTC
(superset) 01/01: fix(dashboard): keep the chart menu usable in fullscreen on production builds
This is an automated email from the ASF dual-hosted git repository.
msyavuz pushed a commit to branch msyavuz/fix/fullscreen-chart-menu-prod-data-test
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 63d607ee8497e269305d2b53835cf8c42049d693
Author: Mehmet Salih Yavuz <sa...@proton.me>
AuthorDate: Wed Aug 26 17:55:22 2026 +0300
fix(dashboard): keep the chart menu usable in fullscreen on production builds
The chart three-dots menu was invisible and unclickable while a chart was in
native fullscreen, but only on production builds.
Native fullscreen paints only the fullscreen element's subtree, so AntD popups
portaled to document.body are never drawn. ChartHolder compensates with a
getPopupContainer that re-parents them into the chart holder, but it located the
holder via closest('[data-test="dashboard-component-chart-holder"]').
babel-plugin-jsx-remove-data-test-id strips data-test in the production babel
env, so the lookup returned null and fell back to document.body. Dev builds keep
the attribute, which is why this never reproduced locally.
Resolve the container from document.fullscreenElement instead of a selector, and
move the fullscreen stylesheet off data-test onto stable class hooks — every
rule in it was dead in production for the same reason.
Also drops declarations the Fullscreen spec's UA stylesheet overrides with
!important (position, width/height, box-sizing) and the blanket
`:fullscreen * { pointer-events: auto }`, none of which have ever applied in a
production build.
Claude-Session: https://claude.ai/code/session_01345mdAxbJW9PgEFT9Lvumm
---
.../components/DashboardBuilder/DashboardWrapper.tsx | 2 +-
.../components/SliceHeader/SliceHeader.test.tsx | 12 ++++++++++++
.../src/dashboard/components/SliceHeader/index.tsx | 6 +++++-
.../components/SliceHeaderControls/Styles.tsx | 18 +++---------------
.../gridComponents/ChartHolder/ChartHolder.tsx | 18 +++++++++++-------
5 files changed, 32 insertions(+), 24 deletions(-)
diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardWrapper.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardWrapper.tsx
index de6565b4d51..cf10e62ac83 100644
--- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardWrapper.tsx
+++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardWrapper.tsx
@@ -117,7 +117,7 @@ const StyledDiv = styled.div`
${
isMobileConsumptionEnabled()
? `@media (max-width: ${theme.screenSMMax}px) {
- [data-test='slice-header'] .header-title {
+ .slice-header .header-title {
-webkit-line-clamp: unset;
display: block;
white-space: normal;
diff --git a/superset-frontend/src/dashboard/components/SliceHeader/SliceHeader.test.tsx b/superset-frontend/src/dashboard/components/SliceHeader/SliceHeader.test.tsx
index 955bbb2a5ca..5ccb7fc546d 100644
--- a/superset-frontend/src/dashboard/components/SliceHeader/SliceHeader.test.tsx
+++ b/superset-frontend/src/dashboard/components/SliceHeader/SliceHeader.test.tsx
@@ -208,6 +208,18 @@ test('Should render', () => {
expect(screen.getByTestId('slice-header')).toBeInTheDocument();
});
+test('Should expose a class hook, not just data-test, for fullscreen styling', () => {
+ const props = createProps();
+ render(<SliceHeader {...props} />, {
+ useRedux: true,
+ useRouter: true,
+ initialState,
+ });
+ // The production build strips data-test attributes, so CSS that targets the
+ // header must hang off a class instead.
+ expect(screen.getByTestId('slice-header')).toHaveClass('slice-header');
+});
+
test('Should render - default props', () => {
const props = createProps();
diff --git a/superset-frontend/src/dashboard/components/SliceHeader/index.tsx b/superset-frontend/src/dashboard/components/SliceHeader/index.tsx
index ccf490f087a..ba8e7b27617 100644
--- a/superset-frontend/src/dashboard/components/SliceHeader/index.tsx
+++ b/superset-frontend/src/dashboard/components/SliceHeader/index.tsx
@@ -270,7 +270,11 @@ const SliceHeader = forwardRef<HTMLDivElement, SliceHeaderProps>(
);
return (
- <ChartHeaderStyles data-test="slice-header" ref={ref}>
+ <ChartHeaderStyles
+ className="slice-header"
+ data-test="slice-header"
+ ref={ref}
+ >
<div className="header-title" ref={headerRef}>
<Tooltip title={headerTooltip}>
{/* this div ensures the hover event triggers correctly and prevents flickering */}
diff --git a/superset-frontend/src/dashboard/components/SliceHeaderControls/Styles.tsx b/superset-frontend/src/dashboard/components/SliceHeaderControls/Styles.tsx
index 27d08fc86b6..671728c0dc1 100644
--- a/superset-frontend/src/dashboard/components/SliceHeaderControls/Styles.tsx
+++ b/superset-frontend/src/dashboard/components/SliceHeaderControls/Styles.tsx
@@ -19,20 +19,13 @@
import { css, SupersetTheme } from '@apache-superset/core/theme';
export const fullscreenStyles = (theme: SupersetTheme) => css`
- [data-test='dashboard-component-chart-holder']:fullscreen {
+ .dashboard-component-chart-holder:fullscreen {
background-color: ${theme.colorBgBase};
- width: 100vw;
- height: 100vh;
- box-sizing: border-box;
display: flex;
flex-direction: column;
padding: ${theme.sizeUnit * 4}px;
overflow: visible;
- position: relative;
pointer-events: auto;
- z-index: ${theme.zIndexPopupBase};
- opacity: 1;
- visibility: visible;
/* Ensure children take up available space */
.dashboard-chart,
@@ -58,13 +51,8 @@ export const fullscreenStyles = (theme: SupersetTheme) => css`
}
}
- /* Interaction and Header fixes */
- [data-test='dashboard-component-chart-holder']:fullscreen * {
- pointer-events: auto;
- }
-
- [data-test='dashboard-component-chart-holder']:fullscreen
- [data-test='slice-header'] {
+ /* Keep the header above the chart it shares the fullscreen layer with */
+ .dashboard-component-chart-holder:fullscreen .slice-header {
z-index: ${theme.zIndexPopupBase};
position: relative;
}
diff --git a/superset-frontend/src/dashboard/components/gridComponents/ChartHolder/ChartHolder.tsx b/superset-frontend/src/dashboard/components/gridComponents/ChartHolder/ChartHolder.tsx
index e8183f15199..2060c0b24ea 100644
--- a/superset-frontend/src/dashboard/components/gridComponents/ChartHolder/ChartHolder.tsx
+++ b/superset-frontend/src/dashboard/components/gridComponents/ChartHolder/ChartHolder.tsx
@@ -337,13 +337,17 @@ const ChartHolder = ({
)}
>
<AntdThemeProvider
- getPopupContainer={(triggerNode: HTMLElement) =>
- document.fullscreenElement
- ? (triggerNode?.closest?.(
- '[data-test="dashboard-component-chart-holder"]',
- ) as HTMLElement) || document.body
- : document.body
- }
+ getPopupContainer={(triggerNode?: HTMLElement) => {
+ // Only the fullscreen element's subtree is painted, so popups
+ // have to be portaled into it rather than to document.body.
+ // Resolve it directly instead of matching a selector: the
+ // production build strips data-test attributes.
+ const fullscreenElement =
+ document.fullscreenElement as HTMLElement | null;
+ return triggerNode && fullscreenElement?.contains(triggerNode)
+ ? fullscreenElement
+ : document.body;
+ }}
>
{!editMode && (
<AnchorLink