Skip to content

Commit

Permalink
Fix tooltip bug in pie charts (metabase#10994)
Browse files Browse the repository at this point in the history
  • Loading branch information
paulrosenzweig authored Sep 27, 2019
1 parent 708d659 commit 619cef1
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,10 @@ export default class PieChart extends Component {
const [slices, others] = _.chain(rows)
.map((row, index) => ({
key: row[dimensionIndex],
// Value is used to determine arc size and is modified for very small
// other slices. We save displayValue for use in tooltips.
value: row[metricIndex],
displayValue: row[metricIndex],
percentage: row[metricIndex] / total,
color: settings["pie._colors"][row[dimensionIndex]],
}))
Expand Down Expand Up @@ -323,13 +326,13 @@ export default class PieChart extends Component {
const slice = slices[index];
if (!slice || slice.noHover) {
return null;
} else if (slice === otherSlice) {
} else if (slice === otherSlice && others.length > 1) {
return {
index,
event: event && event.nativeEvent,
data: others.map(o => ({
key: formatDimension(o.key, false),
value: formatMetric(o.value, false),
value: formatMetric(o.displayValue, false),
})),
};
} else {
Expand All @@ -343,7 +346,7 @@ export default class PieChart extends Component {
},
{
key: getFriendlyName(cols[metricIndex]),
value: formatMetric(slice.value),
value: formatMetric(slice.displayValue),
},
].concat(
showPercentInTooltip && slice.percentage != null
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
jest.mock("metabase/components/ExplicitSize");

import React from "react";
import { render, cleanup } from "@testing-library/react";
import { render, cleanup, fireEvent } from "@testing-library/react";

import { NumberColumn, StringColumn } from "../__support__/visualizations";

Expand Down Expand Up @@ -40,4 +40,34 @@ describe("pie chart", () => {
getAllByText("49%");
getAllByText("1%");
});

it("should show a condensed tooltip for squashed slices", () => {
const rows = [["foo", 0.5], ["bar", 0.49], ["baz", 0.002], ["qux", 0.008]];
const { container, getAllByText, queryAllByText } = render(
<Visualization rawSeries={series(rows)} />,
);
const paths = container.querySelectorAll("path");
const otherPath = paths[paths.length - 1];

// condensed tooltips display as "dimension: metric"
expect(queryAllByText("baz:").length).toBe(0);
expect(queryAllByText("qux:").length).toBe(0);
fireEvent.mouseMove(otherPath);
// these appear twice in the dom due to some popover weirdness
expect(getAllByText("baz:").length).toBe(2);
expect(getAllByText("qux:").length).toBe(2);
});

it("shouldn't show a condensed tooltip for just one squashed slice", () => {
const rows = [["foo", 0.5], ["bar", 0.49], ["baz", 0.002]];
const { container, queryAllByText } = render(
<Visualization rawSeries={series(rows)} />,
);
const paths = container.querySelectorAll("path");
const otherPath = paths[paths.length - 1];

fireEvent.mouseMove(otherPath);
// normal tooltips don't use this "dimension: metric" format
expect(queryAllByText("baz:").length).toBe(0);
});
});

0 comments on commit 619cef1

Please sign in to comment.