Tweak color assignment for 9-16 series (#23078)

enterprise/frontend/src/metabase-enterprise/whitelabel/components/ChartColorSettings/ChartColorSettings.tsx

@@ -78,13 +78,12 @@ const ChartColorTable = ({
       <TableBody>
         {colorGroups.map((colorGroup, index) => (
           <TableBodyRow key={index}>
-            {colorGroup.map((colorName, index) => (
+            {colorGroup.map(colorName => (
               <ChartColorCell
                 key={colorName}
                 color={colors[colorName]}
                 originalColor={color(colorName, colorPalette)}
                 colorName={colorName}
-                isAccent={index === 0}
                 onChange={onChange}
               />
             ))}
@@ -99,7 +98,6 @@ interface ChartColorCellProps {
   color?: string;
   originalColor: string;
   colorName: string;
-  isAccent: boolean;
   onChange: (colorName: string, color?: string) => void;
 }
 
@@ -107,7 +105,6 @@ const ChartColorCell = memo(function ChartColorCell({
   color,
   originalColor,
   colorName,
-  isAccent,
   onChange,
 }: ChartColorCellProps) {
   const handleChange = useCallback(
@@ -121,8 +118,8 @@ const ChartColorCell = memo(function ChartColorCell({
     <TableBodyCell>
       <ColorPicker
         value={color ?? originalColor}
-        placeholder={isAccent ? originalColor : t`Auto`}
-        isAuto={color == null && !isAccent}
+        placeholder={t`Auto`}
+        isAuto={color == null}
         onChange={handleChange}
       />
     </TableBodyCell>

frontend/src/metabase/components/form/widgets/FormColorWidget.jsx

 /* eslint-disable react/prop-types */
 import React from "react";
 
-import { getForegroundColors } from "metabase/lib/colors/groups";
+import { getDistinctColors } from "metabase/lib/colors/groups";
 import ColorSelector from "metabase/core/components/ColorSelector";
 
 const FormColorWidget = ({ field, initial }) => (
@@ -9,7 +9,7 @@ const FormColorWidget = ({ field, initial }) => (
     <ColorSelector
       {...field}
       value={field.value || initial()}
-      colors={getForegroundColors()}
+      colors={getDistinctColors()}
     />
   </div>
 );

frontend/src/metabase/lib/colors/charts.ts

@@ -15,7 +15,7 @@ export const getColorsForValues = (
   } else {
     return getOrderBasedMapping(
       keys,
-      getHarmonyColors(),
+      getHarmonyColors({ light: keys.length > ACCENT_COUNT * 2 }),
       existingMapping,
       getPreferredColor,
     );

frontend/src/metabase/lib/colors/charts.unit.spec.ts

@@ -10,9 +10,9 @@ describe("charts", () => {
     const newMapping = getColorsForValues(keys, existingMapping);
 
     expect(newMapping).toEqual({
-      count: color("accent1"),
-      profit: color("success"),
-      sum_2: color("accent6"),
+      count: color("accent1"), // existing colors are not changed
+      profit: color("success"), // a preferred color
+      sum_2: color("accent6"), // only accent colors are used for other keys
     });
   });
 
@@ -23,10 +23,10 @@ describe("charts", () => {
     const newMapping = getColorsForValues(keys, existingMapping);
 
     expect(newMapping).toEqual({
-      count: color("accent1"),
-      profit: color("success"),
-      distinct: color("accent4"),
-      sum_2: color("accent6"),
+      count: color("accent1"), // existing colors are not changed
+      profit: color("success"), // a preferred color
+      distinct: color("accent4"), // some color based on the hash
+      sum_2: color("accent6"), // the same color is used despite different keys count
     });
   });
 
@@ -37,14 +37,14 @@ describe("charts", () => {
     const newMapping = getColorsForValues(keys, existingMapping);
 
     expect(newMapping).toMatchObject({
-      count: color("accent1"),
-      sum: color("accent0"),
-      profit: color("success"),
-      S0: color("accent0-light"),
-      S1: color("accent0-dark"),
-      S2: color("accent1-light"),
-      S3: color("accent1-dark"),
-      S4: color("accent2"),
+      count: color("accent1"), // existing colors are not changed
+      sum: color("accent0"), // a color from the palette because accent1 would be preferred, but it's already used
+      profit: color("success"), // a preferred color
+      S0: color("accent0-dark"), // the next color from palette
+      S1: color("accent1-dark"), // only dark accents are used when there are <= 16 keys
+      S2: color("accent2"),
+      S3: color("accent2-dark"),
+      S4: color("accent3"),
     });
   });
 
@@ -55,15 +55,15 @@ describe("charts", () => {
     const newMapping = getColorsForValues(keys, existingMapping);
 
     expect(newMapping).toMatchObject({
-      count: color("accent1"),
-      sum: color("accent0"),
-      profit: color("success"),
-      S0: color("accent0-light"),
-      S1: color("accent0-dark"),
+      count: color("accent1"), // existing colors are not changed
+      sum: color("accent0"), // a color from the palette because accent1 would be preferred, but it's already used
+      profit: color("success"), // a preferred color
+      S0: color("accent0-light"), // the next color from palette
+      S1: color("accent0-dark"), // both light and dark accents are used when there are > 16 series
       S2: color("accent1-light"),
       S3: color("accent1-dark"),
       S4: color("accent2"),
-      S28: color("accent2"),
+      S28: color("accent2"), // we have 24 colors in the palette, that's why they would repeat after 24 keys
     });
   });
 });

frontend/src/metabase/lib/colors/groups.ts

 import { times, unzip } from "lodash";
 import { ACCENT_COUNT, color } from "./palette";
+import { ColorGroupOptions } from "./types";
 
 export const getAccentColors = () => {
   return times(ACCENT_COUNT, i => color(`accent${i}`));
@@ -13,16 +14,23 @@ export const getShadeColors = () => {
   return times(ACCENT_COUNT, i => color(`accent${i}-dark`));
 };
 
-export const getForegroundColors = () => {
-  return [...getAccentColors(), ...getTintColors(), ...getShadeColors()];
+export const getDistinctColors = (options?: ColorGroupOptions) => {
+  return getAccentColorRanges(options).flat();
 };
 
-export const getBackgroundColors = () => {
-  return [...getAccentColors(), ...getTintColors()];
+export const getHarmonyColors = (options?: ColorGroupOptions) => {
+  return unzip(getAccentColorRanges(options)).flat();
 };
 
-export const getHarmonyColors = () => {
-  return unzip([getAccentColors(), getTintColors(), getShadeColors()]).flat();
+export const getAccentColorRanges = ({
+  light = true,
+  dark = true,
+}: ColorGroupOptions = {}) => {
+  const ranges = [getAccentColors()];
+  light && ranges.push(getTintColors());
+  dark && ranges.push(getShadeColors());
+
+  return ranges;
 };
 
 export const getStatusColorRanges = () => {

frontend/src/metabase/lib/colors/types.ts

 export type ColorPalette = Record<string, string>;
+
+export interface ColorGroupOptions {
+  light?: boolean;
+  dark?: boolean;
+}

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

@@ -2,13 +2,13 @@
 import React, { Component } from "react";
 import PropTypes from "prop-types";
 import cx from "classnames";
-import { getForegroundColors } from "metabase/lib/colors/groups";
+import { getDistinctColors } from "metabase/lib/colors/groups";
 import Icon, { iconPropTypes } from "metabase/components/Icon";
 import ExplicitSize from "../../components/ExplicitSize";
 import LegendItem from "./LegendItem";
 import styles from "./Legend.css";
 
-const DEFAULT_COLORS = getForegroundColors();
+const DEFAULT_COLORS = getDistinctColors();
 const MIN_WIDTH_PER_SERIES = 100;
 
 class LegendHeader extends Component {

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

@@ -74,7 +74,7 @@ for (let i = 0; i < MAX_SERIES; i++) {
   addCSSRule(`.LineAreaBarChart.mute-${i} svg:not(.stacked) .row`, MUTE_STYLE);
 }
 
-import { getForegroundColors } from "metabase/lib/colors/groups";
+import { getDistinctColors } from "metabase/lib/colors/groups";
 
 export default class LineAreaBarChart extends Component {
   static noHeader = true;
@@ -297,7 +297,7 @@ export default class LineAreaBarChart extends Component {
       : series.map(single => single.card.name);
     const colors = seriesSettings
       ? seriesSettings.map(s => s.color)
-      : Object.values(getForegroundColors());
+      : Object.values(getDistinctColors());
 
     return {
       title,

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

 /* eslint-disable react/prop-types */
 import React from "react";
 
-import { getForegroundColors } from "metabase/lib/colors/groups";
+import { getDistinctColors } from "metabase/lib/colors/groups";
 import ColorSelector from "metabase/core/components/ColorSelector";
 import { SegmentedControl } from "metabase/components/SegmentedControl";
 import Icon from "metabase/components/Icon";
@@ -42,7 +42,7 @@ export default class ChartNestedSettingSeries extends React.Component {
                 <div className="flex align-center">
                   <ColorSelector
                     value={settings.color}
-                    colors={getForegroundColors()}
+                    colors={getDistinctColors()}
                     onChange={value =>
                       onChangeObjectSettings(single, { color: value })
                     }

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

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

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

@@ -5,7 +5,7 @@ import { t } from "ttag";
 import _ from "underscore";
 
 import { color } from "metabase/lib/colors";
-import { getForegroundColors } from "metabase/lib/colors/groups";
+import { getDistinctColors } from "metabase/lib/colors/groups";
 
 import ColorSelector from "metabase/core/components/ColorSelector";
 import Button from "metabase/core/components/Button";
@@ -101,7 +101,7 @@ const ChartSettingGaugeSegments = ({ value: segments, onChange }) => {
 
 function getColorPalette() {
   return [
-    ...getForegroundColors(),
+    ...getDistinctColors(),
     color("error"),
     color("warning"),
     color("success"),

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

@@ -4,7 +4,7 @@ import React from "react";
 import { t, jt } from "ttag";
 
 import {
-  getBackgroundColors,
+  getDistinctColors,
   getStatusColorRanges,
 } from "metabase/lib/colors/groups";
 
@@ -57,7 +57,7 @@ export const ALL_OPERATOR_NAMES = {
 };
 
 // TODO
-const COLORS = getBackgroundColors();
+const COLORS = getDistinctColors({ dark: false });
 const COLOR_RANGES = getStatusColorRanges();
 
 const DEFAULTS_BY_TYPE = {

frontend/src/metabase/visualizations/visualizations/Map.jsx

@@ -31,7 +31,7 @@ import _ from "underscore";
 
 const PIN_MAP_TYPES = new Set(["pin", "heat", "grid"]);
 
-import { getForegroundColors } from "metabase/lib/colors/groups";
+import { getDistinctColors } from "metabase/lib/colors/groups";
 import ColorRangeSelector from "metabase/core/components/ColorRangeSelector";
 
 export default class Map extends Component {
@@ -257,16 +257,16 @@ export default class Map extends Component {
       title: t`Color`,
       widget: ColorRangeSelector,
       props: {
-        colors: getForegroundColors(),
+        colors: getDistinctColors(),
         colorMapping: Object.fromEntries(
-          getForegroundColors().map(color => [
+          getDistinctColors().map(color => [
             color,
             getColorplethColorScale(color),
           ]),
         ),
         isQuantile: true,
       },
-      default: getColorplethColorScale(getForegroundColors()[0]),
+      default: getColorplethColorScale(getDistinctColors()[0]),
       getHidden: (series, vizSettings) => vizSettings["map.type"] !== "region",
     },
     "map.zoom": {},