Skip to content

Commit

Permalink
Prevent infinite resize when vertical scrollbar appears (#6011)
Browse files Browse the repository at this point in the history
If the container size shrank during chart resize, let's assume scrollbar appeared. So we resize again with the scrollbar visible effectively making chart smaller and the scrollbar hidden again. Because we are inside `throttled`, and currently `ticking`, scroll events are ignored during this whole 2 resize process. If we assumed wrong and something else happened, we are resizing twice in a frame (potential performance issue)
  • Loading branch information
kurkle authored and simonbrunel committed Jan 30, 2019
1 parent 29f7fa2 commit 97da221
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 4 deletions.
6 changes: 3 additions & 3 deletions src/core/core.controller.js
Expand Up @@ -277,13 +277,13 @@ helpers.extend(Chart.prototype, /** @lends Chart */ {
plugins.notify(me, 'resize', [newSize]);

// Notify of resize
if (me.options.onResize) {
me.options.onResize(me, newSize);
if (options.onResize) {
options.onResize(me, newSize);
}

me.stop();
me.update({
duration: me.options.responsiveAnimationDuration
duration: options.responsiveAnimationDuration
});
}
},
Expand Down
14 changes: 13 additions & 1 deletion src/platforms/platform.dom.js
Expand Up @@ -273,7 +273,19 @@ function addResizeListener(node, listener, chart) {
// Let's keep track of this added resizer and thus avoid DOM query when removing it.
var resizer = expando.resizer = createResizer(throttled(function() {
if (expando.resizer) {
return listener(createEvent('resize', chart));
var container = chart.options.maintainAspectRatio && node.parentNode;
var w = container ? container.clientWidth : 0;
listener(createEvent('resize', chart));
if (container && container.clientWidth < w && chart.canvas) {
// If the container size shrank during chart resize, let's assume
// scrollbar appeared. So we resize again with the scrollbar visible -
// effectively making chart smaller and the scrollbar hidden again.
// Because we are inside `throttled`, and currently `ticking`, scroll
// events are ignored during this whole 2 resize process.
// If we assumed wrong and something else happened, we are resizing
// twice in a frame (potential performance issue)
listener(createEvent('resize', chart));
}
}
}));

Expand Down

0 comments on commit 97da221

Please sign in to comment.