Skip to content

fix(dataZoom): detail shape is not the same as data with value axis #17143

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Ovilia
Copy link
Contributor

@Ovilia Ovilia commented Jun 1, 2022

Brief Information

This pull request is in the type of:

  • bug fixing
  • new feature
  • others

What does this PR do?

The detail view in the dataZoom was not the same as the data in range, like:

In this case, we can see that there are three data points in the dataZoom detail view but only two points in the visible area of the grid.

Why is this PR necessary

The reason why the detail view should be relative to the visible data is that when user changes the range of the dataZoom, the detail view is servered as an important clue and it is confusing to have the two thing being different.

Fixed issues

Details

Before: What was the problem?

The detail view is not the same as the grid area if the axis is a value type.

The main reason for this problem is that, currently, the x position of a horizontal detail region is decided only by the index of data, rather than the x position of the data, similar for y. As a result, the horizontal distance between each points are equal rather than decided by data, and this is the main reason it's different from that in the series.

After: How is it fixed in this PR?

The detail view should be the same as the grid area. So in this PR, I fixed the points in the detail view such that its position is decided by the data in the series rather than data index.

Here are some known differences after this PR:

Case 1: Two value axes for a line series

When the line series has two value axes, the horizontal dataZoom detail view should be consistant with that in the grid area.

option = {
    xAxis: {
        type: 'value'
    },
    yAxis: {
        type: 'value'
    },
    series: [
        {
        data: [
            [12, 3],
            [15, 50],
            [18, 30],
            [44, 50]
        ],
        type: 'line'
        }
    ],
    dataZoom: getDataZoom({
        startValue: 15,
    })
};

image

The selected area of the dataZoom detail view (green) should be consistant with the line series in the visible grid area.

Please also note that the vertical dataZoom detail views in the second row have changed to be more informative. In the previous implementation (left), the user may get confused about the vertical detail, but in the new implementation (right), the shape in the detail is consistant with the line series (although the areaStyle should be applied from the bottom rather than top).

Case 2: When min/max is provided

When axis min/max is provided, the starting/ending value of the detail view should also be min/max rather than data range.

option = {
    xAxis: {
        type: 'value',
        min: 8
    },
    yAxis: {
        type: 'value'
    },
    series: [
        {
        data: [
            [-12, 3],
            [15, 50],
            [18, 30],
            [44, 50]
        ],
        type: 'line'
        }
    ],
    dataZoom: getDataZoom({
        startValue: 12
    })
};

image

Case 3: Scatter series

image

I think the old design (left) is not very informative because it's very confusing how the lines in the detail view relates to scatter series.

But this PR (right) also have problem with scatter series for now because it connects from the first data to the last data using lines in the detail view. This causes the scatter detail to be messy. Maybe it's better to let the series to decide how the detail view looks? I would suggest using the same scatter radius in the detail view because otherwise small scatters may not be visible in the detail view.

