diff --git a/frontend/src/metabase/visualizations/components/ChartWithLegend.jsx b/frontend/src/metabase/visualizations/components/ChartWithLegend.jsx
index f655c4a058a793e32f345dd36e75e8c89ec61fb1..cdad0444138750f853f0a49d1101895c255da085 100644
--- a/frontend/src/metabase/visualizations/components/ChartWithLegend.jsx
+++ b/frontend/src/metabase/visualizations/components/ChartWithLegend.jsx
@@ -14,11 +14,14 @@ const GRID_ASPECT_RATIO = 4 / 3;
const PADDING = 14;
const DEFAULT_GRID_SIZE = 100;
+export const HIDE_HORIZONTAL_LEGEND_THRESHOLD = 180;
+export const HIDE_SECONDARY_INFO_THRESHOLD = 260;
class ChartWithLegend extends Component {
static defaultProps = {
aspectRatio: 1,
style: {},
+ showLegend: true,
};
render() {
@@ -57,16 +60,13 @@ class ChartWithLegend extends Component {
let type;
let LegendComponent;
const isHorizontal = gridSize.width > gridSize.height / GRID_ASPECT_RATIO;
- if (showLegend === false) {
+ if (!showLegend) {
type = "small";
- } else if (
- !gridSize ||
- (isHorizontal &&
- (showLegend || gridSize.width > 4 || gridSize.height > 4))
- ) {
+ } else if (isHorizontal && width > HIDE_HORIZONTAL_LEGEND_THRESHOLD) {
type = "horizontal";
LegendComponent = LegendVertical;
- if (gridSize && gridSize.width < 6) {
+
+ if (width < HIDE_SECONDARY_INFO_THRESHOLD) {
legendTitles = legendTitles.map(title =>
Array.isArray(title) ? title.slice(0, 1) : title,
);
@@ -78,18 +78,14 @@ class ChartWithLegend extends Component {
chartWidth = desiredWidth;
}
chartHeight = height;
- } else if (
- !isHorizontal &&
- (showLegend || (gridSize.height > 3 && gridSize.width > 2))
- ) {
+ } else if (!isHorizontal && gridSize.height > 3 && gridSize.width > 2) {
type = "vertical";
LegendComponent = LegendHorizontal;
legendTitles = legendTitles.map(title =>
- Array.isArray(title) ? title.join(" – ") : title,
+ Array.isArray(title) ? title.join(" ") : title,
);
const desiredHeight = width * (1 / aspectRatio);
if (desiredHeight > height * (3 / 4)) {
- // chartHeight = height * (3 / 4);
flexChart = true;
} else {
chartHeight = desiredHeight;
@@ -99,6 +95,8 @@ class ChartWithLegend extends Component {
type = "small";
}
+ const hasDimensions = width > 0 && height > 0;
+
const legend = LegendComponent ? (
<LegendComponent
className={styles.Legend}
@@ -138,7 +136,7 @@ class ChartWithLegend extends Component {
className={cx(styles.Chart)}
style={{ width: chartWidth, height: chartHeight }}
>
- {children}
+ {hasDimensions ? children : null}
</div>
{/* spacer div to balance legend */}
{legend && (
diff --git a/frontend/src/metabase/visualizations/components/ChartWithLegend.unit.spec.jsx b/frontend/src/metabase/visualizations/components/ChartWithLegend.unit.spec.jsx
new file mode 100644
index 0000000000000000000000000000000000000000..527d5e5cefb282f2c78100c4cc96538ab513d996
--- /dev/null
+++ b/frontend/src/metabase/visualizations/components/ChartWithLegend.unit.spec.jsx
@@ -0,0 +1,75 @@
+import { render, screen } from "@testing-library/react";
+
+import ChartWithLegend, {
+ HIDE_HORIZONTAL_LEGEND_THRESHOLD,
+ HIDE_SECONDARY_INFO_THRESHOLD,
+} from "./ChartWithLegend";
+
+const defaultProps = {
+ legendTitles: [
+ ["Series 1", "50%"],
+ ["Series 2", "50%"],
+ ],
+ legendColors: ["Red", "Green"],
+ showLegend: true,
+ width: 800,
+ height: 600,
+ gridSize: { width: 8, height: 6 },
+ children: <div>Chart stub</div>,
+};
+
+const setup = (props = {}) => {
+ render(<ChartWithLegend {...defaultProps} {...props} />);
+};
+
+describe("ChartWithLegend", () => {
+ it("should show full legend titles when width is above secondary info threshold", () => {
+ setup();
+
+ expect(screen.getByTestId("chart-legend")).toHaveTextContent("Series 1");
+ expect(screen.getByTestId("chart-legend")).toHaveTextContent("Series 2");
+ expect(screen.getByTestId("chart-legend")).toHaveTextContent("50%");
+ });
+
+ it("should hide secondary info when width is below threshold", () => {
+ setup({ width: HIDE_SECONDARY_INFO_THRESHOLD - 1 });
+
+ expect(screen.getByTestId("chart-legend")).toHaveTextContent("Series 1");
+ expect(screen.getByTestId("chart-legend")).toHaveTextContent("Series 2");
+ expect(screen.getByTestId("chart-legend")).not.toHaveTextContent("50%");
+ });
+
+ it("should not render legend when width is below horizontal legend threshold", () => {
+ setup({ width: HIDE_HORIZONTAL_LEGEND_THRESHOLD - 1 });
+
+ expect(screen.queryByTestId("chart-legend")).not.toBeInTheDocument();
+ });
+
+ it("should not render chart content when dimensions are zero", () => {
+ setup({ width: 0, height: 0 });
+
+ expect(screen.queryByText("Chart stub")).not.toBeInTheDocument();
+ });
+
+ it("should render chart content when dimensions are valid", () => {
+ setup();
+
+ expect(screen.getByText("Chart stub")).toBeInTheDocument();
+ });
+
+ it("should join legend titles with space in vertical layout", () => {
+ setup({
+ width: 400,
+ height: 600,
+ gridSize: { width: 3, height: 6 },
+ aspectRatio: 0.5,
+ legendTitles: [
+ ["Foo", "10%"],
+ ["Bar", "90%"],
+ ],
+ });
+
+ expect(screen.getByTestId("chart-legend")).toHaveTextContent("Foo 10%");
+ expect(screen.getByTestId("chart-legend")).toHaveTextContent("Bar 90%");
+ });
+});
diff --git a/frontend/src/metabase/visualizations/components/EChartsRenderer/EChartsRenderer.styled.tsx b/frontend/src/metabase/visualizations/components/EChartsRenderer/EChartsRenderer.styled.tsx
index ead78bc1336da41803ecf68c37a25c1e48699180..a0bfdddc267e05c53f9bb2633e5ba74d02d84d96 100644
--- a/frontend/src/metabase/visualizations/components/EChartsRenderer/EChartsRenderer.styled.tsx
+++ b/frontend/src/metabase/visualizations/components/EChartsRenderer/EChartsRenderer.styled.tsx
@@ -1,6 +1,9 @@
import styled from "@emotion/styled";
export const EChartsRendererRoot = styled.div`
+ display: flex;
+ align-items: center;
+ justify-content: center;
/* HACK: zrender adds user-select: none to the root svg element which prevents users from selecting text on charts */
& svg {
user-select: auto !important;
diff --git a/frontend/src/metabase/visualizations/components/LegendVertical.jsx b/frontend/src/metabase/visualizations/components/LegendVertical.jsx
index 26665261b268ea374701c678780765fd2b79bf59..2b14c4937e3a735e43a836dd168ef6d9ff6c3109 100644
--- a/frontend/src/metabase/visualizations/components/LegendVertical.jsx
+++ b/frontend/src/metabase/visualizations/components/LegendVertical.jsx
@@ -18,16 +18,10 @@ export default class LegendVertical extends Component {
state = {
overflowCount: 0,
size: null,
- listWidth: null,
};
listRef = createRef();
- componentDidMount() {
- const listWidth = this.listRef.current.getBoundingClientRect().width;
- this.setState({ listWidth });
- }
-
componentDidUpdate(prevProps, prevState) {
// Get the bounding rectangle of the chart widget to determine if
// legend items will overflow the widget area
@@ -55,20 +49,6 @@ export default class LegendVertical extends Component {
this.setState({ overflowCount, size });
}
}
-
- const [previousSampleTitle] = prevProps.titles || [];
- const [sampleTitle] = this.props.titles || [];
-
- if (
- sampleTitle &&
- previousSampleTitle &&
- previousSampleTitle.length !== sampleTitle.length
- ) {
- this.setState({ listWidth: null }, () => {
- const listWidth = this.listRef.current.getBoundingClientRect().width;
- this.setState({ listWidth });
- });
- }
}
render() {
@@ -95,7 +75,6 @@ export default class LegendVertical extends Component {
return (
<ol
className={cx(className, LegendS.Legend, LegendS.vertical)}
- style={{ width: this.state.listWidth }}
ref={this.listRef}
>
{items.map((title, index) => {
diff --git a/frontend/src/metabase/visualizations/visualizations/PieChart/PieChart.tsx b/frontend/src/metabase/visualizations/visualizations/PieChart/PieChart.tsx
index 64a81e545074fd1fd912ef338a27b50979969cb5..7fbb183a84b056ff108a79b503589f48c1d4bb6f 100644
--- a/frontend/src/metabase/visualizations/visualizations/PieChart/PieChart.tsx
+++ b/frontend/src/metabase/visualizations/visualizations/PieChart/PieChart.tsx
@@ -117,27 +117,34 @@ export function PieChart(props: VisualizationProps) {
const eventHandlers = useChartEvents(props, chartRef, chartModel);
- const slices = getArrayFromMapValues(chartModel.sliceTree);
- const legendTitles = slices
- .filter(s => s.includeInLegend)
- .map(s => {
- const label = s.name;
-
- // Hidden slices don't have a percentage
- const sliceHidden = s.normalizedPercentage === 0;
- const percentDisabled =
- settings["pie.percent_visibility"] !== "legend" &&
- settings["pie.percent_visibility"] !== "both";
-
- if (sliceHidden || percentDisabled) {
- return [label];
- }
-
- return [
- label,
- formatters.formatPercent(s.normalizedPercentage, "legend"),
- ];
- });
+ const slices = useMemo(
+ () => getArrayFromMapValues(chartModel.sliceTree),
+ [chartModel.sliceTree],
+ );
+ const legendTitles = useMemo(
+ () =>
+ slices
+ .filter(s => s.includeInLegend)
+ .map(s => {
+ const label = s.name;
+
+ // Hidden slices don't have a percentage
+ const sliceHidden = s.normalizedPercentage === 0;
+ const percentDisabled =
+ settings["pie.percent_visibility"] !== "legend" &&
+ settings["pie.percent_visibility"] !== "both";
+
+ if (sliceHidden || percentDisabled) {
+ return [label];
+ }
+
+ return [
+ label,
+ formatters.formatPercent(s.normalizedPercentage, "legend"),
+ ];
+ }),
+ [formatters, settings, slices],
+ );
const hiddenSlicesLegendIndices = slices
.filter(s => s.includeInLegend)
@@ -157,7 +164,7 @@ export function PieChart(props: VisualizationProps) {
);
const handleToggleSeriesVisibility = (
- event: MouseEvent,
+ _event: MouseEvent,
sliceIndex: number,
) => {
const slice = slices[sliceIndex];