Skip to content

fix(dataZoom): data shape distribution for time axis#16978

Merged
100pah merged 6 commits into
apache:masterfrom
andrearoota:fix-16924
May 17, 2025
Merged

fix(dataZoom): data shape distribution for time axis#16978
100pah merged 6 commits into
apache:masterfrom
andrearoota:fix-16924

Conversation

@andrearoota

@andrearoota andrearoota commented May 4, 2022

Copy link
Copy Markdown
Contributor

Brief Information

This pull request is in the type of:

  • bug fixing
  • new feature
  • others

What does this PR do?

Fix render data shadow line with data not equally distributed in time axis

Fixed issues

#16924
#15921
#16980

Details

Before: What was the problem?

Data shadow line in bottom dataZoom does not match with serie line on graph

image

After: How is it fixed in this PR?

If type of axis is set to "time", scale the original axis data range to dataZoom axis range
I used this mathematical formula

image

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

echarts-bot Bot commented May 4, 2022

Copy link
Copy Markdown

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.

@andrearoota andrearoota changed the title fix(dataZoom): render data not equally distributed. close apache#16924 fix(dataZoom): render data not equally distributed. close #16924 #15921 May 4, 2022
Comment thread src/component/dataZoom/SliderZoomView.ts Outdated
@Ovilia

Ovilia commented Sep 2, 2022

Copy link
Copy Markdown
Contributor

I'm also fixing related problems at #17311 . I will check the logic more carefully later. Thanks for your contribution!

@Ovilia Ovilia removed this from the 5.4.1 milestone Sep 23, 2022
@piotrbubelE

Copy link
Copy Markdown

Hello! Is there a plan to merge this PR anytime soon?
It also enables workaround for bug with axis.min and axis.max on dataShadow

@pchrzaszczewski

Copy link
Copy Markdown

I am also waiting for this fix. What are your plans?

@kamilapolewczykatos

Copy link
Copy Markdown

Merge would be great. I need this fix for my app.

@zsamotsil

zsamotsil commented May 20, 2024

Copy link
Copy Markdown

It would be great to ship it. Need also this fix in my application.

@piotrbubelE

Copy link
Copy Markdown

Here is issue that can use workaround from this PR:
#19957

@grzesikj

Copy link
Copy Markdown

When will this be available for use? Without this I cannot use data zoom slider in my project.

@Bernard-code

Copy link
Copy Markdown

I really could use this fix. When will this be available to use?

@grzesikj

grzesikj commented Aug 9, 2024

Copy link
Copy Markdown

Add it, please.

@grzesikj

Copy link
Copy Markdown

When can you add it?

@noahhai

noahhai commented Sep 25, 2024

Copy link
Copy Markdown

Fix PR pending for two years 😭

@andrearoota

Copy link
Copy Markdown
Contributor Author

@pchrzaszczewski Woah easy there. I get the frustration though.

I am going to create a pnpm patch based on the fixes in the PR and just pin the echarts version in my dependencies. Not great, but I assume at some point it will get fixed.

Does my code still work in the current version? I am willing to improve it and update it if necessary

@pchrzaszczewski

Copy link
Copy Markdown

@pchrzaszczewski Woah easy there. I get the frustration though.

I am going to create a pnpm patch based on the fixes in the PR and just pin the echarts version in my dependencies. Not great, but I assume at some point it will get fixed.

@noahhai I'm grateful that you responded. We can see that something is happening ;)

I've been patient for 2 years 💪 It is always better to at least know if there is a plan or not.
I'm keeping my fingers crossed and cheering that the day will finally come... 🤞

@grzesikj

grzesikj commented Nov 7, 2024

Copy link
Copy Markdown

I'm still waiting

@alinscodes

Copy link
Copy Markdown

Can we just deploy this pleasE?

@alinscodes alinscodes left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good.

@jteez

jteez commented Dec 11, 2024

Copy link
Copy Markdown

Still hoping for this fix too!

@ElicaInc

ElicaInc commented Apr 14, 2025

Copy link
Copy Markdown

@andrearoota fixed the mismatch between the chart and dataZoom. It works well in my setup—thank you, @andrearoota!

@alinscodes

Copy link
Copy Markdown

Lets get this in please

@ElicaInc

Copy link
Copy Markdown

This patch, made possible thanks to @andrearoota, worked perfectly in my setup while waiting for the official merge.

diff --git a/node_modules/echarts/lib/component/dataZoom/SliderZoomView.js b/node_modules/echarts/lib/component/dataZoom/SliderZoomView.js
index be21afa..25e561e 100644
--- a/node_modules/echarts/lib/component/dataZoom/SliderZoomView.js
+++ b/node_modules/echarts/lib/component/dataZoom/SliderZoomView.js
@@ -281,22 +281,35 @@ var SliderZoomView = /** @class */function (_super) {
       var areaPoints_1 = [[size[0], 0], [0, 0]];
       var linePoints_1 = [];
       var step_1 = thisShadowExtent[1] / (data.count() - 1);
-      var thisCoord_1 = 0;
+        // Added: Variables to support time axis
+      var thisDataExtent = data.getDataExtent(info.thisDim);
+      var normalizationConstant = size[0] / (thisDataExtent[1] - thisDataExtent[0]);
+      var isTimeAxis = info.thisAxis.type === 'time';
+      var thisCoord_1 = -step_1;
       // Optimize for large data shadow
       var stride_1 = Math.round(data.count() / size[0]);
       var lastIsEmpty_1;
-      data.each([otherDim], function (value, index) {
-        if (stride_1 > 0 && index % stride_1) {
-          thisCoord_1 += step_1;
+      data.each([info.thisDim, otherDim], function (thisValue, otherValue, index) {
+        if (stride_1 > 0 && (index % stride_1)) {
+          // Added: Only increment step when not using a time axis
+          if (!isTimeAxis) {
+            thisCoord_1 += step_1;
+          }
           return;
         }
+        
+        // Fix: Coordinate calculation split between time axis and non-time axis
+        thisCoord_1 = isTimeAxis
+          ? (+thisValue - thisDataExtent[0]) * normalizationConstant
+          : thisCoord_1 + step_1;
+    
         // FIXME
         // Should consider axis.min/axis.max when drawing dataShadow.
         // FIXME
         // 应该使用统一的空判断?还是在list里进行空判断?
-        var isEmpty = value == null || isNaN(value) || value === '';
+        var isEmpty = otherValue == null || isNaN(otherValue) || otherValue === '';
         // See #4235.
-        var otherCoord = isEmpty ? 0 : linearMap(value, otherDataExtent_1, otherShadowExtent_1, true);
+        var otherCoord = isEmpty ? 0 : linearMap(otherValue, otherDataExtent_1, otherShadowExtent_1, true);
         // Attempt to draw data shadow precisely when there are empty value.
         if (isEmpty && !lastIsEmpty_1 && index) {
           areaPoints_1.push([areaPoints_1[areaPoints_1.length - 1][0], 0]);
@@ -307,7 +320,7 @@ var SliderZoomView = /** @class */function (_super) {
         }
         areaPoints_1.push([thisCoord_1, otherCoord]);
         linePoints_1.push([thisCoord_1, otherCoord]);
-        thisCoord_1 += step_1;
+        
         lastIsEmpty_1 = isEmpty;
       });
       polygonPts = this._shadowPolygonPts = areaPoints_1;

@carlos-manuel-soares-alb

Copy link
Copy Markdown

Any news for this PR to be merged?

@echarts-bot

echarts-bot Bot commented May 17, 2025

Copy link
Copy Markdown

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.

@100pah 100pah merged commit fc6656f into apache:master May 17, 2025
@echarts-bot

echarts-bot Bot commented May 17, 2025

Copy link
Copy Markdown

Congratulations! Your PR has been merged. Thanks for your contribution! 👍

Ovilia added a commit that referenced this pull request Jun 18, 2025
fix(dataZoom): fix cases with dataset, relates to #16978
Ovilia added a commit that referenced this pull request Jun 19, 2025
@Ovilia Ovilia added this to the 6.0.0 milestone Jun 19, 2025
@Ovilia Ovilia changed the title fix(dataZoom): render data not equally distributed. close #16924 #15921 fix(dataZoom): data shape distribution for time axis Jun 21, 2025
Ovilia added a commit that referenced this pull request Jun 21, 2025
fix(dataZoom): fix the case when first value is NaN, relates to #16978
@Bilge

Bilge commented Aug 4, 2025

Copy link
Copy Markdown
Contributor

Does this address #13408 or nah?

Spoiler: it only half fixes it. It fixes the time series issue but not the step line issue.

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

Projects

None yet