Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for detached canvas element #4591

Merged
merged 2 commits into from Aug 1, 2017

Conversation

simonbrunel
Copy link
Member

@simonbrunel simonbrunel commented Jul 31, 2017

Allow to create a chart from a canvas not yet attached to the DOM (detection based on CSS animations described in https://davidwalsh.name/detect-node-insertion). The resize element (IFRAME) is added only when the canvas receives a parent or when style.display changes from none. This change also allows to re-parent the canvas under a different node (the resizer element following). This is a preliminary work for the DIV based resizer.

Fixes #3790
Fixes #4605

Allow to create a chart on a canvas not yet attached to the DOM (detection based on CSS animations described in https://davidwalsh.name/detect-node-insertion). The resize element (IFRAME) is added only when the canvas receives a parent or when `style.display` changes from `none`. This change also allows to re-parent the canvas under a different node (the resizer element following). This is a preliminary work for the DIV based resizer.
});
});

it('should resize the canvas if attached to the DOM after construction', function(done) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[minor] it looks like this test has the same name as the one before

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thanks!

@simonbrunel simonbrunel merged commit f90ee8c into chartjs:master Aug 1, 2017
@simonbrunel simonbrunel deleted the feat/render-event branch August 1, 2017 12:28
@hach-que
Copy link

hach-que commented Aug 2, 2017

FYI This broke our isomorphic app on line 274 of platform.dom.js. The issue is that document doesn't exist for document.createElement('style') under Node.js and somehow that is now being called when the module is being loaded.

@simonbrunel
Copy link
Member Author

Thanks @hach-que for reporting this issue. Not sure what the best way to fix it, ideally, the DOM platform should not be instantiated on Node.js. Are you able to share your app or a minimal use case?

@hach-que
Copy link

hach-que commented Aug 2, 2017

We can't share the application source code at this time. We're using react-chartjs2 and we're rendering on both the client and the server. This way when the user requests a page, they get the HTML immediately as the Javascript loads in (once the JS is downloaded it then takes over navigation so that everything is client side).

The best way to check for this stuff is to see if window is set with if (window) {. If it's not present in a global scope, you're not running the browser and should avoid using window or document on startup.

@simonbrunel
Copy link
Member Author

I'm not familiar with isomorphic apps and I don't understand why Chart.js is evaluated server side. Of course checking window will workaround that issue, but I'm wondering if it would not be better to avoid loading platform.dom.js in the case we are not in a compatible environment (ie. with window and document defined).

@hach-que
Copy link

hach-que commented Aug 2, 2017

Well react-chartjs-2 just renders out the <canvas> tag on the server, but the issue is that the initialize method in module.exports of chart.js is now trying to access window. So I don't think it's that the code is trying to do any runtime stuff with Chart.js on the server, but rather that the initialize method called in src/chart.js is now trying to access document, so the module can't even be loaded server side now (even if we never actually use any of the functionality that hits the DOM on the server).

It's probably reasonable to just not load platform.dom.js though, especially if all DOM related access is encapsulated in that file. Maybe a platform.null.js implementation that does nothing on every call if window / document is not present?

@hach-que
Copy link

hach-que commented Aug 2, 2017

(The reason we want the <canvas> element rendered on the server is that you only get the benefits of server side rendering if the HTML emitted by the server exactly matches the HTML that would be rendered by React on first load, otherwise React has to recalculate and rewrite the DOM to match)

This was referenced Aug 28, 2017
yofreke pushed a commit to yofreke/Chart.js that referenced this pull request Dec 30, 2017
Allow to create a chart on a canvas not yet attached to the DOM (detection based on CSS animations described in https://davidwalsh.name/detect-node-insertion). The resize element (IFRAME) is added only when the canvas receives a parent or when `style.display` changes from `none`. This change also allows to re-parent the canvas under a different node (the resizer element following). This is a preliminary work for the DIV based resizer.
@maria-grigorieva
Copy link

maria-grigorieva commented Apr 19, 2019

hi, I have problems with dynamically created canvases in 2.8.0 version.
I'm trying to insert charts in DataTable, in "td" element. So, for each "td" I use appendChild() with add_shartJS() procedure. Chart data and options were taken from the documentation. The only difference is adding { responsive: false }.

add_chartJS(title) {
        var canvas = document.createElement("canvas");
        var id = 'chartjs_' + title;
        canvas.id = id;
        var ctx = canvas.getContext("2d");

        var myChart = new Chart(ctx, {
            type: 'bar',
            data: {
                labels: ['Red', 'Blue', 'Yellow', 'Green', 'Purple', 'Orange'],
                datasets: [{
                    label: '# of Votes',
                    data: [12, 19, 3, 5, 2, 3],
                    backgroundColor: [
                        'rgba(255, 99, 132, 0.2)',
                        'rgba(54, 162, 235, 0.2)',
                        'rgba(255, 206, 86, 0.2)',
                        'rgba(75, 192, 192, 0.2)',
                        'rgba(153, 102, 255, 0.2)',
                        'rgba(255, 159, 64, 0.2)'
                    ],
                    borderColor: [
                        'rgba(255, 99, 132, 1)',
                        'rgba(54, 162, 235, 1)',
                        'rgba(255, 206, 86, 1)',
                        'rgba(75, 192, 192, 1)',
                        'rgba(153, 102, 255, 1)',
                        'rgba(255, 159, 64, 1)'
                    ],
                    borderWidth: 1
                }]
            },
            options: {
                scales: {
                    yAxes: [{
                        ticks: {
                            beginAtZero: true
                        }
                    }]
                },
                responsive: false
            }
        });
        return canvas;
    }

As a result, I have empty canvases.
Also, tried to use this development build, just to be sure, but all the same.

exwm pushed a commit to exwm/Chart.js that referenced this pull request Apr 30, 2021
Allow to create a chart on a canvas not yet attached to the DOM (detection based on CSS animations described in https://davidwalsh.name/detect-node-insertion). The resize element (IFRAME) is added only when the canvas receives a parent or when `style.display` changes from `none`. This change also allows to re-parent the canvas under a different node (the resizer element following). This is a preliminary work for the DIV based resizer.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants