Adding Color Picker for Metrics (#26486)

* Adding Color Picker for Metrics

* Adding support for breakout charts

* Addressing some PR Feedback

* Adding pillSize to color picker components

* Moving updateSeriesColor logic to visualizations/lib

* Typing viz/lib/series

* Final PR Feedback adjustments

frontend/src/metabase/core/components/ColorPill/ColorPill.styled.tsx

 import styled from "@emotion/styled";
+import { css } from "@emotion/react";
+
 import { color } from "metabase/lib/colors";
 
+import { PillSize } from "./types";
+
 export interface ColorPillRootProps {
   isAuto: boolean;
   isSelected: boolean;
+  pillSize: PillSize;
 }
 
 export const ColorPillRoot = styled.div<ColorPillRootProps>`
@@ -21,6 +26,17 @@ export const ColorPillRoot = styled.div<ColorPillRootProps>`
     border-color: ${props =>
       props.isSelected ? color("text-dark") : color("text-light")};
   }
+
+  ${props =>
+    props.pillSize === "small" &&
+    css`
+      padding: 1px;
+
+      ${ColorPillContent} {
+        height: 0.875rem;
+        width: 0.875rem;
+      }
+    `};
 `;
 
 export const ColorPillContent = styled.div`

frontend/src/metabase/core/components/ColorPill/ColorPill.tsx

@@ -6,6 +6,7 @@ import React, {
   useCallback,
 } from "react";
 import { ColorPillContent, ColorPillRoot } from "./ColorPill.styled";
+import { PillSize } from "./types";
 
 export type ColorPillAttributes = Omit<
   HTMLAttributes<HTMLDivElement>,
