From 7cc09c5490e3bc87d36d63de86aaea1e40536c9b Mon Sep 17 00:00:00 2001 From: Bartosz Prusinowski Date: Fri, 28 Apr 2023 15:19:47 +0200 Subject: [PATCH 1/3] refactor: Pass dimension iri and values instead of whole dimension --- app/charts/area/areas-state.tsx | 3 ++- app/charts/column/columns-grouped-state.tsx | 3 ++- app/charts/column/columns-stacked-state.tsx | 3 ++- app/charts/column/columns-state.tsx | 3 ++- app/charts/line/lines-state.tsx | 3 ++- app/charts/pie/pie-state.tsx | 3 ++- app/charts/scatterplot/scatterplot-state.tsx | 3 ++- app/charts/shared/use-abbreviations.ts | 20 +++++++++++--------- 8 files changed, 25 insertions(+), 16 deletions(-) diff --git a/app/charts/area/areas-state.tsx b/app/charts/area/areas-state.tsx index fda2c734d..55fbf27db 100644 --- a/app/charts/area/areas-state.tsx +++ b/app/charts/area/areas-state.tsx @@ -115,7 +115,8 @@ const useAreasState = ( abbreviationOrLabelLookup: segmentsByAbbreviationOrLabel, } = useMaybeAbbreviations({ useAbbreviations: fields.segment?.useAbbreviations, - dimension: segmentDimension, + dimensionIri: segmentDimension?.iri, + dimensionValues: segmentDimension?.values, }); const { getValue: getSegment, getLabel: getSegmentLabel } = diff --git a/app/charts/column/columns-grouped-state.tsx b/app/charts/column/columns-grouped-state.tsx index 0d4083774..9c55e5508 100644 --- a/app/charts/column/columns-grouped-state.tsx +++ b/app/charts/column/columns-grouped-state.tsx @@ -109,7 +109,8 @@ const useGroupedColumnsState = ( const { getAbbreviationOrLabelByValue: getXAbbreviationOrLabel } = useMaybeAbbreviations({ useAbbreviations: fields.x.useAbbreviations, - dimension: xDimension, + dimensionIri: xDimension.iri, + dimensionValues: xDimension.values, }); const { getValue: getX, getLabel: getXLabel } = useObservationLabels( diff --git a/app/charts/column/columns-stacked-state.tsx b/app/charts/column/columns-stacked-state.tsx index 26e24f4a1..2f7ed0cdb 100644 --- a/app/charts/column/columns-stacked-state.tsx +++ b/app/charts/column/columns-stacked-state.tsx @@ -112,7 +112,8 @@ const useColumnsStackedState = ( const { getAbbreviationOrLabelByValue: getXAbbreviationOrLabel } = useMaybeAbbreviations({ useAbbreviations: fields.x.useAbbreviations, - dimension: xDimension, + dimensionIri: xDimension.iri, + dimensionValues: xDimension.values, }); const { getValue: getX, getLabel: getXLabel } = useObservationLabels( diff --git a/app/charts/column/columns-state.tsx b/app/charts/column/columns-state.tsx index 30b3eb3bd..7cb4ccd3b 100644 --- a/app/charts/column/columns-state.tsx +++ b/app/charts/column/columns-state.tsx @@ -116,7 +116,8 @@ const useColumnsState = ( const { getAbbreviationOrLabelByValue: getXAbbreviationOrLabel } = useMaybeAbbreviations({ useAbbreviations: fields.x.useAbbreviations, - dimension: xDimension, + dimensionIri: xDimension.iri, + dimensionValues: xDimension.values, }); const { getValue: getX, getLabel: getXLabel } = useObservationLabels( diff --git a/app/charts/line/lines-state.tsx b/app/charts/line/lines-state.tsx index c0f04dae3..cf66c2e8b 100644 --- a/app/charts/line/lines-state.tsx +++ b/app/charts/line/lines-state.tsx @@ -111,7 +111,8 @@ const useLinesState = ( abbreviationOrLabelLookup: segmentsByAbbreviationOrLabel, } = useMaybeAbbreviations({ useAbbreviations: fields.segment?.useAbbreviations, - dimension: segmentDimension, + dimensionIri: segmentDimension?.iri, + dimensionValues: segmentDimension?.values, }); const { getValue: getSegment, getLabel: getSegmentLabel } = diff --git a/app/charts/pie/pie-state.tsx b/app/charts/pie/pie-state.tsx index cc9dd0d2d..d472dc37e 100644 --- a/app/charts/pie/pie-state.tsx +++ b/app/charts/pie/pie-state.tsx @@ -72,7 +72,8 @@ const usePieState = ( abbreviationOrLabelLookup: segmentsByAbbreviationOrLabel, } = useMaybeAbbreviations({ useAbbreviations: fields.segment?.useAbbreviations, - dimension: segmentDimension, + dimensionIri: segmentDimension?.iri, + dimensionValues: segmentDimension?.values, }); const { getValue: getSegment, getLabel: getSegmentLabel } = diff --git a/app/charts/scatterplot/scatterplot-state.tsx b/app/charts/scatterplot/scatterplot-state.tsx index 71aa26be3..0f52280aa 100644 --- a/app/charts/scatterplot/scatterplot-state.tsx +++ b/app/charts/scatterplot/scatterplot-state.tsx @@ -76,7 +76,8 @@ const useScatterplotState = ({ abbreviationOrLabelLookup: segmentsByAbbreviationOrLabel, } = useMaybeAbbreviations({ useAbbreviations: fields.segment?.useAbbreviations, - dimension: segmentDimension, + dimensionIri: segmentDimension?.iri, + dimensionValues: segmentDimension?.values, }); const { getValue: getSegment, getLabel: getSegmentLabel } = diff --git a/app/charts/shared/use-abbreviations.ts b/app/charts/shared/use-abbreviations.ts index 3a77ff858..a9b6c6a76 100644 --- a/app/charts/shared/use-abbreviations.ts +++ b/app/charts/shared/use-abbreviations.ts @@ -1,18 +1,19 @@ import React from "react"; import { DimensionValue, Observation, ObservationValue } from "@/domain/data"; -import { DimensionMetadataFragment } from "@/graphql/query-hooks"; export const useMaybeAbbreviations = ({ useAbbreviations, - dimension, + dimensionIri, + dimensionValues, }: { useAbbreviations: boolean | undefined; - dimension: DimensionMetadataFragment | undefined; + dimensionIri: string | undefined; + dimensionValues: DimensionValue[] | undefined; }) => { const { valueLookup, labelLookup, abbreviationOrLabelLookup } = React.useMemo(() => { - const values = dimension?.values ?? []; + const values = dimensionValues ?? []; const valueLookup = new Map< NonNullable, @@ -37,16 +38,16 @@ export const useMaybeAbbreviations = ({ labelLookup, abbreviationOrLabelLookup, }; - }, [dimension?.values, useAbbreviations]); + }, [dimensionValues, useAbbreviations]); const getAbbreviationOrLabelByValue = React.useCallback( (d: Observation) => { - if (!dimension) { + if (!dimensionIri) { return ""; } - const value = d[`${dimension.iri}/__iri__`] as string | undefined; - const label = d[dimension.iri] as string | undefined; + const value = d[`${dimensionIri}/__iri__`] as string | undefined; + const label = d[dimensionIri] as string | undefined; if (value === undefined && label === undefined) { return ""; @@ -55,13 +56,14 @@ export const useMaybeAbbreviations = ({ const lookedUpObservation = (value ? valueLookup.get(value) : null) ?? (label ? labelLookup.get(label) : null); + const lookedUpLabel = lookedUpObservation?.label ?? ""; return useAbbreviations ? lookedUpObservation?.alternateName ?? lookedUpLabel : lookedUpLabel; }, - [dimension, valueLookup, labelLookup, useAbbreviations] + [dimensionIri, valueLookup, labelLookup, useAbbreviations] ); return { From 200622a0247282739acd2e8be1613aa8a7b2a0c6 Mon Sep 17 00:00:00 2001 From: Bartosz Prusinowski Date: Fri, 28 Apr 2023 15:30:18 +0200 Subject: [PATCH 2/3] fix: Undefined X axis values (column charts) --- app/charts/column/columns-grouped-state.tsx | 7 ++- app/charts/column/columns-stacked-state.tsx | 7 ++- app/charts/column/columns-state.tsx | 4 +- app/charts/shared/chart-helpers.spec.tsx | 50 +++++++++++++++++++++ app/charts/shared/chart-helpers.tsx | 27 ++++++++++- 5 files changed, 89 insertions(+), 6 deletions(-) diff --git a/app/charts/column/columns-grouped-state.tsx b/app/charts/column/columns-grouped-state.tsx index 9c55e5508..ca18d073c 100644 --- a/app/charts/column/columns-grouped-state.tsx +++ b/app/charts/column/columns-grouped-state.tsx @@ -29,6 +29,7 @@ import { import { getLabelWithUnit, useDataAfterInteractiveFilters, + useMaybeTemporalDimensionValues, useOptionalNumericVariable, usePlottableData, useTemporalVariable, @@ -105,12 +106,13 @@ const useGroupedColumnsState = ( } const xIsTime = isTemporalDimension(xDimension); + const xDimensionValues = useMaybeTemporalDimensionValues(xDimension, data); const { getAbbreviationOrLabelByValue: getXAbbreviationOrLabel } = useMaybeAbbreviations({ useAbbreviations: fields.x.useAbbreviations, dimensionIri: xDimension.iri, - dimensionValues: xDimension.values, + dimensionValues: xDimensionValues, }); const { getValue: getX, getLabel: getXLabel } = useObservationLabels( @@ -133,7 +135,8 @@ const useGroupedColumnsState = ( abbreviationOrLabelLookup: segmentsByAbbreviationOrLabel, } = useMaybeAbbreviations({ useAbbreviations: fields.segment?.useAbbreviations, - dimension: segmentDimension, + dimensionIri: segmentDimension?.iri, + dimensionValues: segmentDimension?.values, }); const { getValue: getSegment, getLabel: getSegmentLabel } = diff --git a/app/charts/column/columns-stacked-state.tsx b/app/charts/column/columns-stacked-state.tsx index 2f7ed0cdb..821042638 100644 --- a/app/charts/column/columns-stacked-state.tsx +++ b/app/charts/column/columns-stacked-state.tsx @@ -34,6 +34,7 @@ import { getLabelWithUnit, getWideData, useDataAfterInteractiveFilters, + useMaybeTemporalDimensionValues, useOptionalNumericVariable, usePlottableData, useTemporalVariable, @@ -108,12 +109,13 @@ const useColumnsStackedState = ( } const xIsTime = isTemporalDimension(xDimension); + const xDimensionValues = useMaybeTemporalDimensionValues(xDimension, data); const { getAbbreviationOrLabelByValue: getXAbbreviationOrLabel } = useMaybeAbbreviations({ useAbbreviations: fields.x.useAbbreviations, dimensionIri: xDimension.iri, - dimensionValues: xDimension.values, + dimensionValues: xDimensionValues, }); const { getValue: getX, getLabel: getXLabel } = useObservationLabels( @@ -134,7 +136,8 @@ const useColumnsStackedState = ( abbreviationOrLabelLookup: segmentsByAbbreviationOrLabel, } = useMaybeAbbreviations({ useAbbreviations: fields.segment?.useAbbreviations, - dimension: segmentDimension, + dimensionIri: segmentDimension?.iri, + dimensionValues: segmentDimension?.values, }); const { getValue: getSegment, getLabel: getSegmentLabel } = diff --git a/app/charts/column/columns-state.tsx b/app/charts/column/columns-state.tsx index 7cb4ccd3b..323c35993 100644 --- a/app/charts/column/columns-state.tsx +++ b/app/charts/column/columns-state.tsx @@ -26,6 +26,7 @@ import { import { getLabelWithUnit, useDataAfterInteractiveFilters, + useMaybeTemporalDimensionValues, useOptionalNumericVariable, usePlottableData, useSegment, @@ -112,12 +113,13 @@ const useColumnsState = ( const timeUnit = xIsTime ? (xDimension as TemporalDimension).timeUnit : undefined; + const xDimensionValues = useMaybeTemporalDimensionValues(xDimension, data); const { getAbbreviationOrLabelByValue: getXAbbreviationOrLabel } = useMaybeAbbreviations({ useAbbreviations: fields.x.useAbbreviations, dimensionIri: xDimension.iri, - dimensionValues: xDimension.values, + dimensionValues: xDimensionValues, }); const { getValue: getX, getLabel: getXLabel } = useObservationLabels( diff --git a/app/charts/shared/chart-helpers.spec.tsx b/app/charts/shared/chart-helpers.spec.tsx index 6d8e2a730..19c3da186 100644 --- a/app/charts/shared/chart-helpers.spec.tsx +++ b/app/charts/shared/chart-helpers.spec.tsx @@ -1,14 +1,17 @@ +import { renderHook } from "@testing-library/react-hooks"; import { InternMap } from "d3"; import merge from "lodash/merge"; import { getWideData, prepareQueryFilters, + useMaybeTemporalDimensionValues, } from "@/charts/shared/chart-helpers"; import { InteractiveFiltersState } from "@/charts/shared/use-interactive-filters"; import { ChartType, Filters, InteractiveFiltersConfig } from "@/configurator"; import { FIELD_VALUE_NONE } from "@/configurator/constants"; import { Observation } from "@/domain/data"; +import { DimensionMetadataFragment } from "@/graphql/query-hooks"; import line1Fixture from "@/test/__fixtures/config/prod/line-1.json"; const makeCubeNsGetters = (cubeIri: string) => ({ @@ -137,3 +140,50 @@ describe("getWideData", () => { ]); }); }); + +describe("useMaybeTemporalDimensionValues", () => { + it("should return data values if dimension is temporal", () => { + const temporalDimension = { + __typename: "TemporalDimension", + iri: "year", + values: [ + { label: "1996", value: "1996" }, + { label: "2023", value: "2023" }, + ], + } as DimensionMetadataFragment; + const data: Observation[] = [ + { [temporalDimension.iri]: "1997" }, + { [temporalDimension.iri]: "2002" }, + { [temporalDimension.iri]: "2023" }, + ]; + + const { result } = renderHook(() => { + return useMaybeTemporalDimensionValues(temporalDimension, data); + }); + + expect(result.current).toEqual([ + { label: "1997", value: "1997" }, + { label: "2002", value: "2002" }, + { label: "2023", value: "2023" }, + ]); + }); + + it("should return dimension values if dimension is not temporal", () => { + const dimension = { + __typename: "NominalDimension", + iri: "year", + values: [ + { label: "A", value: "A" }, + { label: "B", value: "B" }, + { label: "C", value: "C" }, + ], + } as DimensionMetadataFragment; + const data: Observation[] = []; + + const { result } = renderHook(() => { + return useMaybeTemporalDimensionValues(dimension, data); + }); + + expect(result.current).toEqual(dimension.values); + }); +}); diff --git a/app/charts/shared/chart-helpers.tsx b/app/charts/shared/chart-helpers.tsx index 537a6f82c..5e63720ef 100644 --- a/app/charts/shared/chart-helpers.tsx +++ b/app/charts/shared/chart-helpers.tsx @@ -1,6 +1,7 @@ import { group, InternMap, sum } from "d3"; import omitBy from "lodash/omitBy"; import overEvery from "lodash/overEvery"; +import uniq from "lodash/uniq"; import { useCallback, useMemo } from "react"; import { @@ -22,7 +23,11 @@ import { QueryFilters, } from "@/configurator/config-types"; import { FIELD_VALUE_NONE } from "@/configurator/constants"; -import { Observation } from "@/domain/data"; +import { + DimensionValue, + isTemporalDimension, + Observation, +} from "@/domain/data"; import { truthy } from "@/domain/types"; import { DimensionMetadataFragment } from "@/graphql/query-hooks"; @@ -411,3 +416,23 @@ export const useImputationNeeded = ({ return imputationNeeded; }; + +/** Use to potentially extract temporal values from data. This is needed for + * column charts, where the temporal dimension is used as X axis (and we + * do not fetch all values for temporal dimensions, only the min and max). + */ +export const useMaybeTemporalDimensionValues = ( + dimension: DimensionMetadataFragment, + data: Observation[] +) => { + const maybeTemporalDimensionValues: DimensionValue[] = useMemo(() => { + if (isTemporalDimension(dimension)) { + const dates = data.map((d) => d[dimension.iri] as string); + return uniq(dates).map((d) => ({ label: d, value: d })); + } else { + return dimension.values; + } + }, [dimension, data]); + + return maybeTemporalDimensionValues; +}; From 514ad9052cd550f83a4154d721aed2acb309b70f Mon Sep 17 00:00:00 2001 From: Bartosz Prusinowski Date: Mon, 8 May 2023 09:48:47 +0200 Subject: [PATCH 3/3] refactor: Hook -> regular function (useMaybeTemporalDimensionValues) --- app/charts/column/columns-grouped-state.tsx | 6 ++++-- app/charts/column/columns-stacked-state.tsx | 6 ++++-- app/charts/column/columns-state.tsx | 6 ++++-- app/charts/shared/chart-helpers.spec.tsx | 17 ++++++--------- app/charts/shared/chart-helpers.tsx | 24 +++++++-------------- 5 files changed, 26 insertions(+), 33 deletions(-) diff --git a/app/charts/column/columns-grouped-state.tsx b/app/charts/column/columns-grouped-state.tsx index ca18d073c..bd9bf2081 100644 --- a/app/charts/column/columns-grouped-state.tsx +++ b/app/charts/column/columns-grouped-state.tsx @@ -29,7 +29,7 @@ import { import { getLabelWithUnit, useDataAfterInteractiveFilters, - useMaybeTemporalDimensionValues, + getMaybeTemporalDimensionValues, useOptionalNumericVariable, usePlottableData, useTemporalVariable, @@ -106,7 +106,9 @@ const useGroupedColumnsState = ( } const xIsTime = isTemporalDimension(xDimension); - const xDimensionValues = useMaybeTemporalDimensionValues(xDimension, data); + const xDimensionValues = useMemo(() => { + return getMaybeTemporalDimensionValues(xDimension, data); + }, [xDimension, data]); const { getAbbreviationOrLabelByValue: getXAbbreviationOrLabel } = useMaybeAbbreviations({ diff --git a/app/charts/column/columns-stacked-state.tsx b/app/charts/column/columns-stacked-state.tsx index 821042638..328f950ef 100644 --- a/app/charts/column/columns-stacked-state.tsx +++ b/app/charts/column/columns-stacked-state.tsx @@ -34,7 +34,7 @@ import { getLabelWithUnit, getWideData, useDataAfterInteractiveFilters, - useMaybeTemporalDimensionValues, + getMaybeTemporalDimensionValues, useOptionalNumericVariable, usePlottableData, useTemporalVariable, @@ -109,7 +109,9 @@ const useColumnsStackedState = ( } const xIsTime = isTemporalDimension(xDimension); - const xDimensionValues = useMaybeTemporalDimensionValues(xDimension, data); + const xDimensionValues = useMemo(() => { + return getMaybeTemporalDimensionValues(xDimension, data); + }, [xDimension, data]); const { getAbbreviationOrLabelByValue: getXAbbreviationOrLabel } = useMaybeAbbreviations({ diff --git a/app/charts/column/columns-state.tsx b/app/charts/column/columns-state.tsx index 323c35993..bcc1ffffe 100644 --- a/app/charts/column/columns-state.tsx +++ b/app/charts/column/columns-state.tsx @@ -26,7 +26,7 @@ import { import { getLabelWithUnit, useDataAfterInteractiveFilters, - useMaybeTemporalDimensionValues, + getMaybeTemporalDimensionValues, useOptionalNumericVariable, usePlottableData, useSegment, @@ -113,7 +113,9 @@ const useColumnsState = ( const timeUnit = xIsTime ? (xDimension as TemporalDimension).timeUnit : undefined; - const xDimensionValues = useMaybeTemporalDimensionValues(xDimension, data); + const xDimensionValues = useMemo(() => { + return getMaybeTemporalDimensionValues(xDimension, data); + }, [xDimension, data]); const { getAbbreviationOrLabelByValue: getXAbbreviationOrLabel } = useMaybeAbbreviations({ diff --git a/app/charts/shared/chart-helpers.spec.tsx b/app/charts/shared/chart-helpers.spec.tsx index 19c3da186..9ffe383bf 100644 --- a/app/charts/shared/chart-helpers.spec.tsx +++ b/app/charts/shared/chart-helpers.spec.tsx @@ -1,11 +1,10 @@ -import { renderHook } from "@testing-library/react-hooks"; import { InternMap } from "d3"; import merge from "lodash/merge"; import { getWideData, prepareQueryFilters, - useMaybeTemporalDimensionValues, + getMaybeTemporalDimensionValues, } from "@/charts/shared/chart-helpers"; import { InteractiveFiltersState } from "@/charts/shared/use-interactive-filters"; import { ChartType, Filters, InteractiveFiltersConfig } from "@/configurator"; @@ -141,7 +140,7 @@ describe("getWideData", () => { }); }); -describe("useMaybeTemporalDimensionValues", () => { +describe("getMaybeTemporalDimensionValues", () => { it("should return data values if dimension is temporal", () => { const temporalDimension = { __typename: "TemporalDimension", @@ -157,11 +156,9 @@ describe("useMaybeTemporalDimensionValues", () => { { [temporalDimension.iri]: "2023" }, ]; - const { result } = renderHook(() => { - return useMaybeTemporalDimensionValues(temporalDimension, data); - }); + const result = getMaybeTemporalDimensionValues(temporalDimension, data); - expect(result.current).toEqual([ + expect(result).toEqual([ { label: "1997", value: "1997" }, { label: "2002", value: "2002" }, { label: "2023", value: "2023" }, @@ -180,10 +177,8 @@ describe("useMaybeTemporalDimensionValues", () => { } as DimensionMetadataFragment; const data: Observation[] = []; - const { result } = renderHook(() => { - return useMaybeTemporalDimensionValues(dimension, data); - }); + const result = getMaybeTemporalDimensionValues(dimension, data); - expect(result.current).toEqual(dimension.values); + expect(result).toEqual(dimension.values); }); }); diff --git a/app/charts/shared/chart-helpers.tsx b/app/charts/shared/chart-helpers.tsx index 5e63720ef..39cb8284c 100644 --- a/app/charts/shared/chart-helpers.tsx +++ b/app/charts/shared/chart-helpers.tsx @@ -23,11 +23,7 @@ import { QueryFilters, } from "@/configurator/config-types"; import { FIELD_VALUE_NONE } from "@/configurator/constants"; -import { - DimensionValue, - isTemporalDimension, - Observation, -} from "@/domain/data"; +import { isTemporalDimension, Observation } from "@/domain/data"; import { truthy } from "@/domain/types"; import { DimensionMetadataFragment } from "@/graphql/query-hooks"; @@ -421,18 +417,14 @@ export const useImputationNeeded = ({ * column charts, where the temporal dimension is used as X axis (and we * do not fetch all values for temporal dimensions, only the min and max). */ -export const useMaybeTemporalDimensionValues = ( +export const getMaybeTemporalDimensionValues = ( dimension: DimensionMetadataFragment, data: Observation[] ) => { - const maybeTemporalDimensionValues: DimensionValue[] = useMemo(() => { - if (isTemporalDimension(dimension)) { - const dates = data.map((d) => d[dimension.iri] as string); - return uniq(dates).map((d) => ({ label: d, value: d })); - } else { - return dimension.values; - } - }, [dimension, data]); - - return maybeTemporalDimensionValues; + if (isTemporalDimension(dimension)) { + const dates = data.map((d) => d[dimension.iri] as string); + return uniq(dates).map((d) => ({ label: d, value: d })); + } else { + return dimension.values; + } };