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

Enable callout in the label of line annotation #740

Merged
merged 16 commits into from
Sep 28, 2022
Merged

Enable callout in the label of line annotation #740

merged 16 commits into from
Sep 28, 2022

Conversation

stockiNail
Copy link
Collaborator

@stockiNail stockiNail commented May 19, 2022

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

TODO

  • test cases
  • types
  • documentation
  • samples

@stockiNail stockiNail added this to the 2.0.0 milestone May 19, 2022
@stockiNail
Copy link
Collaborator Author

stockiNail commented May 19, 2022

@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 stockiNail marked this pull request as ready for review May 19, 2022 19:16
@stockiNail
Copy link
Collaborator Author

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

@kurkle
Copy link
Member

kurkle commented May 23, 2022

@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
Copy link
Collaborator Author

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

I wasn't sure. Let me try

@stockiNail
Copy link
Collaborator Author

@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 stockiNail modified the milestones: 2.0.0, 2.1.0 Jun 3, 2022
@stockiNail stockiNail marked this pull request as draft July 21, 2022 15:47
@stockiNail stockiNail marked this pull request as ready for review August 4, 2022 13:12
@stockiNail
Copy link
Collaborator Author

stockiNail commented Aug 4, 2022

@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 stockiNail merged commit 1cfbfc6 into chartjs:master Sep 28, 2022
@stockiNail stockiNail deleted the lineLabelCallout branch September 28, 2022 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants