From d1e8e441f645a25e59d50b5d0744cb38a2dbdc86 Mon Sep 17 00:00:00 2001
From: Oisin Coveney <oisin@metabase.com>
Date: Wed, 17 Apr 2024 09:16:46 +0300
Subject: [PATCH] Convert `EmbedFrame.module.css` to CSS modules (#41367)
---
.../embedding/embedding-dashboard.cy.spec.js | 13 +++++--
.../bar_chart.cy.spec.js | 4 +--
.../line_chart.cy.spec.js | 2 +-
.../drillthroughs/chart_drill.cy.spec.js | 12 ++++---
.../pivot_tables.cy.spec.js | 4 +--
.../src/metabase/css/dashboard.module.css | 5 +--
.../ParameterValueWidget.module.css | 2 +-
.../EmbedFrame/EmbedFrame.module.css | 33 ++++++++---------
.../components/EmbedFrame/EmbedFrame.tsx | 36 +++++++++++++++----
.../PublicApp/PublicApp.unit.spec.tsx | 15 ++++----
.../components/LineAreaBarChart.module.css | 8 ++---
.../lib/LineAreaBarPostRender.js | 26 ++++++++++++--
12 files changed, 106 insertions(+), 54 deletions(-)
diff --git a/e2e/test/scenarios/embedding/embedding-dashboard.cy.spec.js b/e2e/test/scenarios/embedding/embedding-dashboard.cy.spec.js
index 1323acc5eb6..47d33221727 100644
--- a/e2e/test/scenarios/embedding/embedding-dashboard.cy.spec.js
+++ b/e2e/test/scenarios/embedding/embedding-dashboard.cy.spec.js
@@ -614,12 +614,21 @@ describeEE("scenarios > embedding > dashboard appearance", () => {
cy.log("Assert dashboard theme");
getIframeBody()
.findByTestId("embed-frame")
- .should("not.have.class", "Theme--transparent");
+ .invoke("attr", "data-embed-theme")
+ .then(embedTheme => {
+ expect(embedTheme).to.eq("light");
+ });
+
// We're getting an input element which is 0x0 in size
cy.findByLabelText("Transparent").click({ force: true });
+ cy.wait(1000);
getIframeBody()
.findByTestId("embed-frame")
- .should("have.class", "Theme--transparent");
+ .invoke("attr", "data-embed-theme")
+ .then(embedTheme => {
+ expect(embedTheme).to.eq("transparent");
+ });
+
cy.get("@previewEmbedSpy").should("have.callCount", 1);
cy.log("Assert dashboard title");
diff --git a/e2e/test/scenarios/visualizations-charts/bar_chart.cy.spec.js b/e2e/test/scenarios/visualizations-charts/bar_chart.cy.spec.js
index de38e3bc59d..e29ccd34d5c 100644
--- a/e2e/test/scenarios/visualizations-charts/bar_chart.cy.spec.js
+++ b/e2e/test/scenarios/visualizations-charts/bar_chart.cy.spec.js
@@ -168,7 +168,7 @@ describe("scenarios > visualizations > bar chart", () => {
.findByText(columnName)
.should("not.exist");
cy.findAllByTestId("legend-item").should("have.length", 3);
- cy.get(".enable-dots").should("have.length", 3);
+ cy.findAllByTestId("chart-series").should("have.length", 3);
});
getDraggableElements()
@@ -185,7 +185,7 @@ describe("scenarios > visualizations > bar chart", () => {
.findByText(columnName)
.should("exist");
cy.findAllByTestId("legend-item").should("have.length", 4);
- cy.get(".enable-dots").should("have.length", 4);
+ cy.findAllByTestId("chart-series").should("have.length", 4);
});
cy.findAllByTestId("legend-item").contains("Gadget").click();
diff --git a/e2e/test/scenarios/visualizations-charts/line_chart.cy.spec.js b/e2e/test/scenarios/visualizations-charts/line_chart.cy.spec.js
index 765f1175d91..adf64eaec2a 100644
--- a/e2e/test/scenarios/visualizations-charts/line_chart.cy.spec.js
+++ b/e2e/test/scenarios/visualizations-charts/line_chart.cy.spec.js
@@ -130,7 +130,7 @@ describe("scenarios > visualizations > line chart", () => {
});
cy.findByTestId("query-visualization-root")
- .get(".enable-dots")
+ .findAllByTestId("chart-series")
.last()
.find(".dot")
.eq(3)
diff --git a/e2e/test/scenarios/visualizations-tabular/drillthroughs/chart_drill.cy.spec.js b/e2e/test/scenarios/visualizations-tabular/drillthroughs/chart_drill.cy.spec.js
index d0e437e1bad..d41757307da 100644
--- a/e2e/test/scenarios/visualizations-tabular/drillthroughs/chart_drill.cy.spec.js
+++ b/e2e/test/scenarios/visualizations-tabular/drillthroughs/chart_drill.cy.spec.js
@@ -163,7 +163,8 @@ describe("scenarios > visualizations > drillthroughs > chart drill", () => {
visitDashboard(DASHBOARD_ID);
cy.log("The first series line");
- cy.get(".sub.enable-dots._0")
+ cy.findAllByTestId("chart-series")
+ .eq(0)
.find(".dot")
.eq(0)
.click({ force: true });
@@ -175,7 +176,8 @@ describe("scenarios > visualizations > drillthroughs > chart drill", () => {
// Second line from the second question
cy.log("The second series line");
- cy.get(".sub.enable-dots._1")
+ cy.findAllByTestId("chart-series")
+ .eq(1)
.find(".dot")
.eq(0)
.click({ force: true });
@@ -232,7 +234,8 @@ describe("scenarios > visualizations > drillthroughs > chart drill", () => {
visitDashboard(DASHBOARD_ID);
cy.log("The first series line");
- cy.get(".sub.enable-dots._0")
+ cy.findAllByTestId("chart-series")
+ .eq(0)
.find(".dot")
.eq(0)
.click({ force: true });
@@ -244,7 +247,8 @@ describe("scenarios > visualizations > drillthroughs > chart drill", () => {
// Second line from the second question
cy.log("The third series line");
- cy.get(".sub.enable-dots._2")
+ cy.findAllByTestId("chart-series")
+ .eq(2)
.find(".dot")
.eq(0)
.click({ force: true });
diff --git a/e2e/test/scenarios/visualizations-tabular/pivot_tables.cy.spec.js b/e2e/test/scenarios/visualizations-tabular/pivot_tables.cy.spec.js
index dfa378cc580..0caaa17dd9d 100644
--- a/e2e/test/scenarios/visualizations-tabular/pivot_tables.cy.spec.js
+++ b/e2e/test/scenarios/visualizations-tabular/pivot_tables.cy.spec.js
@@ -669,7 +669,7 @@ describe("scenarios > visualizations > pivot tables", { tags: "@slow" }, () => {
.then($value => {
cy.visit($value);
});
- cy.get(".EmbedFrame-header").contains(test.subject);
+ cy.findByTestId("embed-frame-header").contains(test.subject);
assertOnPivotFields();
});
@@ -699,7 +699,7 @@ describe("scenarios > visualizations > pivot tables", { tags: "@slow" }, () => {
// visit the iframe src directly to ensure it's not sing preview endpoints
visitIframe();
- cy.get(".EmbedFrame-header").contains(test.subject);
+ cy.findByTestId("embed-frame-header").contains(test.subject);
assertOnPivotFields();
});
});
diff --git a/frontend/src/metabase/css/dashboard.module.css b/frontend/src/metabase/css/dashboard.module.css
index 6aa8778bd6a..143f568fdc9 100644
--- a/frontend/src/metabase/css/dashboard.module.css
+++ b/frontend/src/metabase/css/dashboard.module.css
@@ -29,8 +29,9 @@
}
.Dashboard.DashboardNight
- :global(.enable-dots-onhover .dc-tooltip circle.dot:hover),
-.Dashboard.DashboardNight :global(.enable-dots .dc-tooltip circle.dot) {
+ .enableDotsOnHover
+ :global(.dc-tooltip circle.dot:hover),
+.Dashboard.DashboardNight .enableDots :global(.dc-tooltip circle.dot) {
fill: currentColor;
}
diff --git a/frontend/src/metabase/parameters/components/ParameterValueWidget.module.css b/frontend/src/metabase/parameters/components/ParameterValueWidget.module.css
index d2d1ae7b666..131a29d9eac 100644
--- a/frontend/src/metabase/parameters/components/ParameterValueWidget.module.css
+++ b/frontend/src/metabase/parameters/components/ParameterValueWidget.module.css
@@ -59,7 +59,7 @@
}
.DashboardNight .parameter.noPopover input:focus,
-:global(.Theme--night) .parameter.noPopover input:focus {
+.ThemeNight .parameter.noPopover input:focus {
color: var(--color-text-white);
}
diff --git a/frontend/src/metabase/public/components/EmbedFrame/EmbedFrame.module.css b/frontend/src/metabase/public/components/EmbedFrame/EmbedFrame.module.css
index 8110f3130e3..b39ac88eddc 100644
--- a/frontend/src/metabase/public/components/EmbedFrame/EmbedFrame.module.css
+++ b/frontend/src/metabase/public/components/EmbedFrame/EmbedFrame.module.css
@@ -2,60 +2,57 @@ body {
background-color: transparent;
}
-:global(.EmbedFrame) {
+.EmbedFrame {
background-color: white;
}
-:global(.EmbedFrame-header),
-:global(.EmbedFrame-footer) {
+.EmbedFrameHeader,
+.EmbedFrameFooter {
color: var(--color-text-dark);
background-color: white;
}
-:global(.Theme--night.EmbedFrame) {
+.ThemeNight.EmbedFrame {
background-color: var(--color-bg-black);
border: 1px solid var(--color-bg-dark);
}
-:global(.Theme--night .EmbedFrame-header),
-:global(.Theme--night .EmbedFrame-footer) {
+.ThemeNight .EmbedFrameHeader,
+.ThemeNight .EmbedFrameFooter {
color: color-mod(var(--color-text-white) alpha(-14%));
background-color: var(--color-bg-black);
border-color: var(--color-bg-dark);
}
-:global(.Theme--night.EmbedFrame) .fullscreenNightText {
+.ThemeNight.EmbedFrame .fullscreenNightText {
color: color-mod(var(--color-text-white) alpha(-14%));
transition: color 1s linear;
}
-:global(.Theme--night.EmbedFrame svg text) {
+.ThemeNight.EmbedFrame svg text {
fill: color-mod(var(--color-text-white) alpha(-14%)) !important;
stroke: none !important;
}
-:global(.Theme--night.EmbedFrame) .DashCard .Card {
+.ThemeNight.EmbedFrame .DashCard .Card {
background-color: var(--color-bg-black);
border: 1px solid var(--color-bg-dark);
}
-:global(.Theme--night.EmbedFrame
- .enable-dots-onhover
- .dc-tooltip
- circle.dot:hover),
-:global(.Theme--night.EmbedFrame .enable-dots .dc-tooltip circle.dot) {
+.ThemeNight.EmbedFrame .enableDotsOnHover :global(.dc-tooltip circle.dot:hover),
+.ThemeNight.EmbedFrame .enableDots :global(.dc-tooltip circle.dot) {
fill: currentColor;
}
-:global(.Theme--transparent.EmbedFrame) {
+.ThemeTransparent.EmbedFrame {
background-color: transparent;
}
-:global(.Theme--transparent .EmbedFrame-header),
-:global(.Theme--transparent .EmbedFrame-footer) {
+.ThemeTransparent .EmbedFrameHeader,
+.ThemeTransparent .EmbedFrameFooter {
background-color: transparent;
}
-:global(.Theme--transparent.EmbedFrame) .DashCard .Card {
+.ThemeTransparent.EmbedFrame .DashCard .Card {
background-color: transparent;
}
diff --git a/frontend/src/metabase/public/components/EmbedFrame/EmbedFrame.tsx b/frontend/src/metabase/public/components/EmbedFrame/EmbedFrame.tsx
index 212d5e8d8c0..f2ad14666a1 100644
--- a/frontend/src/metabase/public/components/EmbedFrame/EmbedFrame.tsx
+++ b/frontend/src/metabase/public/components/EmbedFrame/EmbedFrame.tsx
@@ -35,7 +35,9 @@ import type {
} from "metabase-types/api";
import type { State } from "metabase-types/store";
-import "./EmbedFrame.module.css";
+import ParameterValueWidgetS from "../../../parameters/components/ParameterValueWidget.module.css";
+
+import EmbedFrameS from "./EmbedFrame.module.css";
import type { FooterVariant } from "./EmbedFrame.styled";
import {
ActionButtonsContainer,
@@ -84,7 +86,7 @@ type Props = OwnProps &
interface HashOptions {
bordered?: boolean;
titled?: boolean;
- theme?: string;
+ theme?: "night" | "transparent";
hide_parameters?: string;
hide_download_button?: boolean;
}
@@ -95,6 +97,20 @@ function mapStateToProps(state: State) {
};
}
+const EMBED_THEME_CLASSES = (theme: HashOptions["theme"]) => {
+ if (!theme) {
+ return null;
+ }
+
+ if (theme === "night") {
+ return cx(ParameterValueWidgetS.ThemeNight, EmbedFrameS.ThemeNight);
+ }
+
+ if (theme === "transparent") {
+ return EmbedFrameS.ThemeTransparent;
+ }
+};
+
function EmbedFrame({
className,
children,
@@ -175,15 +191,18 @@ function EmbedFrame({
<Root
hasScroll={hasFrameScroll}
isBordered={bordered}
- className={cx("EmbedFrame", className, {
- [`Theme--${theme}`]: !!theme,
- })}
+ className={cx(
+ EmbedFrameS.EmbedFrame,
+ className,
+ EMBED_THEME_CLASSES(theme),
+ )}
data-testid="embed-frame"
+ data-embed-theme={theme}
>
<ContentContainer>
{hasHeader && (
<Header
- className="EmbedFrame-header"
+ className={EmbedFrameS.EmbedFrameHeader}
data-testid="embed-frame-header"
>
{finalName && (
@@ -247,7 +266,10 @@ function EmbedFrame({
<Body>{children}</Body>
</ContentContainer>
{showFooter && (
- <Footer className="EmbedFrame-footer" variant={footerVariant}>
+ <Footer
+ className={EmbedFrameS.EmbedFrameFooter}
+ variant={footerVariant}
+ >
{hasEmbedBranding && (
<LogoBadge variant={footerVariant} dark={theme === "night"} />
)}
diff --git a/frontend/src/metabase/public/containers/PublicApp/PublicApp.unit.spec.tsx b/frontend/src/metabase/public/containers/PublicApp/PublicApp.unit.spec.tsx
index 92e39a7e944..51c1cdb6db6 100644
--- a/frontend/src/metabase/public/containers/PublicApp/PublicApp.unit.spec.tsx
+++ b/frontend/src/metabase/public/containers/PublicApp/PublicApp.unit.spec.tsx
@@ -120,18 +120,15 @@ describe("PublicApp", () => {
const embedFrame = screen.getByTestId("embed-frame");
- // eslint-disable-next-line jest-dom/prefer-to-have-class
- expect(embedFrame.className).not.toEqual(
- expect.stringContaining("Theme--"),
- );
+ expect(embedFrame).not.toHaveAttribute("data-embed-theme");
});
- test.each([
- ["night", "Theme--night"],
- ["transparent", "Theme--transparent"],
- ])("correctly handles %s theme", (theme, expectedClass) => {
+ test.each(["night", "transparent"])("correctly handles %s theme", theme => {
setup({ hash: `#theme=${theme}` });
- expect(screen.getByTestId("embed-frame")).toHaveClass(expectedClass);
+ expect(screen.getByTestId("embed-frame")).toHaveAttribute(
+ "data-embed-theme",
+ theme,
+ );
});
});
});
diff --git a/frontend/src/metabase/visualizations/components/LineAreaBarChart.module.css b/frontend/src/metabase/visualizations/components/LineAreaBarChart.module.css
index 286ca812d67..273557054a2 100644
--- a/frontend/src/metabase/visualizations/components/LineAreaBarChart.module.css
+++ b/frontend/src/metabase/visualizations/components/LineAreaBarChart.module.css
@@ -152,12 +152,12 @@
stroke-width: 3px;
}
-.LineAreaBarChart :global(.dc-chart .enable-dots .dc-tooltip .dot),
+.LineAreaBarChart :global(.dc-chart) .enableDots :global(.dc-tooltip .dot),
.LineAreaBarChart :global(.dc-chart .dc-tooltip .dot.selected),
.LineAreaBarChart
- :global(.dc-chart .enable-dots-onhover .dc-tooltip .dot:hover),
-.LineAreaBarChart
- :global(.dc-chart .enable-dots-onhover .dc-tooltip .dot.hover) {
+ :global(.dc-chart)
+ .enableDotsOnHover
+ :global(.dc-tooltip .dot:hover) {
fill: white;
stroke: currentColor;
fill-opacity: 1 !important;
diff --git a/frontend/src/metabase/visualizations/lib/LineAreaBarPostRender.js b/frontend/src/metabase/visualizations/lib/LineAreaBarPostRender.js
index 384f8483b4e..08f123ff24d 100644
--- a/frontend/src/metabase/visualizations/lib/LineAreaBarPostRender.js
+++ b/frontend/src/metabase/visualizations/lib/LineAreaBarPostRender.js
@@ -1,9 +1,13 @@
+import cx from "classnames";
import d3 from "d3";
import _ from "underscore";
import CS from "metabase/css/core/index.css";
+import DashboardS from "metabase/css/dashboard.module.css";
import { color } from "metabase/lib/colors";
import { clipPathReference, moveToFront } from "metabase/lib/dom";
+import EmbedFrameS from "metabase/public/components/EmbedFrame/EmbedFrame.module.css";
+import LineAreaBarChartS from "metabase/visualizations/components/LineAreaBarChart.module.css";
import { adjustYAxisTicksIfNeeded } from "./apply_axis";
import { onRenderValueLabels } from "./chart_values";
@@ -154,8 +158,23 @@ function onRenderEnableDots(chart) {
chart
.svg()
.select(`.sub._${index}`)
- .classed("enable-dots", enableDots)
- .classed("enable-dots-onhover", !enableDots);
+ .attr("data-enable-dots", enableDots)
+ .classed(
+ cx(
+ DashboardS.enableDots,
+ EmbedFrameS.enableDots,
+ LineAreaBarChartS.enableDots,
+ ),
+ enableDots,
+ )
+ .classed(
+ cx(
+ DashboardS.enableDotsOnHover,
+ EmbedFrameS.enableDotsOnHover,
+ LineAreaBarChartS.enableDotsOnHover,
+ ),
+ !enableDots,
+ );
}
}
@@ -435,6 +454,9 @@ function onRender(
onDeselectTimelineEvents,
},
) {
+ chart.svg().attr("data-testid", "chart");
+ chart.selectAll(`.sub`).attr("data-testid", `chart-series`);
+
onRenderRemoveClipPath(chart);
onRenderMoveContentToTop(chart);
onRenderReorderCharts(chart);
--
GitLab