Misc

  • The API has been changed (apache/echarts-doc#xxx).
  • This PR depends on ZRender changes (ecomfe/zrender#xxx).

Related test cases or examples to use the new APIs

N.A.

Others

Merging options

  • Please squash the commits into a single one when merging.

Other information

@echarts-bot
Copy link

echarts-bot bot commented Jun 1, 2022

Thanks for your contribution!
The community will review it ASAP. In the meanwhile, please checkout the coding standard and Wiki about How to make a pull request.

The pull request is marked to be PR: author is committer because you are a committer of this project.

@Ovilia
Copy link
Contributor Author

Ovilia commented Jun 1, 2022

TODO: visual testing the dataZoom related changes.

@echarts-bot
Copy link

echarts-bot bot commented Jul 4, 2022

To reviewers: If this PR is going to be described in the changelog in the future release, please make sure this PR has one of the following labels: PR: doc ready, PR: awaiting doc, PR: doc unchanged

This message is shown because the PR description doesn't contain the document related template.

@Ovilia Ovilia dismissed a stale review via 63f0e75 November 8, 2022 07:04
@ianschmitz
Copy link

@Ovilia does this also address stacked bar charts being rendered as a data shadow?

@Ovilia
Copy link
Contributor Author

Ovilia commented Dec 16, 2022

@ianschmitz Current detail view for the stacked bars are not correct because only the first series is considered. I'm not sure this should be included in this PR. I will see if it can be fixed.

@alienzhangyw
Copy link

@Ovilia Your fix is still not as perfect as expected (red lines)
image

@martingrumbt
Copy link

@Ovilia Thank you for having a look into this issue.

Your fix breaks the dataZoom panels when nullish data is involved.
null as x value breaks the x-axis zoom panel.
null as y value breaks the y-axis zoom panel.
Also for undefined, '-'

master branch

master_datazoom_x_and_y DataZoom for x and y axis enabled master_datazoom_x DataZoom only for x axis enabled

fix-17141 branch

fix-17141_datazoom_x_and_y DataZoom for x and y axis enabled fix-17141_datazoom_x DataZoom only for x axis enabled

TestFile

<!DOCTYPE html>
<html>
<head>
    <meta charset="utf-8">
    <meta name="viewport" content="width=device-width, initial-scale=1"/>
    <script src="lib/simpleRequire.js"></script>
    <script src="lib/config.js"></script>
    <script src="lib/jquery.min.js"></script>
    <script src="lib/facePrint.js"></script>
    <script src="lib/testHelper.js"></script>
    <link rel="stylesheet" href="lib/reset.css"/>
</head>
<body>
<style>
</style>

<script>
    const dataNoneLinear = [
        [0, 10],
        [10, 0],
        [80, 1],
        [100, 15],
    ];
    const dataNoneLinearWithNull = [
        [0, 10],
        [10, 0],
        [null, 1],
        [100, null],
    ];

    const dataTimeNoneLinear = [
        [(new Date("07/05/2023 10:00:00")).getTime(), (new Date("07/05/2023 10:00:00")).getTime()],
        [(new Date("07/05/2023 11:00:00")).getTime(), (new Date("07/05/2023 00:00:00")).getTime()],
        [(new Date("07/05/2023 18:00:00")).getTime(), (new Date("07/05/2023 01:00:00")).getTime()],
        [(new Date("07/05/2023 20:00:00")).getTime(), (new Date("07/05/2023 15:00:00")).getTime()],
    ];
    const dataTimeNoneLinearWithNull = [
        [(new Date("07/05/2023 10:00:00")).getTime(), (new Date("07/05/2023 10:00:00")).getTime()],
        [(new Date("07/05/2023 11:00:00")).getTime(), (new Date("07/05/2023 00:00:00")).getTime()],
        [null, (new Date("07/05/2023 01:00:00")).getTime()],
        [(new Date("07/05/2023 20:00:00")).getTime(), null],
    ];

    const charts = [
        {id: '01', type: 'line', axisType: 'category', data: dataNoneLinear},
        {id: '02', type: 'line', axisType: 'category', data: dataNoneLinearWithNull},
        {id: '03', type: 'line', axisType: 'value', data: dataNoneLinear},
        {id: '04', type: 'line', axisType: 'value', data: dataNoneLinearWithNull},
        {id: '05', type: 'line', axisType: 'time', data: dataTimeNoneLinear},
        {id: '06', type: 'line', axisType: 'time', data: dataTimeNoneLinearWithNull},
    ];

    function createChartDOMDiv(id) {
        let element = document.getElementById(id);
        if (!element) {
            element = document.createElement('div');
            element.id = id;
            document.body.appendChild(element);
        }
    }

    function getDataZoom(opt) {
        const dataZoom = {
            borderColor: 'blue',
            fillerColor: 'rgba(0, 255, 0, 0.2)',
            dataBackground: {
                lineStyle: {
                    color: 'blue'
                },
                areaStyle: {
                    color: 'rgba(0, 0, 255, 0.6)'
                }
            },
            selectedDataBackground: {
                lineStyle: {
                    color: 'blue'
                },
                areaStyle: {
                    color: 'rgba(0, 0, 255, 0.6)'
                }
            }
        };
        return Object.assign(opt, dataZoom);
    }

    charts.forEach(function ({id, type, axisType, data}) {
        createChartDOMDiv(id);

        require([
            'echarts',
        ], function (echarts) {
            var option;

            option = {
                xAxis: {
                    type: axisType
                },
                yAxis: {
                    type: axisType
                },
                series: [
                    {
                        data: data,
                        type: type
                    }
                ],
                dataZoom: [
                    getDataZoom({
                        xAxisIndex: 0,
                    }),
                    getDataZoom({
                        yAxisIndex: 0,
                    }),
                ]
            };

            testHelper.create(echarts, id, {
                title: [
                    "Test DataZoom on x and y axis",
                    `${type} | ${axisType}`,
                ],
                info: {
                    type: type,
                    xAxisType: axisType,
                    yAxisType: axisType,
                },
                dataTable: {
                    x: data.map(d => d[0]),
                    y: data.map(d => d[1]),
                },
                option: option
            });
        });
    })
</script>
</body>
</html>

@martingrumbt
Copy link

@Ovilia How can I support you to solve this issue?

@last-Programmer
Copy link

Any idea when this will be released to the stable npm channel? Thanks.

@simonwunderlich
Copy link

Would be great to have this integrated!

@Chasen-Zhang
Copy link

@Ovilia --help

@alinscodes
Copy link

Would be great to have this integrated!

@tditz
Copy link

tditz commented Feb 20, 2025

Any idea when it will be ready?
Is there anything I can do to speed it up?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants