Fix stacked areas overflow on null values when connectNulls is true

[alxnddr]

Brief Information

This pull request is in the type of:

• [x] bug fixing
• [ ] new feature
• [ ] others

What does this PR do?

Minimal reproduction Fix on the preview branch

The pull request fixes the logic of computing stacked on points of stacked areas which fixes areas overflow on null values when connectNulls is true.

Stacked on value can be NaN when:

1. When the value of series exists but there is no series below it or the series it is stacked on do not have suitable values according to stackStrategy
2. When the value of series itself is NaN

The getStackedOnPoint function behaved the same in both scenarios, when the stacked on value is NaN it decided that the area should be stacked on the valueStart of the series coordinates. However, when the value of series itself is NaN the stacked on value should be NaN as well for connectNulls option to work correctly and just connect the line without dropping it to the start of coordinates.

Fixed issues

Closes https://github.com/apache/echarts/issues/19598 Closes https://github.com/apache/echarts/issues/17135

Details

There is an existing test case area-stack.html which has a chart affected by the issue.

Before: What was the problem?

Yellow and blue areas have enabled connectNulls so the lines in the middle are connected but areas drop to the start of coordinates.

After: How does it behave after the fixing?

After the fix not only the lines are connected but also areas.

Document Info

One of the following should be checked.

• [x] This PR doesn't relate to document changes
• [ ] The document should be updated later
• [ ] The document changes have been made in apache/echarts-doc#xxx

Misc

ZRender Changes

• [ ] This PR depends on ZRender changes (ecomfe/zrender#xxx).

Related test cases or examples to use the new APIs

area-stack.html -> line break

Others

Merging options

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

Other information

[echarts-bot[bot]]

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.

[helgasoft]

The purpose of stacked lines is to show totals. This PR would make totals at all red points incorrect. I think responsibility for such "fixes" belongs to the developer, not to the charting library. Solution is simple - preprocess your data by removing or approximating missing values, then ECharts will gladly display what you want to see.

[alxnddr]

@helgasoft thanks for your feedback. In my pull request I addressed areas stacking when connectNulls is enabled so that it literally connects polygon lines on null values instead of dropping the area polygon to zero which does not look correct:

Manually interpolating values would break the step option behavior, and in addition to it supporting the smooth option would be a disproportionate effort to implement with custom interpolation.

To summarize

• the existing behavior is not correct which was was reported by a few people
• the PR changes make connectNulls work as it is called — connect line and polygon on null values
• there is no workaround that does not limit users from using other features of ECharts

Based on these points, I believe this change is valid to be merged to the library code.

[helgasoft]

addressed areas stacking when connectNulls is enabled

I think connectNulls should not be enabled in stacked area charts since it makes ECharts display fake totals.

Here are the two methods of data preprocessing - approximate or remove missing values, Demo Code The developer would choose which one is more appropriate, or a combination of the two.

[github-actions[bot]]

The changes brought by this PR can be previewed at: https://echarts.apache.org/examples/editor?version=PR-19602@536da2c

[helgasoft]

A strong argument in favor would be to find out whether another charting library (highcharts, amcharts, ?) has taken a chance on implementing it.

[visakadev]

addressed areas stacking when connectNulls is enabled

I think connectNulls should not be enabled in stacked area charts since it makes ECharts display fake totals.

Here are the two methods of data preprocessing - approximate or remove missing values, Demo Code The developer would choose which one is more appropriate, or a combination of the two.

approximate solution fixed my problem: 💯 👍

DEMO codesandbox

The data preprocessing - approximate solution fixed my problem:

In my case, when rendering the area range (minimum value, maximum value), and latest, I used the solution shown in the code before and applied a function to approximate the values.

Before: 👎

After approximate null values 👍

I added a custom tooltip to render the original value instead of the approximate value.

[admsev]

@gooroodev review

[gooroodev]

1. Summary of Changes

The pull request addresses an issue where stacked areas overflow on null values when the connectNulls option is set to true. The changes involve updating the getStackedOnPoint function in src/chart/line/helper.ts to handle null values more gracefully by:

• Separating the retrieval of stackedOverValue and stackResultValue.
• Adding a condition to check for stackedOverValue and stackResultValue to determine if stackedOverValue should be set to dataCoordInfo.valueStart.

2. Issues, Bugs, or Typos

Issue 1: Unnecessary Check on dataCoordInfo.stacked

The condition dataCoordInfo.stacked && isNaN(stackResultValue) is redundant since dataCoordInfo.stacked is already checked before retrieving stackResultValue.

Proposed Improvement: Remove the redundant dataCoordInfo.stacked check in the condition.

-    if (isNaN(stackedOverValue) && !(dataCoordInfo.stacked && isNaN(stackResultValue))) {
+    if (isNaN(stackedOverValue) && !isNaN(stackResultValue)) {

3. General Review of Code Quality and Style

Positive Aspects:

• The code changes are clear and well-targeted at solving the specific issue.
• Variable names are meaningful and improve readability.

Suggestions for Improvement:

• Consistency in formatting: Ensure consistent use of indentation and spacing.
• Comments: Adding comments to explain the logic, especially the condition handling, would improve maintainability.

Example of Improved Code with Comments:

export function getStackedOnPoint(
    dataCoordInfo: DataCoordInfo,
    coordSys: CoordinateSystem,
    data: SeriesData,
    idx: number
) {
    let stackedOverValue = NaN;
    let stackResultValue = NaN;

    // Retrieve stacked values if stacking is enabled
    if (dataCoordInfo.stacked) {
        stackedOverValue = data.get(
            data.getCalculationInfo('stackedOverDimension'),
            idx
        ) as number;
        stackResultValue = data.get(
            data.getCalculationInfo('stackResultDimension'),
            idx
        ) as number;
    }

    // If stackedOverValue is NaN but stackResultValue is valid, use valueStart
    if (isNaN(stackedOverValue) && !isNaN(stackResultValue)) {
        stackedOverValue = dataCoordInfo.valueStart;
    }

    const baseDataOffset = dataCoordInfo.baseDataOffset;
    const stackedData = [];
    stackedData[baseDataOffset] = data.get(dataCoordInfo.baseDim, idx);
    stackedData[1 - baseDataOffset] = stackedOverValue;

    return coordSys.dataToPoint(stackedData);
}

Overall, the pull request effectively addresses the issue, but minor improvements in condition handling and code comments would enhance its quality.

Yours, Gooroo.dev. To receive reviews automatically, install Github App