Merge pull request #807 from visualize-admin/fix/missing-loading-stat…

…e-configurator

fix: Crashing of charts when selecting component "too fast"

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>((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 = ({
     <Select
       key={`select-${field}-dimension`}
       id={field}
-      label={optional ? `${label} (${optionalLabel})` : label}
-      disabled={disabled}
+      label={
+        <>
+          {optional ? (
+            <span>
+              {label} ({optionalLabel})
+            </span>
+          ) : (
+            <span>{label}</span>
+          )}
+          {fetching ? (
+            <CircularProgress size={12} className={classes.loadingIndicator} />
+          ) : null}
+        </>
+      }
+      disabled={disabled || fetching}
       options={
         optional
           ? [

app/configurator/config-form.tsx

@@ -5,7 +5,10 @@ import React, {
   InputHTMLAttributes,
   useCallback,
   useContext,
+  useEffect,
   useMemo,
+  useRef,
+  useState,
 } from "react";
 import { useClient } from "urql";
 
@@ -65,16 +68,28 @@ export const useChartFieldField = ({
   field,
 }: {
   field: string;
-}): SelectProps => {
+}): SelectProps & {
+  fetching: boolean;
+} => {
+  const unmountedRef = useRef(false);
+  const [fetching, setFetching] = useState(false);
   const [state, dispatch] = useConfiguratorState();
   const client = useClient();
   const locale = useLocale();
 
+  useEffect(() => {
+    return () => {
+      unmountedRef.current = true;
+    };
+  }, []);
+
   const onChange = useEvent(async (e: SelectChangeEvent<unknown>) => {
     if (!state.dataSet) {
       return;
     }
+
     if (e.target.value !== FIELD_VALUE_NONE) {
+      setFetching(true);
       const dimensionIri = e.target.value as string;
       const { data: hierarchyData } = await client
         .query<DimensionHierarchyQuery, DimensionHierarchyQueryVariables>(
@@ -90,19 +105,20 @@ export const useChartFieldField = ({
         .toPromise();
       const tree = hierarchyData?.dataCubeByIri?.dimensionByIri
         ?.hierarchy as HierarchyValue[];
-
-      // If the dimension has a hierarchy, we select leaves
       const leaves = getLeaves(tree);
 
-      dispatch({
-        type: "CHART_FIELD_CHANGED",
-        value: {
-          locale,
-          field,
-          componentIri: dimensionIri,
-          selectedValues: leaves,
-        },
-      });
+      if (!unmountedRef.current) {
+        dispatch({
+          type: "CHART_FIELD_CHANGED",
+          value: {
+            locale,
+            field,
+            componentIri: dimensionIri,
+            selectedValues: leaves,
+          },
+        });
+        setFetching(false);
+      }
     } else {
       dispatch({
         type: "CHART_FIELD_DELETED",
@@ -124,6 +140,7 @@ export const useChartFieldField = ({
     name: field,
     value,
     onChange,
+    fetching,
   };
 };
 

app/configurator/configurator-state.spec.tsx

@@ -18,6 +18,7 @@ import {
   deriveFiltersFromFields,
   getFiltersByMappingStatus,
   getLocalStorageKey,
+  handleChartFieldChanged,
   handleChartOptionChanged,
   initChartStateFromChart,
   initChartStateFromCube,
@@ -40,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();
 });
@@ -690,20 +724,53 @@ describe("colorMapping", () => {
   });
 });
 
-describe("handleChartOptionChanged", () => {
-  jest.mock("@/graphql/client", () => ({
-    readQuery: () => {
-      return {
-        dimensions: [
-          {
-            iri: "newAreaLayerColorIri",
-            values: [{ value: "orange", label: "orange" }],
+describe("handleChartFieldChanged", () => {
+  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", () => {
   it("should reset previous color filters", () => {
     const state = {
       state: "CONFIGURING_CHART",
@@ -713,6 +780,7 @@ describe("handleChartOptionChanged", () => {
         url: "fakeUrl",
       },
       chartConfig: {
+        chartType: "map",
         fields: {
           areaLayer: {
             componentIri: "areaLayerIri",