refactor legend components to use items prop (#42417)

* refactor legend components to use items prop * update RowChart * add types * add key to type * fix type errors * fix legend.unit.spec.ts
metabase · May 14, 2024 · 0902226 · 0902226
1 parent 2b91354
commit 0902226
Show file tree

Hide file tree

Showing 12 changed files with 50 additions and 59 deletions.
diff --git a/frontend/src/metabase/static-viz/components/Legend/types.ts b/frontend/src/metabase/static-viz/components/Legend/types.ts
@@ -1,7 +1,4 @@
-export type LegendItem = {
-  name: string;
-  color: string;
-};
+import type { LegendItem } from "metabase/visualizations/echarts/cartesian/model/types";
 
 export type PositionedLegendItem = LegendItem & {
   left: number;

diff --git a/frontend/src/metabase/static-viz/components/Legend/utils.ts b/frontend/src/metabase/static-viz/components/Legend/utils.ts
@@ -1,4 +1,5 @@
 import { measureTextWidth, truncateText } from "metabase/static-viz/lib/text";
+import type { LegendItem } from "metabase/visualizations/echarts/cartesian/model/types";
 
 import {
   DEFAULT_LEGEND_FONT_SIZE,
@@ -8,7 +9,7 @@ import {
   LEGEND_ITEM_MARGIN_RIGHT,
   DEFAULT_LEGEND_LINE_HEIGHT,
 } from "./constants";
-import type { LegendItem, PositionedLegendItem } from "./types";
+import type { PositionedLegendItem } from "./types";
 
 const calculateItemWidth = (
   item: LegendItem,
@@ -89,6 +90,7 @@ export const calculateLegendRows = ({
       currentRowX = horizontalPadding + itemWidth + LEGEND_ITEM_MARGIN_RIGHT;
     } else {
       currentRow.push({
+        key: item.key,
         color: item.color,
         name: truncateText(
           item.name,

diff --git a/frontend/src/metabase/static-viz/components/RowChart/RowChart.tsx b/frontend/src/metabase/static-viz/components/RowChart/RowChart.tsx
@@ -92,6 +92,7 @@ const StaticRowChart = ({ data, settings, getColor }: StaticRowChartProps) => {
 
   const legend = calculateLegendRows({
     items: series.map(series => ({
+      key: series.seriesKey,
       name: series.seriesName,
       color: seriesColors[series.seriesKey],
     })),

diff --git a/frontend/src/metabase/visualizations/components/legend/Legend.jsx b/frontend/src/metabase/visualizations/components/legend/Legend.jsx
@@ -19,8 +19,7 @@ const POPOVER_OFFSET = POPOVER_BORDER + POPOVER_PADDING;
 
 const propTypes = {
   className: PropTypes.string,
-  labels: PropTypes.array.isRequired,
-  colors: PropTypes.array.isRequired,
+  items: PropTypes.array.isRequired,
   hovered: PropTypes.object,
   visibleIndex: PropTypes.number,
   visibleLength: PropTypes.number,
@@ -36,11 +35,10 @@ const alwaysTrue = () => true;
 
 const Legend = ({
   className,
-  labels: originalLabels,
-  colors: originalColors,
+  items: originalItems,
   hovered,
   visibleIndex = 0,
-  visibleLength = originalLabels.length,
+  visibleLength = originalItems.length,
   isVertical,
   onHoverChange,
   onSelectSeries,
@@ -62,35 +60,29 @@ const Legend = ({
     setMaxWidth(0);
   }, []);
 
-  const labels = isReversed
-    ? _.clone(originalLabels).reverse()
-    : originalLabels;
-  const colors = isReversed
-    ? _.clone(originalColors).reverse()
-    : originalColors;
+  const items = isReversed ? _.clone(originalItems).reverse() : originalItems;
 
   const overflowIndex = visibleIndex + visibleLength;
-  const visibleLabels = labels.slice(visibleIndex, overflowIndex);
-  const overflowLength = labels.length - overflowIndex;
+  const visibleItems = items.slice(visibleIndex, overflowIndex);
+  const overflowLength = items.length - overflowIndex;
 
   return (
     <LegendRoot
       className={className}
       aria-label={t`Legend`}
       isVertical={isVertical}
     >
-      {visibleLabels.map((label, index) => {
+      {visibleItems.map((item, index) => {
         const localIndex = index + visibleIndex;
         const itemIndex = isReversed
-          ? labels.length - 1 - localIndex
+          ? items.length - 1 - localIndex
           : localIndex;
 
         return (
           <LegendItem
-            key={itemIndex}
-            label={label}
+            key={item.key}
+            item={item}
             index={itemIndex}
-            color={colors[localIndex % colors.length]}
             isMuted={hovered && itemIndex !== hovered.index}
             isVertical={isVertical}
             isReversed={isReversed}
@@ -120,8 +112,7 @@ const Legend = ({
         >
           <LegendPopoverContainer style={{ maxWidth }}>
             <Legend
-              labels={originalLabels}
-              colors={originalColors}
+              items={originalItems}
               hovered={hovered}
               visibleIndex={overflowIndex}
               visibleLength={overflowLength}

diff --git a/frontend/src/metabase/visualizations/components/legend/LegendItem.jsx b/frontend/src/metabase/visualizations/components/legend/LegendItem.jsx
@@ -15,9 +15,8 @@ import {
 } from "./LegendItem.styled";
 
 const propTypes = {
-  label: PropTypes.string,
+  item: PropTypes.object,
   index: PropTypes.number,
-  color: PropTypes.string,
   isMuted: PropTypes.bool,
   isVertical: PropTypes.bool,
   isReversed: PropTypes.bool,
@@ -27,9 +26,8 @@ const propTypes = {
 };
 
 const LegendItem = ({
-  label,
+  item,
   index,
-  color,
   isMuted,
   isVertical,
   isReversed,
@@ -61,15 +59,15 @@ const LegendItem = ({
         onMouseEnter={onHoverChange && handleItemMouseEnter}
         onMouseLeave={onHoverChange && handleItemMouseLeave}
       >
-        <LegendItemDot color={color} />
+        <LegendItemDot color={item.color} />
         <LegendItemTitle
           className={cx(
             DashboardS.fullscreenNormalText,
             DashboardS.fullscreenNightText,
             EmbedFrameS.fullscreenNightText,
           )}
         >
-          <Ellipsified>{label}</Ellipsified>
+          <Ellipsified>{item.name}</Ellipsified>
         </LegendItemTitle>
       </LegendItemLabel>
       {onRemoveSeries && <LegendItemRemoveIcon onClick={handleRemoveClick} />}

diff --git a/frontend/src/metabase/visualizations/components/legend/LegendLayout.jsx b/frontend/src/metabase/visualizations/components/legend/LegendLayout.jsx
@@ -19,8 +19,7 @@ const MIN_LEGEND_WIDTH = 400;
 
 const propTypes = {
   className: PropTypes.string,
-  labels: PropTypes.array.isRequired,
-  colors: PropTypes.array.isRequired,
+  items: PropTypes.array.isRequired,
   hovered: PropTypes.object,
   width: PropTypes.number,
   height: PropTypes.number,
@@ -38,8 +37,7 @@ const propTypes = {
 
 const LegendLayout = ({
   className,
-  labels,
-  colors,
+  items,
   hovered,
   width = 0,
   height = 0,
@@ -59,12 +57,12 @@ const LegendLayout = ({
   const maxXItems = Math.floor(width / MIN_ITEM_WIDTH);
   const maxYItems = Math.floor(height / itemHeight);
   const maxYLabels = Math.max(maxYItems - 1, 0);
-  const minYLabels = labels.length > maxYItems ? maxYLabels : labels.length;
+  const minYLabels = items.length > maxYItems ? maxYLabels : items.length;
 
   const isNarrow = width < MIN_LEGEND_WIDTH;
-  const isVertical = maxXItems < labels.length;
+  const isVertical = maxXItems < items.length;
   const isVisible = hasLegend && !(isVertical && isNarrow);
-  const visibleLength = isVertical ? minYLabels : labels.length;
+  const visibleLength = isVertical ? minYLabels : items.length;
 
   return (
     <LegendLayoutRoot className={className} isVertical={isVertical}>
@@ -74,8 +72,7 @@ const LegendLayout = ({
           isQueryBuilder={isQueryBuilder}
         >
           <Legend
-            labels={labels}
-            colors={colors}
+            items={items}
             hovered={hovered}
             visibleLength={visibleLength}
             isVertical={isVertical}

diff --git a/frontend/src/metabase/visualizations/echarts/cartesian/model/legend.ts b/frontend/src/metabase/visualizations/echarts/cartesian/model/legend.ts
@@ -1,10 +1,10 @@
 import { isBreakoutSeries } from "./guards";
-import type { SeriesModel } from "./types";
+import type { LegendItem, SeriesModel } from "./types";
 
 export const getLegendItems = (
   seriesModels: SeriesModel[],
   showAllLegendItems: boolean = false,
-) => {
+): LegendItem[] => {
   if (
     seriesModels.length === 1 &&
     !isBreakoutSeries(seriesModels[0]) &&
@@ -14,6 +14,7 @@ export const getLegendItems = (
   }
 
   return seriesModels.map(seriesModel => ({
+    key: seriesModel.dataKey,
     name: seriesModel.name,
     color: seriesModel.color,
   }));

diff --git a/frontend/src/metabase/visualizations/echarts/cartesian/model/legend.unit.spec.ts b/frontend/src/metabase/visualizations/echarts/cartesian/model/legend.unit.spec.ts
@@ -17,15 +17,17 @@ describe("getLegendItems", () => {
       createMockSeriesModel({ name: "Series 2", color: "blue" }),
     ]);
     expect(legendItems).toStrictEqual([
-      { name: "Series 1", color: "red" },
-      { name: "Series 2", color: "blue" },
+      { name: "Series 1", color: "red", key: "dataKey" },
+      { name: "Series 2", color: "blue", key: "dataKey" },
     ]);
   });
 
   it("should return legend items when there is a single breakout series", () => {
     const legendItems = getLegendItems([
       createMockBreakoutSeriesModel({ name: "breakout series" }),
     ]);
-    expect(legendItems).toEqual([{ name: "breakout series", color: "red" }]);
+    expect(legendItems).toEqual([
+      { name: "breakout series", color: "red", key: "dataKey" },
+    ]);
   });
 });
diff --git a/frontend/src/metabase/visualizations/echarts/cartesian/model/types.ts b/frontend/src/metabase/visualizations/echarts/cartesian/model/types.ts
@@ -205,3 +205,9 @@ export type CartesianChartModel = BaseCartesianChartModel & {
 };
 
 export type ShowWarning = (warning: string) => void;
+
+export type LegendItem = {
+  key: string;
+  name: string;
+  color: string;
+};
diff --git a/frontend/src/metabase/visualizations/visualizations/CartesianChart/CartesianChart.tsx b/frontend/src/metabase/visualizations/visualizations/CartesianChart/CartesianChart.tsx
@@ -106,8 +106,7 @@ function _CartesianChart(props: VisualizationProps) {
       <CartesianChartLegendLayout
         isReversed={settings["legend.is_reversed"]}
         hasLegend={hasLegend}
-        labels={legendItems.map(item => item.name)}
-        colors={legendItems.map(item => item.color)}
+        items={legendItems}
         actionButtons={!hasTitle ? actionButtons : undefined}
         hovered={hovered}
         isFullscreen={isFullscreen}

diff --git a/frontend/src/metabase/visualizations/visualizations/RowChart/RowChart.tsx b/frontend/src/metabase/visualizations/visualizations/RowChart/RowChart.tsx
@@ -234,7 +234,7 @@ const RowChartVisualization = ({
   const description = settings["card.description"];
   const canSelectTitle = !!onChangeCardAndRun;
 
-  const { labels, colors } = useMemo(
+  const legendItems = useMemo(
     () => getLegendItems(series, seriesColors, settings),
     [series, seriesColors, settings],
   );
@@ -279,8 +279,7 @@ const RowChartVisualization = ({
       )}
       <RowChartLegendLayout
         hasLegend={hasLegend}
-        labels={labels}
-        colors={colors}
+        items={legendItems}
         actionButtons={!hasTitle ? actionButtons : undefined}
         hovered={hovered}
         onHoverChange={onHoverChange}

diff --git a/frontend/src/metabase/visualizations/visualizations/RowChart/utils/legend.ts b/frontend/src/metabase/visualizations/visualizations/RowChart/utils/legend.ts
@@ -1,18 +1,16 @@
+import type { LegendItem } from "metabase/visualizations/echarts/cartesian/model/types";
 import type { Series } from "metabase/visualizations/shared/components/RowChart/types";
 import type { VisualizationSettings } from "metabase-types/api";
 
 export const getLegendItems = <TDatum>(
   multipleSeries: Series<TDatum, unknown>[],
   seriesColors: Record<string, string>,
   settings: VisualizationSettings,
-) => {
-  const orderedTitles = multipleSeries.map(
-    series =>
+): LegendItem[] => {
+  return multipleSeries.map(series => ({
+    key: series.seriesKey,
+    name:
       settings?.series_settings?.[series.seriesKey]?.title ?? series.seriesName,
-  );
-
-  return {
-    labels: orderedTitles,
-    colors: multipleSeries.map(single => seriesColors[single.seriesKey]),
-  };
+    color: seriesColors[series.seriesKey],
+  }));
 };