chartjs / chartjs-plugin-annotation

Annotation plugin for Chart.js
MIT License
607 stars 328 forks source link

Remove `point` node from label annotation #556

Closed stockiNail closed 2 years ago

stockiNail commented 2 years ago

Currently label annotation is taking care also of the point drawing, the point selected with coordinates calculated by x/yVaue.

To simplify the label annotation, reducing duplication of similar code and options, the point node could be removed. If a user wants to draw the selected point, could add another annotation, a point one, with the same x/yVaue.

This is also enabling a common approach if a user wants to draw a polygon instead of a point.

As is now:

const labelAndPoint = {
  type: 'label',
  backgroundColor: 'rgba(245,245,245, 0.5)',
  content: (ctx) => 'Maximum value is ' + maxValue(ctx).toFixed(2),
  point: {
    enabled: true,
    backgroundColor: 'transparent',
    borderColor: (ctx) => ctx.chart.data.datasets[0].borderColor,
    pointStyle: 'rectRounded',
    radius: 10
  },
  xValue: 0,
  yValue: 0
};

As to be:

const label = {
  type: 'label',
  backgroundColor: 'rgba(245,245,245, 0.5)',
  content: (ctx) => 'Maximum value is ' + maxValue(ctx).toFixed(2),
  xValue: 0,
  yValue: 0
};
const point = {
  type: 'point',
  backgroundColor: 'transparent',
  borderColor: (ctx) => ctx.chart.data.datasets[0].borderColor,
  pointStyle: 'rectRounded',
  radius: 10,
  xValue: 0,
  yValue: 0
};

This shouldn't be a breaking change because label annotation is not releases yet in any version.

@kurkle what do you think?

kurkle commented 2 years ago

Its more config for the user for same behavior. I think its still probably best to remove the point now, before we release 1.2, so we can have more options if we want to later add something similar.

stockiNail commented 2 years ago

yes, you're right about more config for user but it sounds to me more coherent.