Skip to content
Snippets Groups Projects
Unverified Commit 6072c971 authored by metabase-bot[bot]'s avatar metabase-bot[bot] Committed by GitHub
Browse files

9432 toggle pie chart total (#29127) (#29166)


* Adding Show Total toggle, adding unit tests

* adjusting unit test, fixing logic

* Adding Static Viz test. PR Feedback

Co-authored-by: default avatarNick Fitzpatrick <nick@metabase.com>
parent 2f59ea82
No related branches found
No related tags found
No related merge requests found
......@@ -25,6 +25,8 @@ describe("static visualizations", () => {
createPieQuestion({ percentVisibility: "off " }),
createPieQuestion({ percentVisibility: "legend" }),
createPieQuestion({ percentVisibility: "inside" }),
createPieQuestion({ showTotal: true }),
createPieQuestion({ showTotal: false }),
],
}).then(({ dashboard }) => {
visitDashboard(dashboard.id);
......@@ -38,9 +40,9 @@ describe("static visualizations", () => {
});
});
function createPieQuestion({ percentVisibility }) {
function createPieQuestion({ percentVisibility, showTotal }) {
const query = {
name: `pie showDataLabels=${percentVisibility}`,
name: `pie showDataLabels=${percentVisibility}, showTotal=${showTotal}`,
native: {
query:
"select 1 x, 1000 y\n" +
......@@ -55,6 +57,7 @@ function createPieQuestion({ percentVisibility }) {
},
visualization_settings: {
"pie.percent_visibility": percentVisibility,
"pie.show_total": showTotal,
},
display: "pie",
database: SAMPLE_DB_ID,
......
......@@ -18,6 +18,7 @@ const propTypes = {
settings: PropTypes.shape({
metric: PropTypes.object,
percent_visibility: PropTypes.oneOf(["off", "legend", "inside"]),
show_total: PropTypes.bool,
}),
};
......@@ -60,6 +61,7 @@ const CategoricalDonutChart = ({
const totalLabel = t`Total`.toUpperCase();
const shouldShowLabels = settings?.percent_visibility === "inside";
const shouldShowTotal = settings?.show_total ?? true;
return (
<svg width={layout.width} height={layout.height}>
......@@ -105,26 +107,31 @@ const CategoricalDonutChart = ({
})
}
</Pie>
<Group fontFamily={layout.font.family} fontWeight={layout.font.weight}>
<Text
y={-textCenter}
fill={layout.colors.textDark}
fontSize={layout.valueFontSize}
textAnchor="middle"
dominantBaseline="middle"
{shouldShowTotal && (
<Group
fontFamily={layout.font.family}
fontWeight={layout.font.weight}
>
{formatNumber(totalValue, settings?.metric)}
</Text>
<Text
y={textCenter}
fill={layout.colors.textLight}
fontSize={layout.labelFontSize}
textAnchor="middle"
dominantBaseline="middle"
>
{totalLabel}
</Text>
</Group>
<Text
y={-textCenter}
fill={layout.colors.textDark}
fontSize={layout.valueFontSize}
textAnchor="middle"
dominantBaseline="middle"
>
{formatNumber(totalValue, settings?.metric)}
</Text>
<Text
y={textCenter}
fill={layout.colors.textLight}
fontSize={layout.labelFontSize}
textAnchor="middle"
dominantBaseline="middle"
>
{totalLabel}
</Text>
</Group>
)}
</Group>
</svg>
);
......
......@@ -15,7 +15,7 @@ type Props = {
hint?: string;
hidden?: boolean;
disabled?: boolean;
widget?: React.ComponentType;
widget?: React.ComponentType<{ id: string }>;
inline?: boolean;
marginBottom?: string;
props?: Record<string, unknown>;
......@@ -23,6 +23,7 @@ type Props = {
variant?: "default" | "form-field";
borderBottom?: boolean;
dataTestId?: string;
id: string;
};
const ChartSettingsWidget = ({
......@@ -58,7 +59,11 @@ const ChartSettingsWidget = ({
borderBottom={borderBottom}
>
{title && (
<Title variant={variant} className={cx({ "Form-label": isFormField })}>
<Title
variant={variant}
className={cx({ "Form-label": isFormField })}
id={extraWidgetProps.id}
>
{title}
{hint && (
<InfoIconContainer>
......
......@@ -120,7 +120,11 @@ class ChartWithLegend extends Component {
paddingRight: PADDING,
}}
>
{legend && <div className={cx(styles.LegendWrapper)}>{legend}</div>}
{legend && (
<div className={cx(styles.LegendWrapper)} data-testid="chart-legend">
{legend}
</div>
)}
<div
className={cx(styles.Chart)}
style={{ width: chartWidth, height: chartHeight }}
......
......@@ -3,8 +3,8 @@ import React from "react";
import Toggle from "metabase/core/components/Toggle";
const ChartSettingToggle = ({ value, onChange }) => (
<Toggle value={value} onChange={onChange} />
const ChartSettingToggle = ({ value, onChange, id }) => (
<Toggle value={value} onChange={onChange} aria-labelledby={id} />
);
export default ChartSettingToggle;
......@@ -121,6 +121,14 @@ export default class PieChart extends Component {
widget: "toggle",
default: true,
inline: true,
marginBottom: "1rem",
},
"pie.show_total": {
section: t`Display`,
title: t`Show total`,
widget: "toggle",
default: true,
inline: true,
},
"pie.percent_visibility": {
section: t`Display`,
......@@ -258,7 +266,16 @@ export default class PieChart extends Component {
requestAnimationFrame(() => {
const groupElement = this.chartGroup.current;
const detailElement = this.chartDetail.current;
if (groupElement.getBoundingClientRect().width < 120) {
const { settings } = this.props;
if (!groupElement || !detailElement) {
return;
}
if (
groupElement.getBoundingClientRect().width < 120 ||
!settings["pie.show_total"]
) {
detailElement.classList.add("hide");
} else {
detailElement.classList.remove("hide");
......
import React, { useState } from "react";
import { thaw } from "icepick";
import userEvent from "@testing-library/user-event";
import { renderWithProviders, screen, waitFor } from "__support__/ui";
import {
SAMPLE_DATABASE,
ORDERS,
metadata,
} from "__support__/sample_database_fixture";
import ChartSettings from "metabase/visualizations/components/ChartSettings";
import Question from "metabase-lib/Question";
const setup = () => {
const Container = () => {
const [question, setQuestion] = useState(
new Question(
{
display: "pie",
dataset_query: {
type: "query",
query: {
"source-table": ORDERS.id,
aggregation: [["count"]],
breakout: [
["field", ORDERS.CREATED_AT.id, { "temporal-unit": "year" }],
],
},
},
database: SAMPLE_DATABASE.id,
visualization_settings: {},
},
metadata,
),
);
const onChange = update => {
setQuestion(q => {
const newQuestion = q.updateSettings(update);
return new Question(thaw(newQuestion.card()), metadata);
});
};
return (
<ChartSettings
onChange={onChange}
series={[
{
card: question.card(),
data: {
rows: [
["2016-01-01T00:00:00-04:00", 500],
["2017-01-01T00:00:00-04:00", 1500],
],
cols: question.query().columns(),
},
},
]}
initial={{ section: "Data" }}
question={question}
/>
);
};
renderWithProviders(<Container />);
};
describe("table settings", () => {
beforeAll(() => {
//Append mocked style for .hide class
const mockedStyle = document.createElement("style");
mockedStyle.innerHTML = `.hide {display: none;}`;
document.body.append(mockedStyle);
});
it("should render", () => {
setup();
expect(screen.getByTestId("pie-chart")).toBeInTheDocument();
});
it("should allow you to show and hide the grand total", async () => {
setup();
jest
.spyOn(Element.prototype, "getBoundingClientRect")
.mockImplementation(() => ({
height: 200,
width: 200,
x: 0,
y: 0,
}));
expect(screen.getByTestId("detail-value")).toBeVisible();
expect(screen.getByTestId("detail-value")).toHaveTextContent("2,000");
userEvent.click(screen.getByText("Display"));
userEvent.click(screen.getByLabelText("Show total"));
await waitFor(() => {
expect(screen.getByTestId("detail-value")).not.toBeVisible();
});
userEvent.click(screen.getByLabelText("Show total"));
await waitFor(() => {
expect(screen.getByTestId("detail-value")).toBeVisible();
});
jest.restoreAllMocks();
});
it("should allow you to show and hide the legend", async () => {
setup();
expect(screen.getByTestId("chart-legend")).toBeVisible();
expect(screen.getByTestId("chart-legend")).toHaveTextContent("2016");
expect(screen.getByTestId("chart-legend")).toHaveTextContent("2017");
userEvent.click(screen.getByText("Display"));
userEvent.click(screen.getByLabelText("Show legend"));
await waitFor(() => {
expect(screen.queryByTestId("chart-legend")).not.toBeInTheDocument();
});
userEvent.click(screen.getByLabelText("Show legend"));
await waitFor(() => {
expect(screen.getByTestId("chart-legend")).toBeVisible();
});
});
it("should allow you to disable showing percentages", async () => {
setup();
expect(screen.getByTestId("chart-legend")).toBeVisible();
expect(screen.getByTestId("chart-legend")).toHaveTextContent("25%");
expect(screen.getByTestId("chart-legend")).toHaveTextContent("75%");
userEvent.click(screen.getByText("Display"));
userEvent.click(screen.getByLabelText("Off"));
await waitFor(() => {
expect(screen.getByTestId("chart-legend")).not.toHaveTextContent("25%");
});
expect(screen.getByTestId("chart-legend")).not.toHaveTextContent("75%");
});
it("should allow you to show percentages in the chart", async () => {
setup();
expect(screen.getByTestId("chart-legend")).toBeVisible();
expect(screen.getByTestId("chart-legend")).toHaveTextContent("25%");
expect(screen.getByTestId("chart-legend")).toHaveTextContent("75%");
expect(screen.getByTestId("pie-chart")).not.toHaveTextContent("25%");
expect(screen.getByTestId("pie-chart")).not.toHaveTextContent("75%");
userEvent.click(screen.getByText("Display"));
userEvent.click(screen.getByLabelText("On the chart"));
await waitFor(() => {
expect(screen.getByTestId("chart-legend")).not.toHaveTextContent("25%");
});
expect(screen.getByTestId("chart-legend")).toBeVisible();
expect(screen.getByTestId("chart-legend")).not.toHaveTextContent("75%");
expect(screen.getByTestId("pie-chart")).toHaveTextContent("25%");
expect(screen.getByTestId("pie-chart")).toHaveTextContent("75%");
});
});
......@@ -463,7 +463,7 @@
{:keys [rows percentages]} (donut-info slice-threshold rows)
legend-colors (merge (zipmap (map first rows) (cycle colors))
(update-keys (:pie.colors viz-settings) name))
settings {:percent_visibility (:pie.percent_visibility viz-settings)}
settings {:percent_visibility (:pie.percent_visibility viz-settings) :show_total (:pie.show_total viz-settings)}
image-bundle (image-bundle/make-image-bundle
render-type
(js-svg/categorical-donut rows legend-colors settings))
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment