Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor/chart config custom color palettes #1930

Draft
wants to merge 21 commits into
base: main
Choose a base branch
from
Draft

Refactor/chart config custom color palettes #1930

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
6f5f99f
refactor: Refactored chart config types
noahonyejese Dec 3, 2024
4465745
refactor: fixed all erros that occured due to chart config refactor
noahonyejese Dec 3, 2024
591492b
fix: Fixed silent error
noahonyejese Dec 3, 2024
c0938df
fix: Fixed all of bartosz's suggestions
noahonyejese Dec 4, 2024
ac51939
wip
noahonyejese Dec 5, 2024
544d51e
refactore: Refactored getPalette
noahonyejese Dec 6, 2024
8c0f353
refactor: Refactored getPalette
noahonyejese Dec 6, 2024
04b8161
refactor: improved formating
noahonyejese Dec 6, 2024
f1229b3
refactor: extended color palettes
noahonyejese Dec 6, 2024
9f4dc02
fix: Changed Type name
noahonyejese Dec 11, 2024
9684d6f
fix: Changed column color handling to conform to other Chart types
noahonyejese Dec 11, 2024
bb8272a
fix: removed unnecessary client check
noahonyejese Dec 11, 2024
f3e7cfb
fix: improved state for color picker changes
noahonyejese Dec 11, 2024
f20ac39
refactor: refactored old way of handling colors
noahonyejese Dec 11, 2024
5c386ff
chore: Added comment for dynamic import reasoning
noahonyejese Dec 11, 2024
1e2147d
chore: Added color control translations
noahonyejese Dec 11, 2024
adec4ab
fix: Added chartConfig fields type check
noahonyejese Dec 11, 2024
d25212f
fix: Added missing getColorLabel type
noahonyejese Dec 11, 2024
9070012
fix: Fixed reverting segmented colors back to single colors
noahonyejese Dec 11, 2024
c5e117e
fix: removed exsesive theme import
noahonyejese Dec 11, 2024
f128f59
fix: Fixed Type issues
noahonyejese Dec 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions app/charts/area/areas-state.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ import { useSize } from "@/charts/shared/use-size";
import { AreaConfig } from "@/configurator";
import { Observation } from "@/domain/data";
import { useFormatNumber, useTimeFormatUnit } from "@/formatters";
import { getPalette } from "@/palettes";
import { getPaletteId } from "@/palettes";
import { useChartInteractiveFilters } from "@/stores/interactive-filters";
import { sortByIndex } from "@/utils/array";
import {
Expand Down Expand Up @@ -248,7 +248,8 @@ const useAreasState = (
const xScaleTimeRange = scaleTime().domain(xScaleTimeRangeDomain);
const colors = scaleOrdinal<string, string>();

if (segmentDimension && fields.segment?.colorMapping) {
if (segmentDimension && fields.color.type === "segment") {
const segmentColor = fields.color;
const orderedSegmentLabelsAndColors = allSegments.map((segment) => {
const dvIri =
segmentsByAbbreviationOrLabel.get(segment)?.value ??
Expand All @@ -257,15 +258,15 @@ const useAreasState = (

return {
label: segment,
color: fields.segment?.colorMapping![dvIri] ?? schemeCategory10[0],
color: segmentColor.colorMapping[dvIri] ?? schemeCategory10[0],
};
});

colors.domain(orderedSegmentLabelsAndColors.map((s) => s.label));
colors.range(orderedSegmentLabelsAndColors.map((s) => s.color));
} else {
colors.domain(allSegments);
colors.range(getPalette(fields.segment?.palette));
colors.range(getPaletteId(fields.color.paletteId));
}

colors.unknown(() => undefined);
Expand All @@ -276,8 +277,7 @@ const useAreasState = (
xScaleTimeRange,
};
}, [
fields.segment?.palette,
fields.segment?.colorMapping,
fields.color,
getX,
scalesData,
timeRangeData,
Expand Down
73 changes: 38 additions & 35 deletions app/charts/chart-config-ui-options.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
ChartConfig,
ChartSubType,
ChartType,
ColorField,
ColorScaleType,
ColumnConfig,
ColumnSegmentField,
Expand All @@ -29,6 +28,7 @@
ComponentType,
GenericField,
LineConfig,
MapColorField,
MapConfig,
PaletteType,
PieConfig,
Expand Down Expand Up @@ -56,7 +56,7 @@
isTemporalEntityDimension,
isTemporalOrdinalDimension,
} from "@/domain/data";
import { getDefaultCategoricalPaletteName, getPalette } from "@/palettes";
import { getDefaultCategoricalPaletteId, getPaletteId } from "@/palettes";

/**
* This module controls chart controls displayed in the UI.
Expand All @@ -65,7 +65,7 @@

type BaseEncodingFieldType = "animation";
type MapEncodingFieldType = "baseLayer" | "areaLayer" | "symbolLayer";
type XYEncodingFieldType = "x" | "y" | "segment";
type XYEncodingFieldType = "x" | "y" | "segment" | "color";
noahonyejese marked this conversation as resolved.
Show resolved Hide resolved
export type EncodingFieldType =
| BaseEncodingFieldType
| MapEncodingFieldType
Expand Down Expand Up @@ -175,7 +175,7 @@
) => {
const basePath = `fields["${field}"]`;
const components = [...dimensions, ...measures];
let newField: ColorField = DEFAULT_FIXED_COLOR_FIELD;
let newField: MapColorField = DEFAULT_FIXED_COLOR_FIELD;
const component = components.find((d) => d.id === id);
const currentColorComponentId = get(
chartConfig,
Expand All @@ -192,7 +192,7 @@
MULTI_FILTER_ENABLED_COMPONENTS.includes(component.__typename) ||
isOrdinalMeasure(component)
) {
const palette = getDefaultCategoricalPaletteName(component, colorPalette);
const palette = getDefaultCategoricalPaletteId(component, colorPalette);
newField = getDefaultCategoricalColorField({
id,
palette,
Expand Down Expand Up @@ -411,7 +411,7 @@
}

const fieldComponentsMap = Object.fromEntries(
Object.entries<GenericField>(chartConfig.fields)

Check failure on line 414 in app/charts/chart-config-ui-options.ts

View workflow job for this annotation

GitHub Actions / lint-typecheck-test 

Argument of type '({ x: { componentId: string; } & { useAbbreviations?: boolean | undefined; } & { sorting?: { sortingType: "byDimensionLabel" | "byMeasure" | "byTotalSize" | "byAuto"; sortingOrder: "desc" | "asc"; } | undefined; }; y: { ...; } & ... 1 more ... & { ...; }; color: { ...; } | { ...; }; } & { ...; }) | { ...; } | ({ ......' is not assignable to parameter of type 'ArrayLike<{ componentId: string; } & { useAbbreviations?: boolean | undefined; }> | { [s: string]: { componentId: string; } & { useAbbreviations?: boolean | undefined; }; }'.
.filter((d) => d[0] !== "animation")
.map(([k, v]) => [v.componentId, k])
);
Expand Down Expand Up @@ -483,25 +483,27 @@
> = (id, { chartConfig, dimensions, measures, selectedValues }) => {
const components = [...dimensions, ...measures];
const component = components.find((d) => d.id === id);
const palette = getDefaultCategoricalPaletteName(
const palette = getDefaultCategoricalPaletteId(
noahonyejese marked this conversation as resolved.
Show resolved Hide resolved
component,
chartConfig.fields.segment && "palette" in chartConfig.fields.segment
? chartConfig.fields.segment.palette
chartConfig.fields.color && "paletteId" in chartConfig.fields.color
? chartConfig.fields.color.paletteId
: undefined
);
const colorMapping = mapValueIrisToColor({
palette,
dimensionValues: component ? component.values : selectedValues,
});

if (chartConfig.fields.segment && "palette" in chartConfig.fields.segment) {
if (chartConfig.fields.segment) {
chartConfig.fields.segment.componentId = id;
chartConfig.fields.segment.colorMapping = colorMapping;
} else {
chartConfig.fields.segment = {
componentId: id,
palette,
sorting: DEFAULT_SORTING,
};
chartConfig.fields.color = {
type: "segment",
paletteId: palette,
colorMapping,
};
}
Expand Down Expand Up @@ -955,12 +957,12 @@
onChange: (ids, options) => {
const { chartConfig } = options;
const { fields } = chartConfig;
const { y } = fields;
const palette = getPalette(y.palette);
const { color } = fields;
const palette = getPaletteId(color.paletteId);
noahonyejese marked this conversation as resolved.
Show resolved Hide resolved
const newColorMapping = Object.fromEntries(
ids.map((id, i) => [id, y.colorMapping[i] ?? palette[i]])
ids.map((id, i) => [id, color.colorMapping[i] ?? palette[i]])
);
chartConfig.fields.y.colorMapping = newColorMapping;
chartConfig.fields.color.colorMapping = newColorMapping;
noahonyejese marked this conversation as resolved.
Show resolved Hide resolved
},
},
},
Expand Down Expand Up @@ -990,22 +992,23 @@
onChange: (id, options) => {
const { chartConfig } = options;
const { fields } = chartConfig;
const { y } = fields;
chartConfig.fields.y.colorMapping = {
[id]: y.colorMapping[y.leftAxisComponentId],
const { y, color } = fields;
chartConfig.fields.color.colorMapping = {
[id]: color.colorMapping[y.leftAxisComponentId],
[y.rightAxisComponentId]:
y.colorMapping[y.rightAxisComponentId],
color.colorMapping[y.rightAxisComponentId],
};
},
},
rightAxisComponentId: {
onChange: (id, options) => {
const { chartConfig } = options;
const { fields } = chartConfig;
const { y } = fields;
chartConfig.fields.y.colorMapping = {
[y.leftAxisComponentId]: y.colorMapping[y.leftAxisComponentId],
[id]: y.colorMapping[y.rightAxisComponentId],
const { y, color } = fields;
chartConfig.fields.color.colorMapping = {
[y.leftAxisComponentId]:
color.colorMapping[y.leftAxisComponentId],
[id]: color.colorMapping[y.rightAxisComponentId],
};
},
},
Expand Down Expand Up @@ -1036,11 +1039,11 @@
onChange: (id, options) => {
const { chartConfig } = options;
const { fields } = chartConfig;
const { y } = fields;
const lineColor = y.colorMapping[y.lineComponentId];
const columnColor = y.colorMapping[y.columnComponentId];
const { y, color } = fields;
const lineColor = color.colorMapping[y.lineComponentId];
const columnColor = color.colorMapping[y.columnComponentId];

chartConfig.fields.y.colorMapping =
chartConfig.fields.color.colorMapping =
y.lineAxisOrientation === "left"
? { [id]: lineColor, [y.columnComponentId]: columnColor }
: { [y.columnComponentId]: columnColor, [id]: lineColor };
Expand All @@ -1050,11 +1053,11 @@
onChange: (id, options) => {
const { chartConfig } = options;
const { fields } = chartConfig;
const { y } = fields;
const columnColor = y.colorMapping[y.columnComponentId];
const lineColor = y.colorMapping[y.lineComponentId];
const { y, color } = fields;
const columnColor = color.colorMapping[y.columnComponentId];
const lineColor = color.colorMapping[y.lineComponentId];

chartConfig.fields.y.colorMapping =
chartConfig.fields.color.colorMapping =
y.lineAxisOrientation === "left"
? { [y.lineComponentId]: lineColor, [id]: columnColor }
: { [id]: columnColor, [y.lineComponentId]: lineColor };
Expand All @@ -1064,7 +1067,7 @@
onChange: (_, options) => {
const { chartConfig } = options;
const { fields } = chartConfig;
const { y } = fields;
const { y, color } = fields;
const lineAxisLeft = y.lineAxisOrientation === "left";
// Need the correct order to not enable "Reset color palette" button.
const firstId = lineAxisLeft
Expand All @@ -1074,9 +1077,9 @@
? y.lineComponentId
: y.columnComponentId;

chartConfig.fields.y.colorMapping = {
[firstId]: y.colorMapping[secondId],
[secondId]: y.colorMapping[firstId],
chartConfig.fields.color.colorMapping = {
[firstId]: color.colorMapping[secondId],
[secondId]: color.colorMapping[firstId],
};
},
},
Expand Down
10 changes: 6 additions & 4 deletions app/charts/column/columns-grouped-state.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import { useSize } from "@/charts/shared/use-size";
import { ColumnConfig } from "@/configurator";
import { Observation } from "@/domain/data";
import { formatNumberWithUnit, useFormatNumber } from "@/formatters";
import { getPalette } from "@/palettes";
import { getPaletteId } from "@/palettes";
import { sortByIndex } from "@/utils/array";
import {
getSortingOrders,
Expand Down Expand Up @@ -189,7 +189,8 @@ const useColumnsGroupedState = (
} = useMemo(() => {
const colors = scaleOrdinal<string, string>();

if (fields.segment && segmentDimension && fields.segment.colorMapping) {
if (fields.segment && segmentDimension && fields.color.type === "segment") {
noahonyejese marked this conversation as resolved.
Show resolved Hide resolved
const segmentColor = fields.color;
const orderedSegmentLabelsAndColors = allSegments.map((segment) => {
const dvIri =
segmentsByAbbreviationOrLabel.get(segment)?.value ||
Expand All @@ -198,15 +199,15 @@ const useColumnsGroupedState = (

return {
label: segment,
color: fields.segment?.colorMapping![dvIri] ?? schemeCategory10[0],
color: segmentColor.colorMapping![dvIri] ?? schemeCategory10[0],
};
});

colors.domain(orderedSegmentLabelsAndColors.map((s) => s.label));
colors.range(orderedSegmentLabelsAndColors.map((s) => s.color));
} else {
colors.domain(allSegments);
colors.range(getPalette(fields.segment?.palette));
colors.range(getPaletteId(fields.color.paletteId));
}

colors.unknown(() => undefined);
Expand Down Expand Up @@ -278,6 +279,7 @@ const useColumnsGroupedState = (
};
}, [
fields.segment,
fields.color,
fields.x?.sorting,
fields.x?.useAbbreviations,
segmentDimension,
Expand Down
10 changes: 6 additions & 4 deletions app/charts/column/columns-stacked-state.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ import { useSize } from "@/charts/shared/use-size";
import { ColumnConfig } from "@/configurator";
import { Observation } from "@/domain/data";
import { useFormatNumber } from "@/formatters";
import { getPalette } from "@/palettes";
import { getPaletteId } from "@/palettes";
import { useChartInteractiveFilters } from "@/stores/interactive-filters";
import { sortByIndex } from "@/utils/array";
import {
Expand Down Expand Up @@ -227,8 +227,9 @@ const useColumnsStackedState = (
if (
fields.segment &&
segmentsByAbbreviationOrLabel &&
fields.segment.colorMapping
fields.color.type === "segment"
) {
const segmentColor = fields.color;
const orderedSegmentLabelsAndColors = allSegments.map((segment) => {
// FIXME: Labels in observations can differ from dimension values because the latter can be concatenated to only appear once per value
// See https://github.com/visualize-admin/visualization-tool/issues/97
Expand All @@ -244,15 +245,15 @@ const useColumnsStackedState = (

return {
label: segment,
color: fields.segment?.colorMapping![dvIri] ?? schemeCategory10[0],
color: segmentColor.colorMapping![dvIri] ?? schemeCategory10[0],
};
});

colors.domain(orderedSegmentLabelsAndColors.map((s) => s.label));
colors.range(orderedSegmentLabelsAndColors.map((s) => s.color));
} else {
colors.domain(allSegments);
colors.range(getPalette(fields.segment?.palette));
colors.range(getPaletteId(fields.color.paletteId));
}

colors.unknown(() => undefined);
Expand Down Expand Up @@ -295,6 +296,7 @@ const useColumnsStackedState = (
};
}, [
fields.segment,
fields.color,
fields.x.sorting,
fields.x.useAbbreviations,
xDimension,
Expand Down
4 changes: 2 additions & 2 deletions app/charts/combo/combo-line-column-state-props.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export const useComboLineColumnStateVariables = (
dimension: measuresById[lineId],
id: lineId,
label: getLabelWithUnit(measuresById[lineId]),
color: fields.y.colorMapping[lineId],
color: fields.color.colorMapping[lineId],
getY: (d) => (d[lineId] !== null ? Number(d[lineId]) : null),
getMinY: (data) => {
const minY =
Expand All @@ -94,7 +94,7 @@ export const useComboLineColumnStateVariables = (
dimension: measuresById[columnId],
id: columnId,
label: getLabelWithUnit(measuresById[columnId]),
color: fields.y.colorMapping[columnId],
color: fields.color.colorMapping[columnId],
getY: (d) => (d[columnId] !== null ? Number(d[columnId]) : null),
getMinY: (data) => {
const minY =
Expand Down
4 changes: 2 additions & 2 deletions app/charts/combo/combo-line-dual-state-props.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export const useComboLineDualStateVariables = (
dimension: measuresById[leftId],
id: leftId,
label: getLabelWithUnit(measuresById[leftId]),
color: fields.y.colorMapping[leftId],
color: fields.color.colorMapping[leftId],
getY: (d) => (d[leftId] !== null ? Number(d[leftId]) : null),
getMinY: (data) => {
const minY =
Expand All @@ -77,7 +77,7 @@ export const useComboLineDualStateVariables = (
dimension: measuresById[rightId],
id: rightId,
label: getLabelWithUnit(measuresById[rightId]),
color: fields.y.colorMapping[rightId],
color: fields.color.colorMapping[rightId],
getY: (d) => (d[rightId] !== null ? Number(d[rightId]) : null),
getMinY: (data) => {
const minY =
Expand Down
2 changes: 1 addition & 1 deletion app/charts/combo/combo-line-single-state-props.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export const useComboLineSingleStateVariables = (
dimension: measuresById[id],
id,
label: measuresById[id].label,
color: fields.y.colorMapping[id],
color: fields.color.colorMapping[id],
getY: (d) => (d[id] !== null ? Number(d[id]) : null),
getMinY: (data) => {
const minY =
Expand Down
5 changes: 5 additions & 0 deletions app/charts/index.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,11 @@ describe("chart type switch", () => {
y: {
componentId: "https://environment.ld.admin.ch/foen/ubd0104/value",
},
color: {
type: "segment",
paletteId: "category10",
colorMapping: {},
},
},
interactiveFiltersConfig: {
legend: {
Expand Down
Loading
Loading