From bcb7d3e32e6a129554ad05a032af3cec7ff827fa Mon Sep 17 00:00:00 2001
From: Emmad Usmani <emmadusmani@berkeley.edu>
Date: Thu, 23 Mar 2023 11:42:10 -0700
Subject: [PATCH] fix pie chart legend not fading on hover (#29415)
* fix pie chart legend not fading on hover
* use `aria-current` in test instead of directly checking style
* fix failing test
---
.../visualizations/pie_chart.cy.spec.js | 21 ++++++
.../components/LegendHorizontal.jsx | 52 +++++++------
.../visualizations/components/LegendItem.jsx | 7 +-
.../components/LegendVertical.jsx | 73 ++++++++++---------
4 files changed, 94 insertions(+), 59 deletions(-)
diff --git a/e2e/test/scenarios/visualizations/pie_chart.cy.spec.js b/e2e/test/scenarios/visualizations/pie_chart.cy.spec.js
index fb51a34b02c..c5ec0ebd8f9 100644
--- a/e2e/test/scenarios/visualizations/pie_chart.cy.spec.js
+++ b/e2e/test/scenarios/visualizations/pie_chart.cy.spec.js
@@ -29,6 +29,21 @@ describe("scenarios > visualizations > pie chart", () => {
ensurePieChartRendered(["Doohickey", "Gadget", "Gizmo", "Widget"], 200);
});
+
+ it("should mute items in legend when hovering (metabase#29224)", () => {
+ visitQuestionAdhoc({
+ dataset_query: testQuery,
+ display: "pie",
+ });
+
+ cy.findByTestId("chart-legend").findByText("Doohickey").realHover();
+ [
+ ["Doohickey", "true"],
+ ["Gadget", "false"],
+ ["Gizmo", "false"],
+ ["Widget", "false"],
+ ].map(args => checkLegendItemAriaCurrent(args[0], args[1]));
+ });
});
function ensurePieChartRendered(rows, totalValue) {
@@ -46,3 +61,9 @@ function ensurePieChartRendered(rows, totalValue) {
});
});
}
+
+function checkLegendItemAriaCurrent(title, value) {
+ cy.findByTestId("chart-legend")
+ .findByTestId(`legend-item-${title}`)
+ .should("have.attr", "aria-current", value);
+}
diff --git a/frontend/src/metabase/visualizations/components/LegendHorizontal.jsx b/frontend/src/metabase/visualizations/components/LegendHorizontal.jsx
index 8041bd76d43..f2d411801de 100644
--- a/frontend/src/metabase/visualizations/components/LegendHorizontal.jsx
+++ b/frontend/src/metabase/visualizations/components/LegendHorizontal.jsx
@@ -12,29 +12,35 @@ export default class LegendHorizontal extends Component {
const { className, titles, colors, hovered, onHoverChange } = this.props;
return (
<ol className={cx(className, styles.Legend, styles.horizontal)}>
- {titles.map((title, index) => (
- <li key={index}>
- <LegendItem
- ref={this["legendItem" + index]}
- title={title}
- color={colors[index % colors.length]}
- isMuted={
- hovered && hovered.index != null && index !== hovered.index
- }
- showTooltip={false}
- onMouseEnter={() =>
- onHoverChange &&
- onHoverChange({
- index,
- element: ReactDOM.findDOMNode(
- this.refs["legendItem" + index],
- ),
- })
- }
- onMouseLeave={() => onHoverChange && onHoverChange(null)}
- />
- </li>
- ))}
+ {titles.map((title, index) => {
+ const isMuted =
+ hovered && hovered.index != null && index !== hovered.index;
+ return (
+ <li
+ key={index}
+ data-testid={`legend-item-${title}`}
+ {...(hovered && { "aria-current": !isMuted })}
+ >
+ <LegendItem
+ ref={this["legendItem" + index]}
+ title={title}
+ color={colors[index % colors.length]}
+ isMuted={isMuted}
+ showTooltip={false}
+ onMouseEnter={() =>
+ onHoverChange &&
+ onHoverChange({
+ index,
+ element: ReactDOM.findDOMNode(
+ this.refs["legendItem" + index],
+ ),
+ })
+ }
+ onMouseLeave={() => onHoverChange && onHoverChange(null)}
+ />
+ </li>
+ );
+ })}
</ol>
);
}
diff --git a/frontend/src/metabase/visualizations/components/LegendItem.jsx b/frontend/src/metabase/visualizations/components/LegendItem.jsx
index e66334c4dfc..f8a5bae36eb 100644
--- a/frontend/src/metabase/visualizations/components/LegendItem.jsx
+++ b/frontend/src/metabase/visualizations/components/LegendItem.jsx
@@ -53,11 +53,14 @@ export default class LegendItem extends Component {
"no-decoration flex align-center fullscreen-normal-text fullscreen-night-text",
{
mr1: showTitle,
- muted: isMuted,
"cursor-pointer": onClick,
},
)}
- style={{ overflowX: "hidden", flex: "0 1 auto" }}
+ style={{
+ overflowX: "hidden",
+ flex: "0 1 auto",
+ opacity: isMuted ? 0.4 : 1,
+ }}
onMouseEnter={onMouseEnter}
onMouseLeave={onMouseLeave}
onClick={onClick}
diff --git a/frontend/src/metabase/visualizations/components/LegendVertical.jsx b/frontend/src/metabase/visualizations/components/LegendVertical.jsx
index a2ddf3cb129..a87e5bbfa65 100644
--- a/frontend/src/metabase/visualizations/components/LegendVertical.jsx
+++ b/frontend/src/metabase/visualizations/components/LegendVertical.jsx
@@ -65,41 +65,46 @@ export default class LegendVertical extends Component {
}
return (
<ol className={cx(className, styles.Legend, styles.vertical)}>
- {items.map((title, index) => (
- <li
- key={index}
- ref={"item" + index}
- className="flex flex-no-shrink"
- onMouseEnter={e =>
- onHoverChange &&
- onHoverChange({
- index,
- element: ReactDOM.findDOMNode(this.refs["legendItem" + index]),
- })
- }
- onMouseLeave={e => onHoverChange && onHoverChange()}
- >
- <LegendItem
- ref={"legendItem" + index}
- title={Array.isArray(title) ? title[0] : title}
- color={colors[index % colors.length]}
- isMuted={
- hovered && hovered.index != null && index !== hovered.index
+ {items.map((title, index) => {
+ const isMuted =
+ hovered && hovered.index != null && index !== hovered.index;
+ const legendItemTitle = Array.isArray(title) ? title[0] : title;
+ return (
+ <li
+ key={index}
+ ref={"item" + index}
+ className="flex flex-no-shrink"
+ onMouseEnter={e =>
+ onHoverChange &&
+ onHoverChange({
+ index,
+ element: ReactDOM.findDOMNode(
+ this.refs["legendItem" + index],
+ ),
+ })
}
- showTooltip={false}
- />
- {Array.isArray(title) && (
- <span
- className={cx("LegendItem", "flex-align-right pl1", {
- muted:
- hovered && hovered.index != null && index !== hovered.index,
- })}
- >
- {title[1]}
- </span>
- )}
- </li>
- ))}
+ onMouseLeave={e => onHoverChange && onHoverChange()}
+ data-testid={`legend-item-${legendItemTitle}`}
+ {...(hovered && { "aria-current": !isMuted })}
+ >
+ <LegendItem
+ ref={"legendItem" + index}
+ title={legendItemTitle}
+ color={colors[index % colors.length]}
+ isMuted={isMuted}
+ showTooltip={false}
+ />
+ {Array.isArray(title) && (
+ <span
+ className={cx("LegendItem", "flex-align-right pl1")}
+ style={{ opacity: isMuted ? 0.4 : 1 }}
+ >
+ {title[1]}
+ </span>
+ )}
+ </li>
+ );
+ })}
{overflowCount > 0 ? (
<li key="extra" className="flex flex-no-shrink">
<Tooltip
--
GitLab