EES-5542 moved geoJson fetching on dataset selection into parent comp…

…onents with callbacks
dfe-analytical-services · Dec 12, 2024 · 62dc8da · 62dc8da
1 parent 3043ec4
commit 62dc8da
Show file tree

Hide file tree

Showing 14 changed files with 156 additions and 213 deletions.
diff --git a/...on-statistics-admin/src/pages/release/datablocks/components/DataBlockPageReadOnlyTabs.tsx b/...on-statistics-admin/src/pages/release/datablocks/components/DataBlockPageReadOnlyTabs.tsx
@@ -76,8 +76,6 @@ const DataBlockPageReadOnlyTabs = ({ releaseId, dataBlock }: Props) => {
                   return (
                     <ChartRenderer
                       {...chart}
-                      releaseId={releaseId}
-                      dataBlockParentId={dataBlock.dataBlockParentId}
                       key={key}
                       id={`dataBlockTabs-chart-${index}`}
                       axes={axes}

diff --git a/...-admin/src/pages/release/datablocks/components/chart/ChartBoundaryLevelsConfiguration.tsx b/...-admin/src/pages/release/datablocks/components/chart/ChartBoundaryLevelsConfiguration.tsx
@@ -22,7 +22,7 @@ interface Props {
 export default function ChartBoundaryLevelsConfiguration({
   buttons,
   // TODO: EES-5402 - Remove when all boundary level changes are done
-  hasDataSetBoundaryLevels = false,
+  hasDataSetBoundaryLevels = true,
   map,
   meta,
   options,

diff --git a/...education-statistics-admin/src/pages/release/datablocks/components/chart/ChartBuilder.tsx b/...education-statistics-admin/src/pages/release/datablocks/components/chart/ChartBuilder.tsx
@@ -100,7 +100,6 @@ interface Props {
   data: TableDataResult[];
   meta: FullTableMeta;
   releaseId: string;
-  dataBlockParentId: DataBlock['dataBlockParentId'];
   initialChart?: Chart;
   tableTitle: string;
   onChartSave: (chart: Chart, file?: File) => Promise<void>;
@@ -112,7 +111,6 @@ export default function ChartBuilder({
   data,
   meta,
   releaseId,
-  dataBlockParentId,
   initialChart,
   tableTitle,
   onChartSave,
@@ -200,8 +198,8 @@ export default function ChartBuilder({
           },
           boundaryLevel: options.boundaryLevel ?? 0,
           type: 'map',
-          releaseId,
-          dataBlockParentId,
+          onBoundaryLevelChange: boundaryLevel =>
+            onTableQueryUpdate({ boundaryLevel }),
         };
       default:
         return undefined;
@@ -215,8 +213,7 @@ export default function ChartBuilder({
     meta,
     options,
     map,
-    releaseId,
-    dataBlockParentId,
+    onTableQueryUpdate,
   ]);
 
   const handleSubmit = useCallback(async () => {

diff --git a/src/explore-education-statistics-common/src/modules/charts/components/ChartRenderer.tsx b/src/explore-education-statistics-common/src/modules/charts/components/ChartRenderer.tsx
@@ -18,8 +18,6 @@ import React, { memo, useMemo } from 'react';
 
 export type ChartRendererProps = {
   source?: string;
-  releaseId?: string;
-  dataBlockParentId?: string;
 } & (
   | ({
       type: 'map';

diff --git a/src/explore-education-statistics-common/src/modules/charts/components/MapBlock.module.scss b/src/explore-education-statistics-common/src/modules/charts/components/MapBlock.module.scss
@@ -8,6 +8,10 @@
   }
 }
 
+.mapLoading {
+  opacity: 0.5;
+}
+
 .tooltip {
   white-space: normal;
   width: 100%;

diff --git a/src/explore-education-statistics-common/src/modules/charts/components/MapBlock.tsx b/src/explore-education-statistics-common/src/modules/charts/components/MapBlock.tsx
@@ -21,9 +21,7 @@ import { LegendConfiguration } from '@common/modules/charts/types/legend';
 import getMapDataSetCategoryConfigs, {
   MapDataSetCategoryConfig,
 } from '@common/modules/charts/util/getMapDataSetCategoryConfigs';
-import tableBuilderService, {
-  GeoJsonFeatureProperties,
-} from '@common/services/tableBuilderService';
+import { GeoJsonFeatureProperties } from '@common/services/tableBuilderService';
 import { Dictionary } from '@common/types';
 import naturalOrderBy from '@common/utils/array/naturalOrderBy';
 import classNames from 'classnames';
@@ -32,9 +30,8 @@ import { Layer, Path, Polyline } from 'leaflet';
 import keyBy from 'lodash/keyBy';
 import React, { useCallback, useEffect, useMemo, useState } from 'react';
 import { MapContainer } from 'react-leaflet';
-import { useQuery, useQueryClient } from '@tanstack/react-query';
-import { LocationFilter } from '@common/modules/table-tool/types/filters';
-import { mapFullTableMetaLocations } from '@common/modules/table-tool/utils/mapFullTableMeta';
+import useToggle from '@common/hooks/useToggle';
+import LoadingSpinner from '@common/components/LoadingSpinner';
 
 export interface MapFeatureProperties extends GeoJsonFeatureProperties {
   colour: string;
@@ -64,8 +61,7 @@ export interface MapBlockProps extends ChartProps {
   map?: MapConfig;
   position?: { lat: number; lng: number };
   boundaryLevel: number;
-  releaseId: string;
-  dataBlockParentId: string;
+  onBoundaryLevelChange: (boundaryLevel: number) => void;
 }
 
 export const mapBlockDefinition: ChartDefinition = {
@@ -130,43 +126,10 @@ export default function MapBlock({
   axes,
   title,
   boundaryLevel,
-  releaseId,
-  dataBlockParentId,
+  onBoundaryLevelChange,
 }: MapBlockProps) {
-  const [selectedBoundaryLevel, setSelectedBoundaryLevel] =
-    useState<number>(boundaryLevel);
-
-  const queryClient = useQueryClient();
-  // add existing geoJson to cache, avoiding double fetching
-  queryClient.setQueryData(
-    ['mapLocationGeoJson', releaseId, dataBlockParentId, boundaryLevel],
-    meta.locations,
-  );
-
-  const { data: locations, isFetching: isFetchingGeoJson } = useQuery<
-    LocationFilter[]
-  >({
-    queryKey: [
-      'mapLocationGeoJson',
-      releaseId,
-      dataBlockParentId,
-      selectedBoundaryLevel,
-    ],
-    queryFn: async () => {
-      return mapFullTableMetaLocations(
-        await tableBuilderService.getLocationGeoJson(
-          releaseId,
-          dataBlockParentId,
-          selectedBoundaryLevel,
-        ),
-      );
-    },
-    staleTime: Infinity,
-    cacheTime: Infinity,
-    keepPreviousData: true,
-    refetchOnWindowFocus: false,
-    refetchOnMount: false,
-  });
+  const [isBoundaryLevelChanging, setIsBoundaryLevelChanging] =
+    useToggle(false);
 
   const axisMajor = useMemo<AxisConfiguration>(
     () => ({
@@ -178,13 +141,8 @@ export default function MapBlock({
   );
 
   const dataSetCategories = useMemo<MapDataSetCategory[]>(() => {
-    return createMapDataSetCategories(
-      axisMajor,
-      data,
-      meta,
-      locations ?? meta.locations,
-    );
-  }, [axisMajor, data, meta, locations]);
+    return createMapDataSetCategories(axisMajor, data, meta);
+  }, [axisMajor, data, meta]);
 
   const dataSetCategoryConfigs = useMemo<Dictionary<MapDataSetCategoryConfig>>(
     () =>
@@ -258,16 +216,31 @@ export default function MapBlock({
     (value: string) => {
       setSelectedFeature(features?.features.find(feat => feat.id === value));
     },
-    [setSelectedFeature, features],
+    [features],
   );
+
   const handleDataSetChange = useCallback(
-    (value: string) => {
+    async (value: string) => {
       setSelectedDataSetKey(value);
-      setSelectedBoundaryLevel(
-        dataSetCategoryConfigs[value].boundaryLevel ?? boundaryLevel,
-      );
+      if (
+        dataSetCategoryConfigs[value].boundaryLevel ??
+        boundaryLevel !== selectedDataSetConfig.boundaryLevel ??
+        boundaryLevel
+      ) {
+        setIsBoundaryLevelChanging.on();
+        await onBoundaryLevelChange(
+          dataSetCategoryConfigs[value].boundaryLevel ?? boundaryLevel,
+        );
+        setIsBoundaryLevelChanging.off();
+      }
     },
-    [dataSetCategoryConfigs, boundaryLevel],
+    [
+      dataSetCategoryConfigs,
+      boundaryLevel,
+      onBoundaryLevelChange,
+      setIsBoundaryLevelChanging,
+      selectedDataSetConfig,
+    ],
   );
 
   if (
@@ -298,11 +271,21 @@ export default function MapBlock({
               width: (width && `${width}px`) || '100%',
               height: `${height || 600}px`,
             }}
-            className={classNames(styles.map, 'dfe-print-break-avoid')}
+            className={classNames(styles.map, 'dfe-print-break-avoid', {
+              [styles.mapLoading]: isBoundaryLevelChanging,
+            })}
             center={position}
             minZoom={5}
             zoom={5}
           >
+            <LoadingSpinner
+              className="govuk-!-margin-top-8"
+              loading={isBoundaryLevelChanging}
+              text="Loading map"
+              size="lg"
+              hideText
+              alert
+            />
             <MapGeoJSON
               dataSetCategoryConfigs={dataSetCategoryConfigs}
               features={features}
@@ -311,7 +294,6 @@ export default function MapBlock({
               selectedDataSetKey={selectedDataSetKey}
               selectedFeature={selectedFeature}
               onSelectFeature={setSelectedFeature}
-              isLoading={isFetchingGeoJson}
             />
           </MapContainer>
         </div>

diff --git a/src/explore-education-statistics-common/src/modules/charts/components/MapGeoJSON.tsx b/src/explore-education-statistics-common/src/modules/charts/components/MapGeoJSON.tsx
@@ -13,7 +13,6 @@ import { FeatureCollection } from 'geojson';
 import Leaflet, { Layer, PathOptions } from 'leaflet';
 import React, { useEffect, useRef, useState } from 'react';
 import { GeoJSON, useMap } from 'react-leaflet';
-import LoadingSpinner from '@common/components/LoadingSpinner';
 
 interface Props {
   dataSetCategoryConfigs: Dictionary<MapDataSetCategoryConfig>;
@@ -23,7 +22,6 @@ interface Props {
   height: number;
   width?: number;
   onSelectFeature: (feature: MapFeature) => void;
-  isLoading: boolean;
 }
 
 export default function MapGeoJSON({
@@ -34,7 +32,6 @@ export default function MapGeoJSON({
   selectedDataSetKey,
   dataSetCategoryConfigs,
   onSelectFeature,
-  isLoading = false,
 }: Props) {
   const map = useMap();
   const container = useRef<HTMLDivElement>(null);
@@ -168,21 +165,15 @@ export default function MapGeoJSON({
             return {
               color: '#cfdce3',
               fillColor: '#003078',
-              fillOpacity: isLoading ? 0.05 : 0.1,
+              fillOpacity: 0.1,
               stroke: false,
               weight: 1,
             };
           }}
           ref={ukRef}
         />
       )}
-      <LoadingSpinner
-        className="govuk-!-margin-top-8"
-        loading={isLoading}
-        text="fetching geometry for data selection"
-        size="lg"
-        hideText
-      />
+
       {features && (
         <GeoJSON
           ref={geometryRef}

diff --git a/...ducation-statistics-common/src/modules/charts/components/__tests__/ChartRenderer.test.tsx b/...ducation-statistics-common/src/modules/charts/components/__tests__/ChartRenderer.test.tsx
@@ -41,8 +41,6 @@ describe('ChartRenderer', () => {
   const testFullTableMeta = mapFullTable(testChartTableData);
   const testMapChartRenderer: ChartRendererProps = {
     type: 'map',
-    releaseId: 'releaseId',
-    dataBlockParentId: 'dataBlockParentId',
     meta: testFullTableMeta.subjectMeta,
     data: testFullTableMeta.results,
     alt: '',
@@ -55,6 +53,7 @@ describe('ChartRenderer', () => {
     },
     boundaryLevel: 1,
     map: { dataSetConfigs: [] },
+    onBoundaryLevelChange: () => {},
   };
 
   test('renders auto-generated boundary level footnote successfully', async () => {