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

Chart Update and ResetZoom #540

Closed
Nico-DF opened this issue Jun 7, 2021 · 1 comment · Fixed by #553
Closed

Chart Update and ResetZoom #540

Nico-DF opened this issue Jun 7, 2021 · 1 comment · Fixed by #553
Labels

Comments

@Nico-DF
Copy link

Nico-DF commented Jun 7, 2021

Hi,

As I continue to work around with chartjs and the zoom plugin, I encoutered an issue with the latest version of zoom plugin.

Description

According to https://www.chartjs.org/docs/latest/developers/updates.html, One can use the update() method after updating either/both data and options to redraw the graph.

But now, probably due to a bug introduced by latest versions of ChartZoom plugin, if you update the data/scale and then perform a reset zoom, the scale would be reseted to the 1st options while options are still correctly pointing to latest data

Reproduction

A sample is available here:
https://codepen.io/Nico-DF/pen/poeKrRV

Do the following:

  • Click on Update Data, it will update the data part and option containing scale and then use the update() function provided by Chartjs
  • The new scale is now 10-12 (previously 0-2)
  • Click on reset zoom
  • The scale will be reseted to 0-2.

Trivia

I could trace it back to some code called by the resetZoom() function, it will use:

const originalScaleLimits = storeOriginalScaleLimits(chart);

Which in turn, calls:

const {originalScaleLimits} = getState(chart);

The getState is a shorthand to take back data from some WeakMap

function getState(chart) {
  let state = chartStates.get(chart);
  if (!state) {
    state = {
      originalScaleLimits: {},
      handlers: {},
      panDelta: {}
    };
    chartStates.set(chart, state);
  }
  return state;
}

It turns out that the chartStates map in the originalScaleLimits still contains the data of the chart used by the constructor.

I still have not tested it yet, but I guess that doing a .destroy() and recreate the whole graph would work but it would defeat the purpose of the update method.
Moreover, I think that the behavior was correct a few version ago (not tested to roll back yet; and if the behavior that I have described is indeed a buggy one)

@kurkle kurkle added the bug label Jun 7, 2021
@Nico-DF
Copy link
Author

Nico-DF commented Jun 8, 2021

I checked as far as zoom v1.0.0-beta1 and chart 3.0.0 and the resetZoom behavior is the same, so it doesn't seem to be a regression

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

Successfully merging a pull request may close this issue.

2 participants