Skip to content

Commit

Permalink
EES-5542 chart types tidy
Browse files Browse the repository at this point in the history
  • Loading branch information
bennettstuart committed Jan 28, 2025
1 parent 9f7d803 commit 86470fc
Show file tree
Hide file tree
Showing 33 changed files with 1,273 additions and 984 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ import _dataBlockService, {
} from '@admin/services/dataBlockService';
import _permissionService from '@admin/services/permissionService';
import render from '@common-test/render';
import { ChartConfig } from '@common/modules/charts/types/chart';
import _tableBuilderService, {
Subject,
} from '@common/services/tableBuilderService';
import { Chart } from '@common/services/types/blocks';
import { waitFor } from '@testing-library/dom';
import { screen, within } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
Expand Down Expand Up @@ -51,7 +51,10 @@ jest.mock('@common/hooks/useMedia', () => ({
}));

describe('ReleaseDataBlockEditPage', () => {
const chart: Chart = {
const chart: ChartConfig = {
stacked: false,
includeNonNumericData: false,
showDataLabels: false,
title: 'Test chart title',
alt: 'Test chart alt',
type: 'verticalbar',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const testId = (dataBlock: ReleaseDataBlock) =>
const DataBlockPageReadOnlyTabs = ({ releaseId, dataBlock }: Props) => {
const getChartFile = useGetChartFile(releaseId);
const {
chart,
fullChart,
isTableDataLoading,
isTableDataError,
fullTable,
Expand Down Expand Up @@ -79,11 +79,11 @@ const DataBlockPageReadOnlyTabs = ({ releaseId, dataBlock }: Props) => {
<WarningMessage>Could not load chart</WarningMessage>
}
>
{chart && (
{fullChart && (
<ChartRenderer
id="dataBlockTabs-chart"
source={dataBlock.source}
chart={chart}
fullChart={fullChart}
/>
)}
</ErrorBoundary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,29 +249,31 @@ const DataBlockPageTabs = ({
const handleDataBlockSourceSave: DataBlockSourceWizardSaveHandler =
useCallback(
async ({ query, table, tableHeaders, details }) => {
const charts = produce(dataBlock?.charts ?? [], draft => {
const majorAxis = draft[0]?.axes?.major;
const legend = draft[0]?.legend;

const charts = produce(dataBlock?.charts ?? [], ([draft]) => {
// If old chart title is the same as the old table title, then the chart title is defaulting to
// the table title, so don't inadvertently set the old chart/table title
if (
dataBlock?.charts[0] &&
dataBlock?.charts[0]?.title === dataBlock?.heading
) {
draft[0].title = undefined;
draft.title = undefined;
}

// Remove data sets that are no longer applicable to a given table's subject meta.
if (majorAxis?.dataSets) {
majorAxis.dataSets = majorAxis.dataSets.filter(
dataSet => !isOrphanedDataSet(dataSet, table.subjectMeta),
);
}
if (legend?.items) {
legend.items = legend.items.filter(
item => !isOrphanedDataSet(item.dataSet, table.subjectMeta),
);
if (draft && draft.type !== 'infographic') {
const majorAxis = draft.axes?.major;
const legend = draft?.legend;

// Remove data sets that are no longer applicable to a given table's subject meta.
if (majorAxis.dataSets) {
majorAxis.dataSets = majorAxis.dataSets.filter(
dataSet => !isOrphanedDataSet(dataSet, table.subjectMeta),
);
}
if (legend.items) {
legend.items = legend.items.filter(
item => !isOrphanedDataSet(item.dataSet, table.subjectMeta),
);
}
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import Tabs from '@common/components/Tabs';
import TabsSection from '@common/components/TabsSection';
import useDebouncedCallback from '@common/hooks/useDebouncedCallback';
import useToggle from '@common/hooks/useToggle';
import { RenderableChart } from '@common/modules/charts/components/ChartRenderer';
import {
horizontalBarBlockDefinition,
HorizontalBarProps,
Expand All @@ -37,15 +36,17 @@ import {
import {
AxisType,
ChartDefinition,
ChartProps,
ChartConfig,
InfographicConfig,
DraftFullChart,
DraftChartConfig,
} from '@common/modules/charts/types/chart';
import { DataSet } from '@common/modules/charts/types/dataSet';
import { FullTableMeta } from '@common/modules/table-tool/types/fullTable';
import {
ReleaseTableDataQuery,
TableDataResult,
} from '@common/services/tableBuilderService';
import { Chart } from '@common/services/types/blocks';
import { ValidationProblemDetails } from '@common/services/types/problemDetails';
import parseNumber from '@common/utils/number/parseNumber';
import { isServerValidationError } from '@common/validation/serverValidations';
Expand All @@ -60,37 +61,41 @@ const chartDefinitions: ChartDefinition[] = [
mapBlockDefinition,
];

type ChartBuilderChartProps = RenderableChart & {
type ChartBuilderChartProps = DraftFullChart & {
file?: File;
};

const filterChartProps = (props: ChartBuilderChartProps): Chart => {
const filterChartProps = (props: ChartBuilderChartProps): ChartConfig => {
const excludedProps: (
| keyof ChartBuilderChartProps
| keyof InfographicChartProps
| keyof MapBlockProps
)[] = [
'data',
'meta',
'getInfographic',
'file',
'titleType',
'onBoundaryLevelChange',
];

if (props.titleType === 'default') {
excludedProps.push('title');
)[] = ['data', 'meta', 'getInfographic', 'file', 'onBoundaryLevelChange'];

const excludedChartConfigProps: (
| keyof ChartConfig
| keyof InfographicConfig
)[] = ['titleType'];

if (props.chartConfig.titleType === 'default') {
excludedChartConfigProps.push('title');
}

if (props.type !== 'infographic') {
excludedProps.push('fileId');
if (props.chartConfig.type !== 'infographic') {
excludedChartConfigProps.push('fileId');
}

let filteredProps = omit(props, excludedProps) as Chart;
let filteredProps = omit(
{
...props,
chartConfig: omit(props.chartConfig, excludedChartConfigProps),
},
excludedProps,
) as ChartConfig;

// Filter out deprecated data set configurations
filteredProps = produce(filteredProps, draft => {
if (draft.axes.major) {
if (draft.axes?.major) {
draft.axes.major.dataSets = draft.axes.major.dataSets.map(
dataSet => omit(dataSet, ['config']) as DataSet,
);
Expand All @@ -108,10 +113,10 @@ interface Props {
data: TableDataResult[];
meta: FullTableMeta;
releaseId: string;
initialChart?: Chart;
initialChart?: ChartConfig;
tableTitle: string;
onChartSave: (chart: Chart, file?: File) => Promise<void>;
onChartDelete: (chart: Chart) => void;
onChartSave: (chart: ChartConfig, file?: File) => Promise<void>;
onChartDelete: (chart: ChartConfig) => void;
onTableQueryUpdate: TableQueryUpdateHandler;
}

Expand Down Expand Up @@ -164,13 +169,13 @@ export default function ChartBuilder({
return undefined;
}

const baseProps: ChartProps = {
...options,
const baseProps: DraftFullChart = {
data,
map,
legend,
axes,
meta,
chartConfig: {
...options,
legend,
},
};

switch (definition.type) {
Expand All @@ -183,32 +188,28 @@ export default function ChartBuilder({
? () => Promise.resolve(options.file as File)
: getChartFile,
};
case 'line':
return {
...(baseProps as LineChartProps),
type: 'line',
};
case 'horizontalbar':
return {
...(baseProps as HorizontalBarProps),
type: 'horizontalbar',
};
case 'verticalbar':
return {
...(baseProps as VerticalBarProps),
type: 'verticalbar',
};
case 'map':
return {
...(baseProps as MapBlockProps),
map: map ?? {
dataSetConfigs: [],
data,
meta,
chartConfig: {
...baseProps,
map: map ?? {
dataSetConfigs: [],
},
boundaryLevel: options.boundaryLevel ?? 0,
},
boundaryLevel: options.boundaryLevel ?? 0,
type: 'map',
onBoundaryLevelChange: (boundaryLevel: number) =>
onTableQueryUpdate({ boundaryLevel }),
};
case 'line':
case 'horizontalbar':
case 'verticalbar':
return {
...baseProps,
type: definition.type,
};

default:
return undefined;
}
Expand Down Expand Up @@ -333,7 +334,7 @@ export default function ChartBuilder({
/>

{definition && (
<ChartBuilderPreview chart={chart} loading={isDataLoading} />
<ChartBuilderPreview fullChart={chart} loading={isDataLoading} />
)}

{definition && (
Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,19 @@
import styles from '@admin/pages/release/datablocks/components/chart/ChartBuilder.module.scss';
import Details from '@common/components/Details';
import LoadingSpinner from '@common/components/LoadingSpinner';
import ChartRenderer, {
RenderableChart,
} from '@common/modules/charts/components/ChartRenderer';
import ChartRenderer from '@common/modules/charts/components/ChartRenderer';
import { DraftFullChart } from '@common/modules/charts/types/chart';
import isChartRenderable, {
getChartPreviewText,
} from '@common/modules/charts/util/isChartRenderable';
import React, { useRef } from 'react';

interface Props {
chart?: RenderableChart;
fullChart?: DraftFullChart;
loading: boolean;
}

const ChartBuilderPreview = ({ chart, loading }: Props) => {
const ChartBuilderPreview = ({ fullChart, loading }: Props) => {
const renderCount = useRef(0);

return (
Expand All @@ -23,19 +22,19 @@ const ChartBuilderPreview = ({ chart, loading }: Props) => {
className="govuk-width-container govuk-!-margin-bottom-6"
data-testid="chartBuilderPreviewContainer"
>
{chart && isChartRenderable(chart) ? (
{fullChart && isChartRenderable(fullChart) ? (
<LoadingSpinner loading={loading} text="Loading chart data">
<div className="govuk-width-container govuk-!-padding-4 dfe-border">
<ChartRenderer
chart={chart}
fullChart={fullChart}
key={renderCount.current}
id="chartBuilderPreview"
/>
</div>
</LoadingSpinner>
) : (
<div className={styles.previewPlaceholder}>
<p>{getChartPreviewText(chart)}</p>
<p>{getChartPreviewText(fullChart?.chartConfig)}</p>
</div>
)}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ import ChartBuilder, {
import { SavedDataBlock } from '@admin/pages/release/datablocks/components/DataBlockPageTabs';
import { ReleaseDataBlock } from '@admin/services/dataBlockService';
import releaseChartFileService from '@admin/services/releaseChartFileService';
import { ChartConfig } from '@common/modules/charts/types/chart';
import { FullTable } from '@common/modules/table-tool/types/fullTable';
import mapFullTable from '@common/modules/table-tool/utils/mapFullTable';
import tableBuilderService, {
ReleaseTableDataQuery,
TableDataQuery,
} from '@common/services/tableBuilderService';
import { Chart } from '@common/services/types/blocks';
import isEqual from 'lodash/isEqual';
import React, { useCallback, useMemo } from 'react';

Expand Down Expand Up @@ -47,7 +47,7 @@ const ChartBuilderTabSection = ({
);

const handleChartSave = useCallback(
async (chart: Chart, file?: File) => {
async (chart: ChartConfig, file?: File) => {
let chartToSave = chart;

if (chart.type === 'infographic' && file) {
Expand All @@ -72,7 +72,7 @@ const ChartBuilderTabSection = ({
);

const handleChartDelete = useCallback(
async (chart: Chart) => {
async (chart: ChartConfig) => {
// Cleanup potential infographic chart file if required
if (chart.type === 'infographic' && chart.fileId) {
await releaseChartFileService.deleteChartFile(releaseId, chart.fileId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
ChartBuilderFormsContextProvider,
} from '@admin/pages/release/datablocks/components/chart/contexts/ChartBuilderFormsContext';
import render from '@common-test/render';
import { Chart } from '@common/services/types/blocks';
import { ChartConfig } from '@common/modules/charts/types/chart';
import { screen, waitFor, within } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import noop from 'lodash/noop';
Expand Down Expand Up @@ -360,7 +360,7 @@ describe('ChartBuilder', () => {
});

test('calls `onTableQueryUpdate` when change boundary level', async () => {
const testInitialChart: Chart = {
const testInitialChart: ChartConfig = {
type: 'map',
boundaryLevel: 2,
map: {
Expand Down Expand Up @@ -451,7 +451,7 @@ describe('ChartBuilder', () => {
});

describe('data groupings tab', () => {
const testInitialChart: Chart = {
const testInitialChart: ChartConfig = {
type: 'map',
boundaryLevel: 2,
legend: {
Expand Down
Loading

0 comments on commit 86470fc

Please sign in to comment.