search

chartjs / chartjs-plugin-annotation

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

Throw error when non-existing scale id is used #518

Closed kurkle closed 2 years ago

kurkle commented 2 years ago

Closes: #468

Updated the Chrome flags so it would not timeout when in background.

stockiNail commented 2 years ago

@kurkle I think this should be documented. Sorry if I recognized it just now.

stockiNail commented 2 years ago

@kurkle maybe I'm wrong but it should be marked as breaking change. I think before the chart was shown anyway.

kurkle commented 2 years ago

@stockiNail the issue was marked as bug, so this can be considered to be a bug fix. Although this could be breaking in some use cases, the annotations did not always show before either (see the issue).

Should be mentioned in the release notes when 1.2 is released, but I'm not sure this needs to be documented. I don't think it was ever a valid use case to specify a non-existing scale id for an annotation.

stockiNail commented 2 years ago

@kurkle thank you! Agree!

stockiNail commented 2 years ago

Sorry @kurkle... I don't want to boring you, but having a look to the doc (for instance box annotation, https://www.chartjs.org/chartjs-plugin-annotation/latest/guide/types/box.html#configuration), I think the sentence

If one of the axes does not match an axis in the chart, the box will take the entire chart dimension.

should be removed. If ok for you, I can do it.

kurkle commented 2 years ago

Oh, did not see that. Looks like that actually is a supported use case, so this is a breaking change. I'll deal with it tomorrow.