Add formatter option

[stockiNail]

Fixes #41
Fixes #24
Fixes #20

Ref #38, #44

This PR adds formatter options in order to format the label to show on the treemap elements.

The PR changes also the label option defintion in order to be align to the dataset label option of all other controllers.

[kurkle]

I had the feeling, but never got around to test. The label property is scriptable already, but labels are only drawn when the data is grouped:
https://codepen.io/kurkle/pen/QWMwZMw

[stockiNail]

@kurkle yes, I discovered today that ;)

Nevertheless label is already an option for dataset which defines a name to it. And it is used on the legend as well (when the legend is enabled).
For this reason I have added another option to format the labels for each element (to be aligned with datalabels plugin as well).

The implementation of this PR enables also the possibility to add and format labels for no-group data.

Make sense to you?

[stockiNail]

https://codepen.io/kurkle/pen/QWMwZMw

If you comment the legend: false,, this is the result:

This is the reason why I think label shouldn't be used for this purpose.

[stockiNail]

@kurkle I have committed first changes to go to the direction you proposed.

There are some behaviors that I don't know why there are happening:

• controller.js at line 125 I had to add an additional control about the context because, testing it, the callback was called with an undefined context: if (!labelsOpts || !labelsOpts.display || !(rect.$context)) {
• controller.js at line 129-130 I had to add the callback invocation otherwise, using groups, the reference to formatter wasn't resolved as callback but I have got the function (and not the result of function): label = typeof label === 'function' ? callCallback(label, [rect.$context]) : label;

These behavoirs are weird...
I've pushed anyway because maybe you know where I'm making any mistake.

Furthermore, due to my low knowledge, I have defined the types of labels as following:

type TreemapControllerDatasetLabelsOptions = {
  display?: boolean;
  formatter?: Scriptable<string | Array<string>, ScriptableContext<'treemap'>>
}
...
export interface TreemapControllerDatasetOptions<DType> {
 ...
 labels?: TreemapControllerDatasetLabelsOptions;
 ...
}

When I test the types, it seems that the context is not recognized and I have got the following error:

types/tests/options.ts(57,18): error TS2571: Object is of type 'unknown'.

Maybe you can help me.

[stockiNail]

@kurkle I have tested the Chart.js master with PR chartjs/Chart.js#9758 in order to understand if some behaviors are changed.

What I see is when you configure the treemap chart to use groups, and only enable the labels

data: {
  datasets: [{
    tree: statsByState,
    key: 'area',
    groups: [],
    spacing: -0.5,
    borderWidth: 0.5,
    borderColor: 'rgba(200,200,200,1)',
    hoverBackgroundColor: 'rgba(220,230,220,0.5)',
    labels: {
      display: true,
    }
  }]
},

but you want to use the default formatter, the formatter property is not resolved and returns the option as function, only at the first draw, because hovering the elements the label is well drawn.

I'm still investigating...

[kurkle]

Did you check the changes I proposed in https://github.com/pepstock-org/chartjs-chart-treemap/pull/1/files ?
Specifically the descriptors are needed for scriptable options to be resolved

[stockiNail]

nooooo... I didn't... apologize. I'm gonna apply your updates and test it again,

[stockiNail]

Did you check the changes I proposed in https://github.com/pepstock-org/chartjs-chart-treemap/pull/1/files ?

Done!

Specifically the descriptors are needed for scriptable options to be resolved

Before seeing your PR, I was playing with descriptors too.

Nevertheless it does still not working.
I use the us-switchable.html sample, adding the following to datasets options:

    labels: {
      display: true,
    }

With this config, it should use the default formatter callback.

I have also changed utils.js in the samples folder in order to load Chart.js master:

(function(Utils) {
	//const chartjsUrl = 'https://cdn.jsdelivr.net/npm/chart.js/dist/chart.js';
	const chartjsUrl = 'https://www.chartjs.org/dist/master/chart.js';
        .....

Opening the sample by FF, this is the result:

[kurkle]

chartjs/Chart.js#9770

[kurkle]

Nevertheless it does still not working.

Just verified it works with 9770 merged to master

[stockiNail]

@kurkle thanks a lot! let me try again. feedback soon

[stockiNail]

Yes, it works! THANK YOUUU! I'd like to change the package.json setting Chart.js 3.5.1 or more (to change when Chart.js 3.6.0 will be published). Make sense?
Let me also add additional test cases before completing the PR.

Another thoughts: does it make sense to use the option named formatter or could be better to use content (like in the Chart.js for title and subtitle, or in annotation plugin)?

[kurkle]

Need to update the package.lock too (just run npm i)
Are you happy with this already, or should the alignment with chart.js and I plugins be re-checked still?

[stockiNail]

Need to update the package.lock too (just run npm i)

I didn't commit it... I do.

Are you happy with this already, or should the alignment with chart.js and I plugins be re-checked still?

In my opinion, this PR should address the issues above mentioned. To align to other plugins, maybe it's better another PR to add additional options.
About formatter option name, I think we could leave it now.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication