chartjs-plugin-annotation icon indicating copy to clipboard operation
chartjs-plugin-annotation copied to clipboard

Enable callout in the label of line annotation

Open stockiNail opened this issue 3 years ago • 6 comments

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

TODO

  • [x] test cases
  • [x] types
  • [x] documentation
  • [x] samples

stockiNail avatar May 19 '22 08:05 stockiNail

~~@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 avatar May 19 '22 19:05 stockiNail

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

stockiNail avatar May 22 '22 17:05 stockiNail

@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.

kurkle avatar May 23 '22 09:05 kurkle

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

I wasn't sure. Let me try

stockiNail avatar May 23 '22 10:05 stockiNail

@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 avatar May 23 '22 10:05 stockiNail

@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

stockiNail avatar Aug 04 '22 13:08 stockiNail