Skip to content

Code Insights: Refactor series chart data API

Warren Gifford requested to merge vk/refactor-line-chart-api into main

Created by: vovakulikov

Background

Prior to this PR, we had an API where you should pass one big data array with all series data within, so series share their x value, if there is no value for some series at some particular point of time in this case we provide null value for it.

Example:

const DATA_WITH_STEP: DatumWithMissingData[] = [
        { x: 1588965700286 - 4 * 24 * 60 * 60 * 1000, a: null, b: null, c: null },
        { x: 1588965700286 - 3 * 24 * 60 * 60 * 1000, a: null, b: null, c: null },
        { x: 1588965700286 - 2 * 24 * 60 * 60 * 1000, a: 94, b: null, c: null },
        { x: 1588965700286 - 1.5 * 24 * 60 * 60 * 1000, a: 134, b: null, c: 200 },
        { x: 1588965700286 - 1.4 * 24 * 60 * 60 * 1000, a: null, b: 150, c: null },
        { x: 1588965700286 - 1.3 * 24 * 60 * 60 * 1000, a: null, b: 150, c: 150 },
        { x: 1588965700286 - 24 * 60 * 60 * 1000, a: 134, b: 190, c: 190 },
        { x: 1588965700286, a: 123, b: 170, c: 170 },
        { x: 1588965700286 + 24 * 60 * 60 * 1000, a: null, b: 200, c: null },
        { x: 1588965700286 + 1.3 * 24 * 60 * 60 * 1000, a: null, b: 180, c: null },
    ]

    const SERIES: Series<DatumWithMissingData>[] = [
        {
            dataKey: 'a',
            name: 'A metric',
            color: 'var(--blue)',
        },
        {
            dataKey: 'c',
            name: 'C metric',
            color: 'var(--purple)',
        },
        {
            dataKey: 'b',
            name: 'B metric',
            color: 'var(--warning)',
        },
    ]

This isn't the optimal way how to handle data for series, in fact, our Line Chart component converts this data shae back to shape where series have data within a series configuration.

This prior version of data API it also was complicated to handle TS types around Datum, because datum is too generic it can have a lot of data (data for each series)

So in this PR, we simplify data API around Series chart. Now series chart requires a series with data, which means that we have the following data API instead

const SERIES: Series<StandardDatum>[] = [
        {
            id: 'series_001',
            data: [
                { x: 1588965700286 - 4 * 24 * 60 * 60 * 1000, value: null },
                { x: 1588965700286 - 3 * 24 * 60 * 60 * 1000, value: null },
                { x: 1588965700286 - 2 * 24 * 60 * 60 * 1000, value: 94 },
            ],
            name: 'A metric',
            color: 'var(--blue)',
        },
        {
            id: 'series_002',
            data: [
                { x: 1588965700286 - 4 * 24 * 60 * 60 * 1000, value: null },
                { x: 1588965700286 - 3 * 24 * 60 * 60 * 1000, value: null },
                { x: 1588965700286 - 2 * 24 * 60 * 60 * 1000, value: 200 },
            ],
            name: 'B metric',
            color: 'var(--warning)',
        },
    ]

For reviewers

This PR has a big diff but this is because I had to migrate all chart data mocks and we got a lot of them around series chart related components.

So the main commits for review

  • Changing Line Chart API f49bc1eb90f573a48cf11cd7512c998fd9de4148
  • Migration runtime insight data pipeline ad96b62dcec3699e8af778418b636d4a424c1e88
  • Migration backend insight data pipeline 3c745f088b97502f6dfda736cbd5eb409e8800b3

Test plan

  • Make sure that there are no regression visuals and performance around the line chart
  • Make sure that all stories have proper mocks and don't have any problems visually and on the console

App preview:

Check out the client app preview documentation to learn more.

Merge request reports