Improvements to data labels #103
Conversation
- Create a macro that could be repurposed by the design system
- Tidy up and refactor all the JavaScript
- Don't include the annotations in the chart config from the back-end, and instead build this in the JavaScript to create the arrows
- Fix missing annotations JS
…t for cluster charts
… or outside the bar, and a bit of tidying up of function declarations
…abels-stacked-chart
| (this.chartType === 'bar' && | ||
| this.useStackedLayout === false && | ||
| this.apiConfig.series.length > 2) || | ||
| this.useStackedLayout === true; |
There was a problem hiding this comment.
Question: Are stacked and clustered the only two possibilities? And useStackedLayout === false == clustered?
There was a problem hiding this comment.
Could be a simple bar without being stacked or clustered, or could be a clustered bar chart with 2 series - in either of those cases hideDataLabels would be false.
There was a problem hiding this comment.
For stacked layouts we always want to hide data labels.
| this.postLoadDataLabels(event); | ||
| if (!hideDataLabels) { | ||
| this.postLoadDataLabels(event); | ||
| } |
There was a problem hiding this comment.
Question: Previously were we calling postLoadDataLabels for any bar, even if we'd set series.dataLabels.enabled to false?
There was a problem hiding this comment.
Previously we hid them for clustered bar charts with more than 2 series - this updates to hide them for stacked bar charts too.
| const labelWidth = point.dataLabel && point.dataLabel.absoluteBox.width; | ||
| // Move the data labels inside the bar if the bar is wider than the label | ||
| if (point.shapeArgs.height > labelWidth) { |
There was a problem hiding this comment.
Praise: Gotcha. Much better. Good change.
What is the context of this PR?
How to review
Follow-up Actions
List any follow-up actions (if applicable), like needed documentation updates or additional testing.