search

chartjs / chartjs-plugin-annotation

Annotation plugin for Chart.js
MIT License
603 stars 325 forks source link

Enable callout in the label of line annotation #740

Closed stockiNail closed 2 years ago

stockiNail commented 2 years ago

This PR is enabling the callout options in the label of line annotation.

TODO

stockiNail commented 2 years ago

@kurkle I have a doubt about CC issue. We could need an object to create and add to element.defaults but where? Maybe a additional file "callout.js", which contains all functions to manage the callout? Maybe this could be applied to the labels and arrowHeads (to reduce the size of the js files and close some CC issues). Just a proposal.

EDIT: solved importing LabelAnnotation class.

stockiNail commented 2 years ago

@kurkle I solved CC duplication code issue using the defaults of the label annotation for callout.

kurkle commented 2 years ago

@kurkle I solved CC duplication code issue using the defaults of the label annotation for callout.

I think the common should now work for these shared defaults.

stockiNail commented 2 years ago

I think the common should now work for these shared defaults.

I wasn't sure. Let me try

stockiNail commented 2 years ago

@kurkle I solved CC duplication code issue using the defaults of the label annotation for callout.

I think the common should now work for these shared defaults.

I have removed the callout options from label ( leaving ONLY the callout node) and adding them in the common, you don't have any fallback. In my opinion, but maybe I'm wrong, when the options are loaded in the element, only defaults and defaultRoutes of the element are used (and not the common).

Therefore to work, we should maintain all callout options and setting to undefined to have the fallback but only to centralize the defaults, not the options.

stockiNail commented 2 years ago

@LeeLenaleee @kurkle I have marked as ready for review. Nevertheless we should NOT merge it till mid/end of next week, giving at least 2 weeks to version 2.0.0