Skip to content

Commit

Permalink
Fix responsive issue when the chart is recreated
Browse files Browse the repository at this point in the history
Chrome specific issue that happens when destroying a chart and re-creating it immediately (same animation frame?). The CSS animation used to detect when the canvas become visible is not re-evaluated, breaking responsiveness. So if the node is already rendered, let's call the handler immediately and skip the first `animationstart` event.
  • Loading branch information
simonbrunel committed Sep 18, 2017
1 parent ea703a5 commit 110050d
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 2 deletions.
19 changes: 19 additions & 0 deletions src/platforms/platform.dom.js
Expand Up @@ -227,16 +227,35 @@ function createResizer(handler) {
function watchForRender(node, handler) {
var expando = node[EXPANDO_KEY] || (node[EXPANDO_KEY] = {});
var proxy = expando.renderProxy = function(e) {
if (expando.rendered) {
// Skip this "first" notification since the node was already rendered when calling
// watchForRender, the handler having been manually called already (see below).
expando.rendered = false;
return;
}

if (e.animationName === CSS_RENDER_ANIMATION) {
handler();
}
};

// #4737: Chrome might skip the CSS animation when the CSS_RENDER_MONITOR class is removed
// then added back immediately (same animation frame?). If the node is already rendered,
// let's call the handler immediately (see below) and skip the first animationstart.
// Note(SB): accessing `offsetParent` will also force a reflow and reset the animation.
// https://gist.github.com/paulirish/5d52fb081b3570c81e3a#box-metrics
// https://github.com/chartjs/Chart.js/issues/4737
var rendered = expando.rendered = !!node.offsetParent;

helpers.each(ANIMATION_START_EVENTS, function(type) {
addEventListener(node, type, proxy);
});

node.classList.add(CSS_RENDER_MONITOR);

if (rendered) {
handler();
}
}

function unwatchForRender(node) {
Expand Down
8 changes: 7 additions & 1 deletion test/jasmine.utils.js
Expand Up @@ -75,7 +75,13 @@ function acquireChart(config, options) {
wrapper.appendChild(canvas);
window.document.body.appendChild(wrapper);

chart = new Chart(canvas.getContext('2d'), config);
try {
chart = new Chart(canvas.getContext('2d'), config);
} catch (e) {
window.document.body.removeChild(wrapper);
throw e;
}

chart.$test = {
persistent: options.persistent,
wrapper: wrapper
Expand Down
41 changes: 40 additions & 1 deletion test/specs/core.controller.tests.js
Expand Up @@ -495,6 +495,45 @@ describe('Chart', function() {
});
});
});

// https://github.com/chartjs/Chart.js/issues/4737
it('should resize the canvas when re-creating the chart', function(done) {
var chart = acquireChart({
options: {
responsive: true
}
}, {
wrapper: {
style: 'width: 320px'
}
});

waitForResize(chart, function() {
var canvas = chart.canvas;
expect(chart).toBeChartOfSize({
dw: 320, dh: 320,
rw: 320, rh: 320,
});

chart.destroy();
chart = new Chart(canvas, {
type: 'line',
options: {
responsive: true
}
});

canvas.parentNode.style.width = '455px';
waitForResize(chart, function() {
expect(chart).toBeChartOfSize({
dw: 455, dh: 455,
rw: 455, rh: 455,
});

done();
});
});
});
});

describe('config.options.responsive: true (maintainAspectRatio: true)', function() {
Expand Down Expand Up @@ -627,7 +666,7 @@ describe('Chart', function() {
});
});

describe('config.options.devicePixelRatio 3', function() {
describe('config.options.devicePixelRatio', function() {
beforeEach(function() {
this.devicePixelRatio = window.devicePixelRatio;
window.devicePixelRatio = 1;
Expand Down

0 comments on commit 110050d

Please sign in to comment.