artus9033 / chartjs-plugin-dragdata

Draggable data points plugin for Chart.js
MIT License
257 stars 55 forks source link

Constraining drag not working correctly? #90

Open bdorninger opened 2 years ago

bdorninger commented 2 years ago

Describe the bug

Using chart.js 3.7.0, dragdata 2.2.4 Drag constrained by "onDrag" does not work as intended. If dragging a point, the constraint works as expected DURING drag (point keeps glued to upper/lower limit), but on dragEnd - if the cursor is beyond a limit - the point suddenly jumps to a "forbidden" value

To Reproduce I have the code (line chart) in a public github Also on stackblitz

The constraint of the drag is done via (index.ts:403) :

onDrag: function (e, datasetIndex, index, value: number | Point) {
      e.target.style.cursor = 'grabbing';
          if (typeof value === 'number') {
            return value >= 2 && value <= 19;
          }
          return value.y >= 2 && value.y <= 19;
        },

Expected behavior

If beyond a limit point during drag, the point shall stay there, regardless if the cursor moved further beyond on dragEnd.

chrispahm commented 2 years ago

Hey @bdorninger,

That is indeed strange, I tried to reproduce with a simple example, but unfortunately couldn’t: https://jsfiddle.net/92vwpr31/2/

Maybe it's the combination of the zoom/crosshair/drag-data pluging that is causing this, but I'm just guessing for now

bdorninger commented 2 years ago

Hi chris

Thx for your quick response It's not the other plugins, because the error persists when removing the other plugins.

After further looking into it , I heavily suspect the usage of object data being the cause. Your fiddle uses number arrays for Y and fixed categories for the X axis. My sample works with point data objects {x: number, y: number} on having linear axes for both x and y (https://www.chartjs.org/docs/latest/general/data-structures.html#object)

Forked your fiddle to : https://jsfiddle.net/w8fj2hm1/ , modified it and could reproduce the issue

I suppose, plugin code on releasing the drag assumes data to be numbers only.

Hey @bdorninger,

That is indeed strange, I tried to reproduce with a simple example, but unfortunately couldn’t: https://jsfiddle.net/92vwpr31/2/

Maybe it's the combination of the zoom/crosshair/drag-data pluging that is causing this, but I'm just guessing for now

chrispahm commented 2 years ago

Thanks @bdorninger for digging into this! The "object theory" would somewhat make sense, could be that we need to make a deep copy of the object to make sure it's not modified afterwards... I'll have a look

chrispahm commented 2 years ago

Alright, I now made a minor change creating a deep copy of the original data object. That solves the bug on my end, would be nice if you could confirm! chartjs-plugin_dragdata-v2.2.5_beta.zip

Best Chris

bdorninger commented 2 years ago

Looks good! I had done a similar change in the meantime, which also solved it:

const dp = chartInstance.data.datasets[datasetIndex].data[index];
const dataPoint = typeof dp === 'object' ? { ...dp } : dp;

Thx for your quick help!

regards, Bernhard

chrispahm commented 2 years ago

Yes that's great, I just did a quick JSON.stringify(JSON.parse())

bdorninger commented 2 years ago

..yours is a real deep copy, mine is just flat ;-)

chrispahm commented 2 years ago

The flat copy may have the benefit of not stripping any getters/setters from the original object. Not sure if it's a common pattern to use them in this context but I guess it could be annoying if the plug-in would unintentionally remove those

ggomaeng commented 1 year ago

@chrispahm Is this fix available on npm? Facing the same issue in version 2.2.5

ggomaeng commented 1 year ago

update: forked the repo and fixed it myself, but it would be nice if it's published on npm 👍