feat: allow add plugins to the Chart and Dataset components #1638

rgah2107 · 2020-06-17T10:04:27Z

fix: #1626

Changes proposed in this PR:

allow add plugins to the Chart and Dataset components
I have followed (at least) the PR section of the contributing guide.

@nexxtway/react-rainbow

fix: nexxtway#1626

commit-lint · 2020-06-17T10:04:31Z

Features

allow add plugins to the Chart and Dataset components (a7845c6)

Bug Fixes

modify component according to the last comments (2b9d212)
modify solution to unregister global plugins (b885ae8)
modify test (971daca)
modify component according to the established api (47e5961)
improve helper (6112680)

Contributors

@rgah2107

Commit-Lint commands

You can trigger Commit-Lint actions by commenting on this PR:

@Commit-Lint merge patch will merge dependabot PR on "patch" versions (X.X.Y - Y change)
@Commit-Lint merge minor will merge dependabot PR on "minor" versions (X.Y.Y - Y change)
@Commit-Lint merge major will merge dependabot PR on "major" versions (Y.Y.Y - Y change)
@Commit-Lint merge disable will desactivate merge dependabot PR
@Commit-Lint review will approve dependabot PR
@Commit-Lint stop review will stop approve dependabot PR

reiniergs · 2020-06-18T01:08:26Z

package.json

