From e44cfd815b49dd3ccb7ddd55e5931d2e558a4a0d Mon Sep 17 00:00:00 2001 From: Bartosz Prusinowski Date: Tue, 25 Oct 2022 10:48:43 +0200 Subject: [PATCH 1/4] perf: Only compute colorMapping when it's needed --- app/configurator/configurator-state.tsx | 27 ++++++++++--------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/app/configurator/configurator-state.tsx b/app/configurator/configurator-state.tsx index f6ebc00ff..8f549b736 100644 --- a/app/configurator/configurator-state.tsx +++ b/app/configurator/configurator-state.tsx @@ -727,16 +727,13 @@ const handleChartFieldChanged = ( // optionalFields = ['segment', 'areaLayer', 'symbolLayer'], // should be reflected in chart encodings if (field === "segment") { - const colorMapping = - component?.values && - mapValueIrisToColor({ - palette: DEFAULT_PALETTE, - dimensionValues: component?.values, - }); - // FIXME: This should be more chart specific // (no "stacked" for scatterplots for instance) if (isSegmentInConfig(draft.chartConfig)) { + const colorMapping = mapValueIrisToColor({ + palette: DEFAULT_PALETTE, + dimensionValues: component?.values || [], + }); draft.chartConfig.filters[componentIri] = { type: "multi", values: Object.fromEntries( @@ -744,12 +741,12 @@ const handleChartFieldChanged = ( ), }; draft.chartConfig.fields.segment = { - componentIri: componentIri, + componentIri, palette: DEFAULT_PALETTE, // Type exists only within column charts. ...(isColumnConfig(draft.chartConfig) && { type: "stacked" }), sorting: DEFAULT_SORTING, - colorMapping: colorMapping, + colorMapping, }; } @@ -785,12 +782,10 @@ const handleChartFieldChanged = ( draft.chartConfig.fields.segment && "palette" in draft.chartConfig.fields.segment ) { - const colorMapping = - component && - mapValueIrisToColor({ - palette: draft.chartConfig.fields.segment.palette || DEFAULT_PALETTE, - dimensionValues: component?.values, - }); + const colorMapping = mapValueIrisToColor({ + palette: draft.chartConfig.fields.segment.palette || DEFAULT_PALETTE, + dimensionValues: component?.values || [], + }); draft.chartConfig.fields.segment.componentIri = componentIri; draft.chartConfig.fields.segment.colorMapping = colorMapping; @@ -803,7 +798,7 @@ const handleChartFieldChanged = ( } else { // Reset other field options (draft.chartConfig.fields as GenericFields)[field] = { - componentIri: componentIri, + componentIri, }; // if x !== time, also deactivate interactive time filter if ( From 4cfe9dca9c7619868db546ebaeed3e692891a7bd Mon Sep 17 00:00:00 2001 From: Bartosz Prusinowski Date: Wed, 26 Oct 2022 13:09:02 +0200 Subject: [PATCH 2/4] fix: Populate map fields when they are being updated --- app/configurator/configurator-state.spec.tsx | 69 ++++++++++++++++++++ app/configurator/configurator-state.tsx | 69 +++++++++++++++----- 2 files changed, 121 insertions(+), 17 deletions(-) diff --git a/app/configurator/configurator-state.spec.tsx b/app/configurator/configurator-state.spec.tsx index 6420e5928..59544ba29 100644 --- a/app/configurator/configurator-state.spec.tsx +++ b/app/configurator/configurator-state.spec.tsx @@ -18,6 +18,7 @@ import { deriveFiltersFromFields, getFiltersByMappingStatus, getLocalStorageKey, + handleChartFieldChanged, handleChartOptionChanged, initChartStateFromChart, initChartStateFromCube, @@ -690,6 +691,69 @@ describe("colorMapping", () => { }); }); +describe("handleChartFieldChanged", () => { + jest.mock("@/graphql/client", () => ({ + readQuery: () => { + return { + dimensions: [ + { + iri: "newAreaLayerColorIri", + values: [{ value: "orange", label: "orange" }], + }, + { + iri: "symbolLayerIri", + values: [{ value: "x", label: "y" }], + }, + ], + }; + }, + })); + + it("should not reset symbol layer when it's being updated", () => { + const state = { + state: "CONFIGURING_CHART", + dataSet: "mapDataset", + dataSource: { + type: "sparql", + url: "fakeUrl", + }, + chartConfig: { + chartType: "map", + fields: { + symbolLayer: { + componentIri: "symbolLayerIri", + color: { + type: "categorical", + componentIri: "symbolLayerColorIri", + palette: "oranges", + colorMapping: { + red: "green", + green: "blue", + blue: "red", + }, + }, + }, + }, + filters: {}, + }, + }; + + handleChartFieldChanged( + state as unknown as ConfiguratorStateConfiguringChart, + { + type: "CHART_FIELD_CHANGED", + value: { + locale: "en", + field: "symbolLayer", + componentIri: "symbolLayerIri", + }, + } + ); + + expect(state.chartConfig.fields.symbolLayer.color).toBeDefined(); + }); +}); + describe("handleChartOptionChanged", () => { jest.mock("@/graphql/client", () => ({ readQuery: () => { @@ -699,6 +763,10 @@ describe("handleChartOptionChanged", () => { iri: "newAreaLayerColorIri", values: [{ value: "orange", label: "orange" }], }, + { + iri: "symbolLayerIri", + values: [{ value: "x", label: "y" }], + }, ], }; }, @@ -713,6 +781,7 @@ describe("handleChartOptionChanged", () => { url: "fakeUrl", }, chartConfig: { + chartType: "map", fields: { areaLayer: { componentIri: "areaLayerIri", diff --git a/app/configurator/configurator-state.tsx b/app/configurator/configurator-state.tsx index a762d72a8..6c6b758b5 100644 --- a/app/configurator/configurator-state.tsx +++ b/app/configurator/configurator-state.tsx @@ -41,6 +41,8 @@ import { isColumnConfig, isMapConfig, isSegmentInConfig, + MapConfig, + MapFields, NumericalColorField, } from "@/configurator/config-types"; import { @@ -71,6 +73,8 @@ import { DataCubeMetadataWithComponentValuesQuery, DataCubeMetadataWithComponentValuesQueryVariables, DimensionMetadataFragment, + NumericalMeasure, + OrdinalMeasure, } from "@/graphql/query-hooks"; import { DataCubeMetadata } from "@/graphql/types"; import { Locale } from "@/locales/locales"; @@ -728,7 +732,37 @@ export const getChartOptionField = ( ); }; -const handleChartFieldChanged = ( +const initializeMapField = ({ + chartConfig, + field, + componentIri, + dimensions, + measures, +}: { + chartConfig: MapConfig; + field: "areaLayer" | "symbolLayer"; + componentIri: string; + dimensions: DimensionMetadataFragment[]; + measures: (NumericalMeasure | OrdinalMeasure)[]; +}) => { + if (field === "areaLayer") { + chartConfig.fields.areaLayer = getInitialAreaLayer({ + component: dimensions + .filter(isGeoShapesDimension) + .find((d) => d.iri === componentIri)!, + measure: measures[0], + }); + } else if (field === "symbolLayer") { + chartConfig.fields.symbolLayer = getInitialSymbolLayer({ + component: dimensions + .filter(isGeoDimension) + .find((d) => d.iri === componentIri)!, + measure: measures.filter(isNumericalMeasure)[0], + }); + } +}; + +export const handleChartFieldChanged = ( draft: ConfiguratorState, action: Extract ) => { @@ -793,21 +827,13 @@ const handleChartFieldChanged = ( ); } } else if (isMapConfig(draft.chartConfig)) { - if (field === "areaLayer") { - draft.chartConfig.fields.areaLayer = getInitialAreaLayer({ - component: dimensions - .filter(isGeoShapesDimension) - .find((d) => d.iri === componentIri)!, - measure: measures[0], - }); - } else if (field === "symbolLayer") { - draft.chartConfig.fields.symbolLayer = getInitialSymbolLayer({ - component: dimensions - .filter(isGeoDimension) - .find((d) => d.iri === componentIri)!, - measure: measures.filter(isNumericalMeasure)[0], - }); - } + initializeMapField({ + chartConfig: draft.chartConfig, + field: field as keyof MapFields, + componentIri, + dimensions, + measures, + }); } } else { // The field is being updated @@ -831,10 +857,11 @@ const handleChartFieldChanged = ( ), }; } else { - // Reset other field options + // Reset field properties, excluding componentIri. (draft.chartConfig.fields as GenericFields)[field] = { componentIri, }; + // if x !== time, also deactivate interactive time filter if ( isColumnConfig(draft.chartConfig) && @@ -848,6 +875,14 @@ const handleChartFieldChanged = ( false, Object ); + } else if (isMapConfig(draft.chartConfig)) { + initializeMapField({ + chartConfig: draft.chartConfig, + field: field as keyof MapFields, + componentIri, + dimensions, + measures, + }); } } From 008dba9d5f50ab31a25a21fdc978b2f5c2103881 Mon Sep 17 00:00:00 2001 From: Bartosz Prusinowski Date: Wed, 26 Oct 2022 13:09:27 +0200 Subject: [PATCH 3/4] fix: Mocking of GQL client --- app/configurator/configurator-state.spec.tsx | 67 ++++++++++---------- 1 file changed, 33 insertions(+), 34 deletions(-) diff --git a/app/configurator/configurator-state.spec.tsx b/app/configurator/configurator-state.spec.tsx index 59544ba29..d991545da 100644 --- a/app/configurator/configurator-state.spec.tsx +++ b/app/configurator/configurator-state.spec.tsx @@ -41,6 +41,39 @@ jest.mock("@/utils/chart-config/exchange", () => ({ fetchChartConfig: jest.fn(), })); +jest.mock("@/graphql/client", () => { + return { + client: { + readQuery: () => { + return { + data: { + dataCubeByIri: { + dimensions: [ + { + __typename: "GeoShapesDimension", + iri: "newAreaLayerColorIri", + values: [{ value: "orange", label: "orange" }], + }, + { + __typename: "GeoCoordinatesDimension", + iri: "symbolLayerIri", + values: [{ value: "x", label: "y" }], + }, + ], + measures: [ + { + __typename: "NumericalMeasure", + iri: "measure", + }, + ], + }, + }, + }; + }, + }, + }; +}); + afterEach(() => { jest.restoreAllMocks(); }); @@ -692,23 +725,6 @@ describe("colorMapping", () => { }); describe("handleChartFieldChanged", () => { - jest.mock("@/graphql/client", () => ({ - readQuery: () => { - return { - dimensions: [ - { - iri: "newAreaLayerColorIri", - values: [{ value: "orange", label: "orange" }], - }, - { - iri: "symbolLayerIri", - values: [{ value: "x", label: "y" }], - }, - ], - }; - }, - })); - it("should not reset symbol layer when it's being updated", () => { const state = { state: "CONFIGURING_CHART", @@ -755,23 +771,6 @@ describe("handleChartFieldChanged", () => { }); describe("handleChartOptionChanged", () => { - jest.mock("@/graphql/client", () => ({ - readQuery: () => { - return { - dimensions: [ - { - iri: "newAreaLayerColorIri", - values: [{ value: "orange", label: "orange" }], - }, - { - iri: "symbolLayerIri", - values: [{ value: "x", label: "y" }], - }, - ], - }; - }, - })); - it("should reset previous color filters", () => { const state = { state: "CONFIGURING_CHART", From 782a3316f6cab4c6e5d47da186e067a91aadfb63 Mon Sep 17 00:00:00 2001 From: Bartosz Prusinowski Date: Thu, 27 Oct 2022 09:59:14 +0200 Subject: [PATCH 4/4] feat: Disable chart field select element when hierarchy is being fetched --- app/configurator/components/field.tsx | 31 ++++++++++++++++++-- app/configurator/config-form.tsx | 41 +++++++++++++++++++-------- 2 files changed, 57 insertions(+), 15 deletions(-) diff --git a/app/configurator/components/field.tsx b/app/configurator/components/field.tsx index 51c7839c4..968dc0aa6 100644 --- a/app/configurator/components/field.tsx +++ b/app/configurator/components/field.tsx @@ -1,4 +1,6 @@ import { t } from "@lingui/macro"; +import { CircularProgress, Theme } from "@mui/material"; +import { makeStyles } from "@mui/styles"; import { extent, timeFormat, TimeLocaleObject, timeParse } from "d3"; import get from "lodash/get"; import { ChangeEvent, ReactNode, useCallback, useMemo, useState } from "react"; @@ -53,6 +55,15 @@ import { IconName } from "@/icons"; import { useLocale } from "@/locales/use-locale"; import { getPalette } from "@/palettes"; +const useStyles = makeStyles((theme) => ({ + loadingIndicator: { + color: theme.palette.grey[700], + display: "inline-block", + marginTop: theme.spacing(1), + marginLeft: theme.spacing(2), + }, +})); + export const ControlTabField = ({ component, value, @@ -625,7 +636,8 @@ export const ChartFieldField = ({ optional?: boolean; disabled?: boolean; }) => { - const fieldProps = useChartFieldField({ field }); + const classes = useStyles(); + const { fetching, ...fieldProps } = useChartFieldField({ field }); const noneLabel = t({ id: "controls.dimension.none", @@ -641,8 +653,21 @@ export const ChartFieldField = ({