@@ -17,6 +18,7 @@ export interface ColorPillProps extends ColorPillAttributes {
   isAuto?: boolean;
   isSelected?: boolean;
   onSelect?: (newColor: string) => void;
+  pillSize?: PillSize;
 }
 
 const ColorPill = forwardRef(function ColorPill(
@@ -25,6 +27,7 @@ const ColorPill = forwardRef(function ColorPill(
     isAuto = false,
     isSelected = true,
     "aria-label": ariaLabel = color,
+    pillSize = "medium",
     onClick,
     onSelect,
     ...props
@@ -47,10 +50,14 @@ const ColorPill = forwardRef(function ColorPill(
       isSelected={isSelected}
       aria-label={ariaLabel}
       onClick={handleClick}
+      pillSize={pillSize}
     >
       <ColorPillContent style={{ backgroundColor: color }} />
     </ColorPillRoot>
   );
 });
 
-export default ColorPill;
+export default Object.assign(ColorPill, {
+  Content: ColorPillContent,
+  Root: ColorPillRoot,
+});

frontend/src/metabase/core/components/ColorPill/types.ts

+export type PillSize = "small" | "medium";

frontend/src/metabase/visualizations/components/ChartSettings.jsx

@@ -11,6 +11,7 @@ import Radio from "metabase/core/components/Radio";
 import Visualization from "metabase/visualizations/components/Visualization";
 
 import { getSettingsWidgetsForSeries } from "metabase/visualizations/lib/settings/visualization";
+import { updateSeriesColor } from "metabase/visualizations/lib/series";
 import * as MetabaseAnalytics from "metabase/lib/analytics";
 import {
   getVisualizationTransformed,
@@ -112,6 +113,12 @@ class ChartSettings extends Component {
     this.props.onChange(updateSettings(this._getSettings(), changedSettings));
   };
 
+  handleChangeSeriesColor = (seriesKey, color) => {
+    this.props.onChange(
+      updateSeriesColor(this._getSettings(), seriesKey, color),
+    );
+  };
+
   handleDone = () => {
     this.props.onDone(this._getSettings());
     this.props.onClose();
@@ -302,6 +309,8 @@ class ChartSettings extends Component {
       onEndShowWidget: this.handleEndShowWidget,
       currentSectionHasColumnSettings,
       columnHasSettings: col => this.columnHasSettings(col),
+      onChangeSeriesColor: (seriesKey, color) =>
+        this.handleChangeSeriesColor(seriesKey, color),
     };
 
     const sectionPicker = (

frontend/src/metabase/visualizations/components/settings/ChartSettingColorPicker.jsx

 /* eslint-disable react/prop-types */
 import React from "react";
+import cx from "classnames";
 
 import { getAccentColors } from "metabase/lib/colors/groups";
 import ColorSelector from "metabase/core/components/ColorSelector";
 
 export default function ChartSettingColorPicker(props) {
-  const { value, onChange } = props;
+  const { value, onChange, className, pillSize } = props;
 
   return (
-    <div className="flex align-center mb1">
+    <div className={cx("flex align-center mb1", className)}>
       <ColorSelector
         value={value}
         colors={getAccentColors()}
         onChange={onChange}
+        pillSize={pillSize}
       />
       {props.title && <h4 className="ml1">{props.title}</h4>}
     </div>

frontend/src/metabase/visualizations/components/settings/ChartSettingFieldPicker.jsx

@@ -2,11 +2,13 @@
 import React from "react";
 import { t } from "ttag";
 import _ from "underscore";
+import { keyForSingleSeries } from "metabase/visualizations/lib/settings/series";
 import { getColumnKey } from "metabase-lib/queries/utils/get-column-key";
 import ChartSettingSelect from "./ChartSettingSelect";
 import {
   SettingsIcon,
   ChartSettingFieldPickerRoot,
+  FieldPickerColorPicker,
 } from "./ChartSettingFieldPicker.styled";
 
 const ChartSettingFieldPicker = ({
@@ -20,6 +22,10 @@ const ChartSettingFieldPicker = ({
   showColumnSetting,
   showDragHandle,
   columnHasSettings,
+  showColorPicker,
+  colors,
+  series,
+  onChangeSeriesColor,
 }) => {
   let columnKey;
   if (value && showColumnSetting && columns) {
@@ -28,6 +34,17 @@ const ChartSettingFieldPicker = ({
       columnKey = getColumnKey(column);
     }
   }
+
+  let seriesKey;
+  if (series && columnKey && showColorPicker) {
+    const seriesForColumn = series.find(single => {
+      const metricColumn = single.data.cols[1];
+      return getColumnKey(metricColumn) === columnKey;
+    });
+    if (seriesForColumn) {
+      seriesKey = keyForSingleSeries(seriesForColumn);
+    }
+  }
   return (
     <ChartSettingFieldPickerRoot
       className={className}
@@ -37,6 +54,15 @@ const ChartSettingFieldPicker = ({
       {showDragHandle && (
         <SettingsIcon name="grabber2" size={12} noPointer noMargin />
       )}
+      {showColorPicker && seriesKey && (
+        <FieldPickerColorPicker
+          pillSize="small"
+          value={colors[seriesKey]}
+          onChange={value => {
+            onChangeSeriesColor(seriesKey, value);
+          }}
+        />
+      )}
       <ChartSettingSelect
         value={value}
         options={options}

frontend/src/metabase/visualizations/components/settings/ChartSettingFieldPicker.styled.tsx

@@ -3,6 +3,8 @@ import { color } from "metabase/lib/colors";
 import Icon from "metabase/components/Icon";
 import SelectButton from "metabase/core/components/SelectButton";
 import Triggerable from "metabase/components/Triggerable";
+import ColorPill from "metabase/core/components/ColorPill";
+import ChartSettingColorPicker from "./ChartSettingColorPicker";
 
 interface ChartSettingFieldPickerRootProps {
   disabled: boolean;
@@ -65,3 +67,8 @@ export const SettingsIcon = styled(Icon)<SettingsIconProps>`
     color: ${color("brand")};
   }
 `;
+
+export const FieldPickerColorPicker = styled(ChartSettingColorPicker)`
+  margin-bottom: 0;
+  margin-left: 0.25rem;
+`;

frontend/src/metabase/visualizations/components/settings/ChartSettingOrderedItems.tsx

@@ -10,6 +10,7 @@ import ColumnItem from "./ColumnItem";
 
 interface SortableItem {
   enabled: boolean;
+  color?: string;
 }
 
 interface SortableColumnFunctions<T> {
@@ -19,6 +20,7 @@ interface SortableColumnFunctions<T> {
   onAdd?: (item: T) => void;
   onEnable?: (item: T) => void;
   getItemName: (item: T) => string;
+  onColorChange?: (item: T, color: string) => void;
 }
 
 interface SortableColumnProps<T> extends SortableColumnFunctions<T> {
@@ -35,6 +37,7 @@ const SortableColumn = SortableElement(function SortableColumn<
   onClick,
   onAdd,
   onEnable,
+  onColorChange,
 }: SortableColumnProps<T>) {
   return (
     <ColumnItem
@@ -48,6 +51,10 @@ const SortableColumn = SortableElement(function SortableColumn<
       onClick={onClick ? () => onClick(item) : null}
       onAdd={onAdd ? () => onAdd(item) : null}
       onEnable={onEnable && !item.enabled ? () => onEnable(item) : null}
+      onColorChange={
+        onColorChange ? (color: string) => onColorChange(item, color) : null
+      }
+      color={item.color}
       draggable
     />
   );
@@ -69,6 +76,7 @@ const SortableColumnList = SortableContainer(function SortableColumnList<
   onRemove,
   onEnable,
   onAdd,
+  onColorChange,
 }: SortableColumnListProps<T>) {
   return (
     <div>
@@ -82,6 +90,7 @@ const SortableColumnList = SortableContainer(function SortableColumnList<
           onRemove={onRemove}
           onEnable={onEnable}
           onAdd={onAdd}
+          onColorChange={onColorChange}
         />
       ))}
     </div>
@@ -110,6 +119,7 @@ export function ChartSettingOrderedItems<T extends SortableItem>({
   onClick,
   getItemName,
   items,
+  onColorChange,
 }: ChartSettingOrderedItemsProps<T>) {
   return (
     <SortableColumnList
@@ -122,6 +132,7 @@ export function ChartSettingOrderedItems<T extends SortableItem>({
       onEnable={onEnable}
       onClick={onClick}
       onSortEnd={onSortEnd}
+      onColorChange={onColorChange}
       distance={5}
     />
   );

frontend/src/metabase/visualizations/components/settings/ChartSettingOrderedSimple.tsx

@@ -14,6 +14,7 @@ interface SortableItem {
   enabled: boolean;
   originalIndex: number;
   name: string;
+  color?: string;
 }
 
 interface ChartSettingOrderedSimpleProps {
@@ -26,6 +27,7 @@ interface ChartSettingOrderedSimpleProps {
   ) => void;
   series: Series;
   hasEditSettings: boolean;
+  onChangeSeriesColor: (seriesKey: string, color: string) => void;
 }
 
 export const ChartSettingOrderedSimple = ({
@@ -35,6 +37,7 @@ export const ChartSettingOrderedSimple = ({
   series,
   onShowWidget,
   hasEditSettings = true,
+  onChangeSeriesColor,
 }: ChartSettingOrderedSimpleProps) => {
   const toggleDisplay = (selectedItem: SortableItem) => {
     const index = orderedItems.findIndex(
@@ -71,16 +74,26 @@ export const ChartSettingOrderedSimple = ({
     );
   };
 
+  const handleColorChange = (item: SortableItem, color: string) => {
+    const singleSeries = series[item.originalIndex];
+    const seriesKey = keyForSingleSeries(singleSeries);
+    onChangeSeriesColor(seriesKey, color);
+  };
+
   return (
     <ChartSettingOrderedSimpleRoot>
       {orderedItems.length > 0 ? (
         <ChartSettingOrderedItems
-          items={orderedItems}
+          items={orderedItems.map(item => ({
+            ...item,
+            color: items[item.originalIndex].color,
+          }))}
           getItemName={getItemTitle}
           onRemove={toggleDisplay}
           onEnable={toggleDisplay}
           onSortEnd={handleSortEnd}
           onEdit={hasEditSettings ? handleOnEdit : undefined}
+          onColorChange={handleColorChange}
           distance={5}
         />
       ) : (

frontend/src/metabase/visualizations/components/settings/ColumnItem.jsx

@@ -8,6 +8,7 @@ import {
   ColumnItemContainer,
   ColumnItemRoot,
   ColumnItemDragHandle,
+  ColumnItemColorPicker,
 } from "./ColumnItem.styled";
 
 const ActionIcon = ({ icon, onClick }) => (
@@ -22,11 +23,13 @@ const ActionIcon = ({ icon, onClick }) => (
 
 const ColumnItem = ({
   title,
+  color,
   onAdd,
   onRemove,
   onClick,
   onEdit,
   onEnable,
+  onColorChange,
   draggable,
   className = "",
 }) => {
@@ -39,6 +42,13 @@ const ColumnItem = ({
     >
       <ColumnItemContainer>
         {draggable && <ColumnItemDragHandle name="grabber2" size={12} />}
+        {onColorChange && color && (
+          <ColumnItemColorPicker
+            value={color}
+            onChange={onColorChange}
+            pillSize="small"
+          />
+        )}
         <ColumnItemContent>
           <ColumnItemSpan>{title}</ColumnItemSpan>
           {onEdit && <ActionIcon icon="ellipsis" onClick={onEdit} />}

frontend/src/metabase/visualizations/components/settings/ColumnItem.styled.tsx

@@ -2,6 +2,8 @@ import styled from "@emotion/styled";
 import { color } from "metabase/lib/colors";
 
 import Icon from "metabase/components/Icon";
+import ColorPill from "metabase/core/components/ColorPill";
+import ChartSettingColorPicker from "./ChartSettingColorPicker";
 
 interface ColumnItemRootProps {
   isDraggable: boolean;
@@ -74,3 +76,8 @@ export const ColumnItemIcon = styled(Icon)`
 export const ColumnItemDragHandle = styled(Icon)`
   color: ${color("text-medium")};
 `;
+
+export const ColumnItemColorPicker = styled(ChartSettingColorPicker)`
+  margin-bottom: 0;
+  margin-left: 0.25rem;
+`;

frontend/src/metabase/visualizations/lib/series.ts

+import { assocIn } from "icepick";
+import { VisualizationSettings } from "metabase-types/api/card";
+import { SETTING_ID } from "./settings/series";
+
+export const updateSeriesColor = (
+  settings: VisualizationSettings,
+  seriesKey: string,
+  color: string,
+) => {
+  return assocIn(settings, [SETTING_ID, seriesKey, "color"], color);
+};

frontend/src/metabase/visualizations/lib/settings.js

@@ -131,6 +131,7 @@ function getSettingWidget(
     onChangeSettings(newSettings);
   };
   if (settingDef.useRawSeries && object._raw) {
+    extra.transformedSeries = object;
     object = object._raw;
   }
   return {

frontend/src/metabase/visualizations/lib/settings/graph.js

@@ -174,11 +174,13 @@ export const GRAPH_DATA_SETTINGS = {
     },
     getProps: (series, settings) => {
       const seriesSettings = settings["series_settings"] || {};
+      const seriesColors = settings["series_settings.colors"] || {};
       const keys = series.map(s => keyForSingleSeries(s));
       return {
         items: keys.map((key, index) => ({
           name: seriesSettings[key]?.title || key,
           originalIndex: index,
+          color: seriesColors[key],
         })),
         series,
       };
@@ -187,6 +189,7 @@ export const GRAPH_DATA_SETTINGS = {
       return settings["graph.dimensions"]?.length < 2 || series.length > 20;
     },
     dashboard: false,
+    readDependencies: ["series_settings.colors"],
   },
   "graph.metrics": {
     section: t`Data`,
@@ -206,9 +209,9 @@ export const GRAPH_DATA_SETTINGS = {
             vizSettings["graph._metric_filter"],
           ),
       ),
-    getDefault: (series, vizSettings) => getDefaultColumns(series).metrics,
+    getDefault: series => getDefaultColumns(series).metrics,
     persistDefault: true,
-    getProps: ([{ card, data }], vizSettings) => {
+    getProps: ([{ card, data }], vizSettings, _onChange, extra) => {
       const options = data.cols
         .filter(vizSettings["graph._metric_filter"])
         .map(getOptionFromColumn);
@@ -230,9 +233,16 @@ export const GRAPH_DATA_SETTINGS = {
         addAnother: canAddAnother ? t`Add another series` : null,
         columns: data.cols,
         showColumnSetting: true,
+        showColorPicker: !hasBreakout,
+        colors: vizSettings["series_settings.colors"],
+        series: extra.transformedSeries,
       };
     },
-    readDependencies: ["graph._dimension_filter", "graph._metric_filter"],
+    readDependencies: [
+      "graph._dimension_filter",
+      "graph._metric_filter",
+      "series_settings.colors",
+    ],
     writeDependencies: ["graph.dimensions"],
     dashboard: false,
     useRawSeries: true,

frontend/src/metabase/visualizations/lib/settings/series.js

@@ -14,14 +14,14 @@ export function keyForSingleSeries(single) {
 
 const LINE_DISPLAY_TYPES = new Set(["line", "area"]);
 
+export const SETTING_ID = "series_settings";
+export const COLOR_SETTING_ID = "series_settings.colors";
+
 export function seriesSetting({
   readDependencies = [],
   noPadding,
   ...def
 } = {}) {
-  const settingId = "series_settings";
-  const colorSettingId = "series_settings.colors";
-
   const COMMON_SETTINGS = {
     // title, and color don't need widgets because they're handled direclty in ChartNestedSettingSeries
     title: {
@@ -69,7 +69,7 @@ export function seriesSetting({
     color: {
       getDefault: (single, settings, { settings: vizSettings }) =>
         // get the color for series key, computed in the setting
-        getIn(vizSettings, [colorSettingId, keyForSingleSeries(single)]),
+        getIn(vizSettings, [COLOR_SETTING_ID, keyForSingleSeries(single)]),
     },
     "line.interpolate": {
       title: t`Line style`,
@@ -156,7 +156,7 @@ export function seriesSetting({
   }
 
   return {
-    ...nestedSettings(settingId, {
+    ...nestedSettings(SETTING_ID, {
       getHidden: ([{ card }], settings, { isDashboard }) =>
         !isDashboard || card?.display === "waterfall",
       getSection: (series, settings, { isDashboard }) =>
@@ -166,17 +166,17 @@ export function seriesSetting({
       getObjectKey: keyForSingleSeries,
       getSettingDefinitionsForObject: getSettingDefinitionsForSingleSeries,
       component: ChartNestedSettingSeries,
-      readDependencies: [colorSettingId, ...readDependencies],
+      readDependencies: [COLOR_SETTING_ID, ...readDependencies],
       noPadding: true,
       ...def,
     }),
     // colors must be computed as a whole rather than individually
-    [colorSettingId]: {
+    [COLOR_SETTING_ID]: {
       getValue(series, settings) {
         const keys = series.map(single => keyForSingleSeries(single));
 
         const assignments = _.chain(keys)
-          .map(key => [key, getIn(settings, [settingId, key, "color"])])
+          .map(key => [key, getIn(settings, [SETTING_ID, key, "color"])])
           .filter(([key, color]) => color != null)
           .object()
           .value();