@@ -33,6 +33,7 @@
    "dependencies": {
        "autosize": "^4.0.2",
        "chart.js": "2.7.3",
+        "chartjs-plugin-datalabels": "^0.7.0",


this dependency should be only for examples purposes, move it to devDependency

reiniergs · 2020-06-18T01:14:36Z

src/components/Chart/index.d.ts

@@ -17,6 +17,8 @@ export interface ChartProps extends BaseProps {
    disableXAxisTickLabels?: boolean;
    disableYAxisTickLabels?: boolean;
    maintainAspectRatio?: boolean;
+    plugins?: Array<any>;
+    datalabels?: object;


the plugins feature is for any possible plugin, right? which means there will be more props depending on the plugins passed in plugins prop, I would say we need to find some dynamic manner of defining those props.

i.e. a plugin named foo will receive it's config data through foo prop

import foo from 'chart-plugin-foo'; <Chart plugins={{ foo }} foo={fooConfigAtChartLevel}> <Dataset foo={fooConfigAtDatasetLevel}> </Chart>

reiniergs · 2020-06-18T01:17:37Z

src/components/Chart/index.js

-        const { type, labels, ...conditions } = this.props;
+        const { type, labels, plugins, ...conditions } = this.props;
+        if (plugins) {
+            ChartJS.plugins.unregister(plugins);


why we have to unregister the plugins?? I know the labels one needs to be unregistered ... but I guess that's not the case for all the plugins, right?

Again this feature should a mechanism to inject any possible chart plugin

I was looking at this.

The problem of globally registered plugins is solved like this:

ChartJS.plugins.clear();

this must be done in everything always, because when it is done

import datalabels from 'chartjs-plugin-datalabels'; import deferred from 'chartjs-plugin-deferred';

All ChartJS already have that plugin registered.

all global plugins can be used inline

{ plugins: [ datalabels, deferred ], }

ChartJS.plugins.clear();

is the way to keep ChartJS plugins isolated

yvmunayev · 2020-07-04T06:21:44Z

src/components/Chart/index.js

@@ -55,6 +59,7 @@ export class Chart extends Component {
                labels,
                datasets: this.datasets,
            },
+            plugins: plugins || null,
            options: resolveOptions({ type, ...conditions }),


you should add the plugins variable here, because with the line

const { type, labels, plugins, ...conditions } = this.props;

conditions no longer contains plugins

yvmunayev · 2020-07-04T06:23:09Z

src/components/Chart/readme.md

+
+const datalabelsLevelChart = {color: '#fe4849', anchor: 'end', align: 'top'};
+const deferredChart = {delay: 500};
+const pluginsChartConf = {datalabels: datalabelsLevelChart, deferred: deferredChart};
+
+const datalabelsLebelDataSet_0 = {color: '#ff6837'};
+const datalabelsLebelDataSet_2 = {color: '#01b6f5'};
+const pluginsDatasetConf_0 = {datalabels: datalabelsLebelDataSet_0};
+const pluginsDatasetConf_2 = {datalabels: datalabelsLebelDataSet_2};
+
+const containerStyles = {
+    maxWidth: 600,
+};


check lint rules

yvmunayev · 2020-07-05T04:35:18Z

src/components/Chart/index.js

@@ -49,7 +48,7 @@ export class Chart extends Component {
    renderChart() {
        const { type, labels, plugins, ...conditions } = this.props;
        if (plugins) {
-            ChartJS.plugins.unregister(getGlobalsPlugins(plugins));
+            ChartJS.plugins.clear();


this must always be done in order to isolate the component so that the global plugins do not affect it.

yvmunayev · 2020-07-07T08:13:37Z

src/components/Chart/index.d.ts

+    pluginsChartConf?: object;
+    pluginsDatasetConf?: object;


remove this props from the API. the component has to be like this:

#1626

yvmunayev · 2020-07-10T07:21:36Z

src/components/Chart/helpers/getPluginsConf.js

+    const keys = plugins.map(plugin => plugin.id);
+    keys.forEach(key => {
+        if (rest[key]) {
+            pluginsConf[key] = rest[key];
+        }
+    });


that's easier

const pluginsConf = {}; plugins.forEach(plugin => { const { id } = plugin; if (rest[id]) { pluginsConf[id] = rest[id]; } });

maxxgreene · 2020-07-11T03:07:25Z

src/components/Chart/index.js

@@ -46,7 +47,8 @@ export class Chart extends Component {
    }

    renderChart() {
-        const { type, labels, ...conditions } = this.props;
+        unregisterGlobalPLugins(ChartJS);


unregisterGlobalP(L)ugins => unregisterGlobalPlugins

maxxgreene · 2020-07-11T03:09:05Z

@rgah2107 @yvmunayev it looks good to me, please change the typo in other PR, I will merge it

feat: allow add plugins to the Chart and Dataset components

a7845c6

fix: nexxtway#1626

rgah2107 requested review from TahimiLeonBravo and LeandroTorresSicilia June 17, 2020 10:05

reiniergs reviewed Jun 18, 2020

View reviewed changes

fix: modify component according to the last comments

2b9d212

rgah2107 requested a review from reiniergs June 18, 2020 11:48

yvmunayev reviewed Jul 4, 2020

View reviewed changes

fix: modify solution to unregister global plugins

b885ae8

rgah2107 requested a review from yvmunayev July 5, 2020 03:48

yvmunayev reviewed Jul 5, 2020

View reviewed changes

fix: modify test

971daca

yvmunayev approved these changes Jul 6, 2020

View reviewed changes

TahimiLeonBravo assigned yvmunayev Jul 6, 2020

yvmunayev reviewed Jul 7, 2020

View reviewed changes

fix: modify component according to the established api

47e5961

yvmunayev reviewed Jul 10, 2020

View reviewed changes

fix: improve helper

6112680

yvmunayev approved these changes Jul 10, 2020

View reviewed changes

maxxgreene reviewed Jul 11, 2020

View reviewed changes

maxxgreene merged commit 58b660c into nexxtway:master Jul 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: allow add plugins to the Chart and Dataset components #1638

feat: allow add plugins to the Chart and Dataset components #1638

rgah2107 commented Jun 17, 2020

commit-lint bot commented Jun 17, 2020 •

edited

Loading

reiniergs Jun 18, 2020

reiniergs Jun 18, 2020

reiniergs Jun 18, 2020

yvmunayev Jul 4, 2020

yvmunayev Jul 4, 2020 •

edited

Loading

yvmunayev Jul 4, 2020

yvmunayev Jul 4, 2020 •

edited

Loading

yvmunayev Jul 5, 2020

yvmunayev Jul 7, 2020

yvmunayev Jul 10, 2020

maxxgreene Jul 11, 2020

maxxgreene commented Jul 11, 2020

feat: allow add plugins to the Chart and Dataset components #1638

feat: allow add plugins to the Chart and Dataset components #1638

Conversation

rgah2107 commented Jun 17, 2020

Changes proposed in this PR:

commit-lint bot commented Jun 17, 2020 • edited Loading

Features

Bug Fixes

Contributors

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yvmunayev Jul 4, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yvmunayev Jul 4, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxxgreene commented Jul 11, 2020

commit-lint bot commented Jun 17, 2020 •

edited

Loading

yvmunayev Jul 4, 2020 •

edited

Loading

yvmunayev Jul 4, 2020 •

edited

Loading