chartjs / chartjs-plugin-datalabels

Chart.js plugin to display labels on data elements
https://chartjs-plugin-datalabels.netlify.app
MIT License
863 stars 460 forks source link

Bar Graph Label Position Not Consistent Depending on Execution Order #332

Closed Whobeu closed 1 year ago

Whobeu commented 1 year ago

I have a strange bug involving labels on bar graphs. If the first graph that I plot in a series of plots is a bar graph that I want to have labels placed at the top of the bar, I have to use "anchor: end" in conjunction with "align: top", which appears to be the correct setting for what I want to do. This also seems to work fine for line graphs. However, if I draw a line graph first and then follow it with a bar graph, I have to change "end" to "start" in order to get the labels at the top of the bar. Line graphs do not appear to be affected by either "start" or "end" nor do they exhibit the issue with being first that bar graphs exhibit.

The configuration I am using follows and has "anchor: start" as I usually have a bar graph created after a line graph so it works as I want it to:

        datalabels: {
          anchor: "start",
          align: "top",
          color: ["rgb(17, 13, 100)"],
          font: {
            size: 10
          },
          display: (context) => {
            return (context.dataset.data[context.dataIndex] !== 0);
          },
          formatter: (value, context) => {
            switch (field) {
              case "TMP":
              case "DPT":
                return context.dataset.data[context.dataIndex]?.toString().concat("°");
              //NOTE: "kt" is the preferred abbreviation, vs "kn", for aviation.
              // The "kt" gets crowded on the graph causing overlaps in the
              // labels. Some kind of field specific padding should be created.
              // For now the size 10 font handles up to 99.
              case "WSP":
              case "WGS":
                return context.dataset.data[context.dataIndex]?.toString().concat("kt");
              case "PPO":
              case "LP1":
              case "CP1":
                return context.dataset.data[context.dataIndex]?.toString().concat("%");
              default:
                return context.dataset.data[context.dataIndex];
            }
          }
        },

Using the above configuration, if I run a bar graph followed by a line graph, I get the following: Bad

But if I leave the configuration the same and run the line graph first followed by the bar graph, I get the following graph which is what I want: Good I have checked this over for some kind of variable leakage but there is only one global referenced by the function generating the graph and it is used to return text unrelated to the labels.

Whobeu commented 1 year ago

I finally had some time to dig into the issue to see why it was failing. The problem turns out to be here:

function getPositioner(el) {
  if (el instanceof chart_js.ArcElement) {
    return positioners.arc;
  }
  if (el instanceof chart_js.PointElement) {
    return positioners.point;
  }
  if (el instanceof chart_js.BarElement) {
    return positioners.bar;
  }
  return positioners.fallback;
}

After the first graph is created during the run, all subsequent "instanceof" checks fail so the return is always "positioners.fallback". Inspecting "el" it is in fact "BarElement" just not an instance of chart_js.BarElement. I modified the function as follows and it now works as it should. Of course there are downsides to this method (minification for example) just as there are to "instanceof":

  function getPositioner(el) {
    if (el.constructor.name === "ArcElement") {
      return positioners.arc;
    }
    if (el.constructor.name === "PointElement") {
      return positioners.point;
    }
    if (el.constructor.name === "BarElement") {
      return positioners.bar;
    }
    return positioners.fallback;
  }
simonbrunel commented 1 year ago

@whobeu it looks similar to #251. A fix was suggested in this comment, maybe that the same problem in your case. If not, please provide a way to reproduce your issue.

Whobeu commented 1 year ago

Yes, it look like #251. Changing to the require('chart.js/auto') call as the above linked comment suggests works. I am using Chart.js in Node through chartjs-node-canvas so a permanent fix will need to be implemented by the maintainer of that package. Making the change in my copy of the module solves the issue.

I had come up with a workaround that solved the issue as well. Rather than creating a new instance ChartJSNodeCanvas on every call to my method, I created a global instance in my module. It still worked fine with graphing and the data labels were now correctly positioned as every call to getPositioner successfully found chart_js.BarElement was the instance. I will let the maintainer of ChartJSNodeCanvas of the change that should be made. My current workaround actually cuts down on the overhead of creating a new canvas for every call of my method. Serendipity I suppose.