-
Notifications
You must be signed in to change notification settings - Fork 328
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 centered label drawing for doughnut controllers #825
Conversation
I don”t think the innerlabel is needed. All can be mamaged by annotation plugin. New updates asap |
About the animations, without reading any code; The element and its options were designed to be the "view", containing currently displayed value for everything. I'm guessing you are using an array of colors for the lines of text? In this "philosophy", each line should be an "element", containing its resolved current display values. I put quotation marks around the element, because from the animations point of view, it can be any object. Now, I'm out of context wit the annotations plugin, but I think something like sub elements is a thing or was it abandoned? |
yes, that's correct
I understand the philosophy and I agree. But a value is a value, also an array could be a value ;), therefore the implementation it should not be against of that philosophy, I guess.
I have to admit that this use case forced me to go more in deep how animation is working and I have learned more, that's good! Nevertheless, staying on this use case and the plugin itself, the animation was thought (at least for what I had understood) only for the properties of the elements and not for their options. In fact we have only 1 animations configuration, based on the properties of the elements. And, in my opinion, doesn't make sense to extend it to the options (like color, bordeColor, etc.). And this animations setup (for colors) is coming from the controller which is implementing that, and plugin animations config is falling back to that.
It's still present for all labels and the points (for polygons). Nothing is changed. But, as written above, it doesn't seem to make sense to have sub-elements for each row. I think good compromise is what I described above, assign options to element after animation item creation.
Ok... may I ask what it means exactly? is it just a period and it will be forever? I'm worried that you will leave the project... 😟 |
Yes, it might not be appropriate. Though then you could have hover / click effects for each row etc :) Another option could be to use something else, like
It jus means its been long enough that I don't remember how things are anymore (hence the question about sub elements). |
I put it in my TODO. I will have a look in next days. I think it could make sense, only for color option.
Personally, I don't like it because it's a deviation of the other plugins and chart.js itself where they are using color for text color. And a breaking change as well.
Fiuuuu... 😄 ... I had some time during the Christmas period but starting from next Monday, I will not have much time as well. |
Anyway, I'm still working on this PR and I'd like to have something really consistent to show you asap (apart the discussion about animation). I have refactored it during last 2 days to provide the background inside the doughnut controller (and not the box around the label which doesn't make sense). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some quick comments, but this is not a proper review.
Conflicts: src/helpers/helpers.options.js types/options.d.ts
@kurkle apologize because your approval was dismissed. PR had to be rebased |
Conflicts: README.md docs/.vuepress/config.js docs/guide/index.md
@kurkle what would be the steps needed to get this merged? 🤩 |
@LeeLenaleee I have rebased. Ready for review, when you have time. |
Wow! Are donut center labels now supported natively? 🤩 |
Not yet, there is no version released yet, I have made a bump pr so we can release one: |
A prototype.
This PR is enabling the possibility to add an inner label in the middle of a doughnut controller.
This is leveraging on the already label calculation and drawing used for label annotation.
SAMPLE chart configuration:
TODO