Skip to content
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

Partially missing line on time axis, regression in 4.3.1 #11426

Closed
StephanTLavavej opened this issue Jul 28, 2023 · 11 comments · Fixed by #11432
Closed

Partially missing line on time axis, regression in 4.3.1 #11426

StephanTLavavej opened this issue Jul 28, 2023 · 11 comments · Fixed by #11432

Comments

@StephanTLavavej
Copy link

Expected behavior

With datapoints covering the entire range of the x-axis, I expect that the entire line should be displayed. This worked perfectly up through chart.js 4.3.0.

Expected codepen with 4.3.0: https://codepen.io/StephanTLavavej/pen/VwVVWmy

chartjs_430_expected

Current behavior

Starting with chart.js 4.3.1 and continuing with chart.js 4.3.2, part of the line isn't displayed.

chartjs_432_bad

Reproducible sample

https://codepen.io/StephanTLavavej/pen/MWzzojY

Optional extra steps/info to reproduce

No response

Possible solution

No response

Context

We use chart.js for the microsoft/STL Status Chart at https://microsoft.github.io/STL/ . Thank you for this amazing library!

chart.js version

v4.3.2

Browser name and version

Chrome 115.0.5790.110

Link to your project

No response

@etimberg
Copy link
Member

Interesting, this only happens if the min is set on the axis 🤔

@stockiNail I'm guessing this is somehow related to https://github.com/chartjs/Chart.js/pull/11377/files

@StephanTLavavej
Copy link
Author

If it makes the investigation easier, I've simplified the repro to a plain linear axis, instead of needing a time axis. The new repro: https://codepen.io/StephanTLavavej/pen/QWJzqWg

@stockiNail
Copy link
Contributor

stockiNail commented Jul 31, 2023

@etimberg @StephanTLavavej I'll have a look.

EDIT: The issue is related to line.element draw where start and count are related to total amount of points and not the subset.

@ErikDorstel
Copy link

ErikDorstel commented Aug 1, 2023

Based on "https://codepen.io/StephanTLavavej/pen/QWJzqWg" the line is not displayed when:

(xmin>4) && (x<(xmin*2)-4)

@stockiNail
Copy link
Contributor

@etimberg @StephanTLavavej @ErikDorstel I have seen where the issue is. To fix #11273 the line dataset element is fill with only the points to show to calculate correctly the control points.
Unfortunately the draw method (but also others to calculate the paths and segments) are invoked passing the start index of the points which is related to the whole amount of points, instead the points in the line element are always starting from.
I need a bit of time to find out the best fix to solve both issues.

@elinake
Copy link

elinake commented Aug 1, 2023

I have same issue: https://stackblitz.com/edit/stackblitz-starters-ngv2t8?file=src%2Fapp%2Fapp.component.ts

Bug happens with both wheel and drag and on all axis modes. I have linear scales and no limits. Downgrading to Chartjs 4.3.0 fixed it.

I also noticed that same issue happens in chartjs-plugin-zoom Click-to-zoom sample (only sample with linear scale): https://www.chartjs.org/chartjs-plugin-zoom/latest/samples/wheel/click-zoom.html, even though it says be using 4.2.1.

@etimberg
Copy link
Member

etimberg commented Aug 1, 2023

@elinake thanks! Under the hood the zoom plugin changes the scale limits to do the zooming so I could see it breaking once a zoom starts

@stockiNail
Copy link
Contributor

@etimberg I think the best thing could be to revert the previous PR to solve this issue. Tee other issue (no blocking) needs more investigation, in my opinion. What do you think? if agree, I'll prepare the PR.

@etimberg
Copy link
Member

etimberg commented Aug 2, 2023

Sure, lets revert the PR. @LeeLenaleee what do you think about making that into a v4.3.3?

@LeeLenaleee
Copy link
Collaborator

Yeah its a bug fix so that would sound good to me

@stockiNail
Copy link
Contributor

@etimberg @LeeLenaleee PR #11432

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

Successfully merging a pull request may close this issue.

6 participants