Allow category scales to use locally-set labels

[AlbinoDrought]

Instead of only using data.labels, data.xLabels, or data.yLabels for category scales, also allow users to set labels on the scale config itself.

Example config:

        {
          type: 'category',
          position: 'right',
          
          labels: ["Different1", "Different2"]
        }

I attempted to update the documentation as specified in CONTRIBUTING.md, but I am not sure if it is understandable enough.

I've also left the changed function as a one-liner. I'm not sure if it is still okay, or if it should be expanded due to length now.

This implements/fixes #3725

Example functional codepen copy of the issue:

http://codepen.io/albinodrought/pen/ObGVmY

[etimberg]

Code looks fine to me. Can you write a test for this as well?

[AlbinoDrought]

Added a test similar to the one above it, named "Should generate ticks from the scale labels"

Confirms scale.ticks is equal to config.labels, which is where we get our label data from

[etimberg]

awesome thanks. One last thing, I noticed we use yLabels here will that be a cause for concern with this change? Tbh, I don't know why that line exists 😕

[AlbinoDrought]

Hey, it seems like that line will display the Y-value in the tooltip instead of the value from the category scale (label), if and only if the scale is vertical.

If data.yLabels and options.labels is set, the Y value is shown on the tooltip: http://codepen.io/albinodrought/pen/rWbeVM

If data.yLabels is not set, and options.labels is set, the label is shown on the tooltip:
http://codepen.io/albinodrought/pen/eBoZNz

This is unexpected behavior (not sure how expected it is to get the y-value if on the y-axis, but I think that is outside of this PR).

A possible fix for this could be changing if (data.yLabels && !isHorizontal) { to if (!me.labels && data.yLabels && !isHorizontal) {. Not sure how much I like it though... what do you think?

[etimberg]

I personally think your second fiddle is the most correct behaviour. Based on d39ac38 which removed the x check, can we just go ahead and remove this entire if statement and just do return me.ticks[index - me.minIndex]; ?

This is the original issue where this was added #3278 if we can confirm that this is still fixed after removing this then it is safe to do.

[etimberg]

I looked back at the old commit: the reason it was needed is because getLabelForIndex needs to return me.ticks[index-minIndex] in the case where the axis is horizontal. When it's vertical, it needs to return the value for the data. This is because the x value is implied by the index but the y value is based on the value in the dataset.data array

[AlbinoDrought]

@etimberg I've changed that line to resemble previous functionality.

Here is a sample from before the change (same as above): http://codepen.io/albinodrought/pen/eBoZNz

Here is a sample from after the change: http://codepen.io/albinodrought/pen/XNQqdw

I've left the data.yLabels check in there in case there is an edgecase I'm missing. It currently looks like this:

			if ((me.options.labels || data.yLabels) && !isHorizontal) {
				return me.getRightValue(data.datasets[datasetIndex].data[index]);
			}

			return me.ticks[index - me.minIndex];

With this, if only data.labels is set, the labels instead of the values will be shown.

Would something like this make more sense? This would show the values whenever the axis is not horizontal, which makes sense to me - but I might be missing something.

			if (!isHorizontal) {
				return me.getRightValue(data.datasets[datasetIndex].data[index]);
			}

			return me.ticks[index - me.minIndex];

[etimberg]

I don't think the condition needs to change. Changing it might cause problems for the horizontal bar chart where the y axis is the primary axis.

The solution to this might just mean we need to introduce the concept of a "primary" axis direction (horizontal vs vertical) for a chart. Most charts would have horizontal as a primary direction except for the horizontal bar chart. Making those changes is outside the scope of this change though.

CC @chartjs/maintainers for further review before this is merged

[andig]

ping @etimberg can be closed, I'll fix the remaining issue with another PR.

[etimberg]

Closing per @andig