search

artus9033 / chartjs-plugin-dragdata

Draggable data points plugin for Chart.js
MIT License
266 stars 56 forks source link

Added custom interaction mode option #115

Closed Ruanrls closed 1 month ago

Ruanrls commented 1 year ago

Problem

Sometimes we need different ways to search for the points to attach the drag. For example, when we have two different datasets, one is behind. We need to be able to use a custom mode to search between these two datasets.

This PR also introduces proper tests, and fixes dragData per-axis option disabling dragging also on other axes.

Solution

Add any way to use custom interaction way which developers can implement while using react chart js.

References

Custom interaction modes

Ruanrls commented 11 months ago

Bump @chrispahm

artus9033 commented 1 month ago

Thanks for catching this & creating a PR @Ruanrls! The only thing I am concerned about is why in the PR you proposed to provide the interaction variant via drag data plugin's options, while - as stated in the docs that you linked - it is already configured directly in the chart's configuration under options.interaction.mode. I believe it would be better to be consistent with the interaction behaviour (e.g. the tooltip) and use the same configuration property; moreover, I don't see any actual use case in a different situation, but maybe I am missing a point. WDYT?

In the meantime, I merged the current master branch & reformatted a bit your code. Also, I applied the change I described above - in case nobody brings an argument for creating a separate option for that in plugin configuration, I will test & merge the new version which uses the chart's interaction mode.

Ruanrls commented 1 month ago

Thanks for catching this & creating a PR @Ruanrls! The only thing I am concerned about is why in the PR you proposed to provide the interaction variant via drag data plugin's options, while - as stated in the docs that you linked - it is already configured directly in the chart's configuration under options.interaction.mode. I believe it would be better to be consistent with the interaction behaviour (e.g. the tooltip) and use the same configuration property; moreover, I don't see any actual use case in a different situation, but maybe I am missing a point. WDYT?

In the meantime, I merged the current master branch & reformatted a bit your code. Also, I applied the change I described above - in case nobody brings an argument for creating a separate option for that in plugin configuration, I will test & merge the new version which uses the chart's interaction mode.

Hey @artus9033. I don't remember exactly why I implemented this way (it was a very long time ago, sorry). But I remember that the issue was when using the dragdata plugin and you have two points (one behind the other), the drag data don't select correctly the point to move, and all the points stays in place, without moving (even if using options.interaction.mode. If I'm not mistaken).

To reproduce that you can simple create a chart, add drag data and try moving the two points at the same place.

artus9033 commented 1 month ago

Sure, thanks for the response @Ruanrls. I see, then I changed the code to pass the options from chart configuration.