From 2a57e9e2ebe435e162d173c5a51b4969cab0f0b1 Mon Sep 17 00:00:00 2001
From: Nick Fitzpatrick <nick@metabase.com>
Date: Wed, 16 Nov 2022 12:46:11 -0400
Subject: [PATCH] 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
---
.../components/ColorPill/ColorPill.styled.tsx | 16 ++++++++++++
.../core/components/ColorPill/ColorPill.tsx | 9 ++++++-
.../core/components/ColorPill/types.ts | 1 +
.../components/ChartSettings.jsx | 9 +++++++
.../settings/ChartSettingColorPicker.jsx | 6 +++--
.../settings/ChartSettingFieldPicker.jsx | 26 +++++++++++++++++++
.../ChartSettingFieldPicker.styled.tsx | 7 +++++
.../settings/ChartSettingOrderedItems.tsx | 11 ++++++++
.../settings/ChartSettingOrderedSimple.tsx | 15 ++++++++++-
.../components/settings/ColumnItem.jsx | 10 +++++++
.../components/settings/ColumnItem.styled.tsx | 7 +++++
.../src/metabase/visualizations/lib/series.ts | 11 ++++++++
.../metabase/visualizations/lib/settings.js | 1 +
.../visualizations/lib/settings/graph.js | 16 +++++++++---
.../visualizations/lib/settings/series.js | 16 ++++++------
15 files changed, 146 insertions(+), 15 deletions(-)
create mode 100644 frontend/src/metabase/core/components/ColorPill/types.ts
create mode 100644 frontend/src/metabase/visualizations/lib/series.ts
diff --git a/frontend/src/metabase/core/components/ColorPill/ColorPill.styled.tsx b/frontend/src/metabase/core/components/ColorPill/ColorPill.styled.tsx
index be38503853b..2d290c88277 100644
--- a/frontend/src/metabase/core/components/ColorPill/ColorPill.styled.tsx
+++ b/frontend/src/metabase/core/components/ColorPill/ColorPill.styled.tsx
@@ -1,9 +1,14 @@
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`
diff --git a/frontend/src/metabase/core/components/ColorPill/ColorPill.tsx b/frontend/src/metabase/core/components/ColorPill/ColorPill.tsx
index 071ca11ec53..3b472b1c507 100644
--- a/frontend/src/metabase/core/components/ColorPill/ColorPill.tsx
+++ b/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,
+});
diff --git a/frontend/src/metabase/core/components/ColorPill/types.ts b/frontend/src/metabase/core/components/ColorPill/types.ts
new file mode 100644
index 00000000000..901639b4519
--- /dev/null
+++ b/frontend/src/metabase/core/components/ColorPill/types.ts
@@ -0,0 +1 @@
+export type PillSize = "small" | "medium";
diff --git a/frontend/src/metabase/visualizations/components/ChartSettings.jsx b/frontend/src/metabase/visualizations/components/ChartSettings.jsx
index b573744913c..88fcb84d9bd 100644
--- a/frontend/src/metabase/visualizations/components/ChartSettings.jsx
+++ b/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 = (
diff --git a/frontend/src/metabase/visualizations/components/settings/ChartSettingColorPicker.jsx b/frontend/src/metabase/visualizations/components/settings/ChartSettingColorPicker.jsx
index 3e3a9161f07..16257d06943 100644
--- a/frontend/src/metabase/visualizations/components/settings/ChartSettingColorPicker.jsx
+++ b/frontend/src/metabase/visualizations/components/settings/ChartSettingColorPicker.jsx
@@ -1,18 +1,20 @@
/* 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>
diff --git a/frontend/src/metabase/visualizations/components/settings/ChartSettingFieldPicker.jsx b/frontend/src/metabase/visualizations/components/settings/ChartSettingFieldPicker.jsx
index 2fa061a4f86..55a4219354f 100644
--- a/frontend/src/metabase/visualizations/components/settings/ChartSettingFieldPicker.jsx
+++ b/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}
diff --git a/frontend/src/metabase/visualizations/components/settings/ChartSettingFieldPicker.styled.tsx b/frontend/src/metabase/visualizations/components/settings/ChartSettingFieldPicker.styled.tsx
index 56a175572a0..0c9e638949d 100644
--- a/frontend/src/metabase/visualizations/components/settings/ChartSettingFieldPicker.styled.tsx
+++ b/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;
+`;
diff --git a/frontend/src/metabase/visualizations/components/settings/ChartSettingOrderedItems.tsx b/frontend/src/metabase/visualizations/components/settings/ChartSettingOrderedItems.tsx
index dd9ea674cc0..d974020ca37 100644
--- a/frontend/src/metabase/visualizations/components/settings/ChartSettingOrderedItems.tsx
+++ b/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}
/>
);
diff --git a/frontend/src/metabase/visualizations/components/settings/ChartSettingOrderedSimple.tsx b/frontend/src/metabase/visualizations/components/settings/ChartSettingOrderedSimple.tsx
index e4f56c12341..0c7baf42ba6 100644
--- a/frontend/src/metabase/visualizations/components/settings/ChartSettingOrderedSimple.tsx
+++ b/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}
/>
) : (
diff --git a/frontend/src/metabase/visualizations/components/settings/ColumnItem.jsx b/frontend/src/metabase/visualizations/components/settings/ColumnItem.jsx
index 29dc9fb78f5..6dd7f501da4 100644
--- a/frontend/src/metabase/visualizations/components/settings/ColumnItem.jsx
+++ b/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} />}
diff --git a/frontend/src/metabase/visualizations/components/settings/ColumnItem.styled.tsx b/frontend/src/metabase/visualizations/components/settings/ColumnItem.styled.tsx
index 2e9bf7d12f4..a2cf35c075d 100644
--- a/frontend/src/metabase/visualizations/components/settings/ColumnItem.styled.tsx
+++ b/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;
+`;
diff --git a/frontend/src/metabase/visualizations/lib/series.ts b/frontend/src/metabase/visualizations/lib/series.ts
new file mode 100644
index 00000000000..efdfc240b50
--- /dev/null
+++ b/frontend/src/metabase/visualizations/lib/series.ts
@@ -0,0 +1,11 @@
+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);
+};
diff --git a/frontend/src/metabase/visualizations/lib/settings.js b/frontend/src/metabase/visualizations/lib/settings.js
index df37172964a..f52dc8900e6 100644
--- a/frontend/src/metabase/visualizations/lib/settings.js
+++ b/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 {
diff --git a/frontend/src/metabase/visualizations/lib/settings/graph.js b/frontend/src/metabase/visualizations/lib/settings/graph.js
index 9257d826e99..d895cba8798 100644
--- a/frontend/src/metabase/visualizations/lib/settings/graph.js
+++ b/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,
diff --git a/frontend/src/metabase/visualizations/lib/settings/series.js b/frontend/src/metabase/visualizations/lib/settings/series.js
index 4a49f0a624e..2eeafa283c0 100644
--- a/frontend/src/metabase/visualizations/lib/settings/series.js
+++ b/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();
--
GitLab