From 7a46f04ddc805dc2676004b924a6966eff2c8496 Mon Sep 17 00:00:00 2001 From: kurkle Date: Mon, 18 May 2020 17:47:39 +0300 Subject: [PATCH 01/10] Delay animations until attached --- src/core/core.controller.js | 3 ++- src/helpers/helpers.dom.js | 14 ++++++-------- src/platform/platform.dom.js | 15 +++++++++++---- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/core/core.controller.js b/src/core/core.controller.js index bc60d506fc6..e0a02a9ee33 100644 --- a/src/core/core.controller.js +++ b/src/core/core.controller.js @@ -223,6 +223,7 @@ export default class Chart { this.$plugins = undefined; this.$proxies = {}; this._hiddenIndices = {}; + this.attached = false; // Add the chart instance to the global namespace Chart.instances[me.id] = me; @@ -663,7 +664,7 @@ export default class Chart { }; if (Animator.has(me)) { - if (!Animator.running(me)) { + if (me.attached && !Animator.running(me)) { Animator.start(me); } } else { diff --git a/src/helpers/helpers.dom.js b/src/helpers/helpers.dom.js index 74d8f957641..443dcaf50da 100644 --- a/src/helpers/helpers.dom.js +++ b/src/helpers/helpers.dom.js @@ -126,13 +126,14 @@ export function getRelativePosition(evt, chart) { }; } +function fallbackIfNotValid(measure, fallback) { + return typeof measure === 'number' && measure > 0 ? measure : fallback; +} + export function getMaximumWidth(domNode) { const container = _getParentNode(domNode); if (!container) { - if (typeof domNode.clientWidth === 'number') { - return domNode.clientWidth; - } - return domNode.width; + return fallbackIfNotValid(domNode.clientWidth, domNode.width); } const clientWidth = container.clientWidth; @@ -147,10 +148,7 @@ export function getMaximumWidth(domNode) { export function getMaximumHeight(domNode) { const container = _getParentNode(domNode); if (!container) { - if (typeof domNode.clientHeight === 'number') { - return domNode.clientHeight; - } - return domNode.height; + return fallbackIfNotValid(domNode.clientHeight, domNode.height); } const clientHeight = container.clientHeight; diff --git a/src/platform/platform.dom.js b/src/platform/platform.dom.js index ee5aafa8083..01dcb7f3287 100644 --- a/src/platform/platform.dom.js +++ b/src/platform/platform.dom.js @@ -272,23 +272,29 @@ function unlistenForResize(proxies) { } /** - * @param {HTMLCanvasElement} canvas + * @param {Chart} chart * @param {{ resize?: any; detach?: MutationObserver; attach?: MutationObserver; }} proxies * @param {function} listener */ -function listenForResize(canvas, proxies, listener) { +function listenForResize(chart, proxies, listener) { + const canvas = chart.canvas; // Helper for recursing when canvas is detached from it's parent - const detached = () => listenForResize(canvas, proxies, listener); + const detached = () => { + chart.attached = false; + listenForResize(chart, proxies, listener); + }; // First make sure all observers are removed unlistenForResize(proxies); // Then check if we are attached const container = _getParentNode(canvas); if (container) { + chart.attached = true; // The canvas is attached (or was immediately re-attached when called through `detached`) proxies.resize = watchForResize(container, listener); proxies.detach = watchForDetachment(canvas, detached); } else { + chart.attached = false; // The canvas is detached proxies.attach = watchForAttachment(canvas, () => { // The canvas was attached. @@ -296,6 +302,7 @@ function listenForResize(canvas, proxies, listener) { const parent = _getParentNode(canvas); proxies.resize = watchForResize(parent, listener); proxies.detach = watchForDetachment(canvas, detached); + chart.attached = true; }); } } @@ -382,7 +389,7 @@ export default class DomPlatform extends BasePlatform { const canvas = chart.canvas; const proxies = chart.$proxies || (chart.$proxies = {}); if (type === 'resize') { - return listenForResize(canvas, proxies, listener); + return listenForResize(chart, proxies, listener); } const proxy = proxies[type] = throttled((event) => { From 577bd0a8c564ee0c99b599bfe968ab0e596daf22 Mon Sep 17 00:00:00 2001 From: Jukka Kurkela Date: Mon, 18 May 2020 22:40:39 +0300 Subject: [PATCH 02/10] Detect container detachment --- src/platform/platform.dom.js | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/src/platform/platform.dom.js b/src/platform/platform.dom.js index 01dcb7f3287..554e4aa704b 100644 --- a/src/platform/platform.dom.js +++ b/src/platform/platform.dom.js @@ -278,32 +278,36 @@ function unlistenForResize(proxies) { */ function listenForResize(chart, proxies, listener) { const canvas = chart.canvas; - // Helper for recursing when canvas is detached from it's parent + + // First make sure all observers are removed + unlistenForResize(proxies); + + const container = _getParentNode(canvas); + const detached = () => { chart.attached = false; listenForResize(chart, proxies, listener); }; - // First make sure all observers are removed - unlistenForResize(proxies); - // Then check if we are attached - const container = _getParentNode(canvas); - if (container) { + const attached = () => { chart.attached = true; - // The canvas is attached (or was immediately re-attached when called through `detached`) + listener(); proxies.resize = watchForResize(container, listener); proxies.detach = watchForDetachment(canvas, detached); + }; + + + if (container) { + const upper = _getParentNode(container); + if (upper) { + attached(); + } else { + chart.attached = false; + proxies.attach = watchForAttachment(container, attached); + } } else { chart.attached = false; - // The canvas is detached - proxies.attach = watchForAttachment(canvas, () => { - // The canvas was attached. - removeObserver(proxies, 'attach'); - const parent = _getParentNode(canvas); - proxies.resize = watchForResize(parent, listener); - proxies.detach = watchForDetachment(canvas, detached); - chart.attached = true; - }); + proxies.attach = watchForAttachment(canvas, attached); } } From e73bab16f20d5cdd76518926f38f640fea53967c Mon Sep 17 00:00:00 2001 From: Jukka Kurkela Date: Mon, 18 May 2020 23:13:21 +0300 Subject: [PATCH 03/10] Initial update after attachment --- src/core/core.controller.js | 15 ++++++++++++--- src/platform/platform.dom.js | 8 ++------ 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/core/core.controller.js b/src/core/core.controller.js index e0a02a9ee33..b9f39ee0b1a 100644 --- a/src/core/core.controller.js +++ b/src/core/core.controller.js @@ -223,7 +223,7 @@ export default class Chart { this.$plugins = undefined; this.$proxies = {}; this._hiddenIndices = {}; - this.attached = false; + this.attached = true; // Add the chart instance to the global namespace Chart.instances[me.id] = me; @@ -251,7 +251,9 @@ export default class Chart { Animator.listen(me, 'progress', onAnimationProgress); me._initialize(); - me.update(); + if (me.attached) { + me.update(); + } } /** @@ -344,7 +346,12 @@ export default class Chart { options.onResize(me, newSize); } - me.update('resize'); + if (me.attached) { + me.update('resize'); + } else { + me.attached = true; + me.update(); + } } } @@ -956,6 +963,8 @@ export default class Chart { me.platform.addEventListener(me, 'resize', listener); listeners.resize = listener; + } else { + me.attached = true; } } diff --git a/src/platform/platform.dom.js b/src/platform/platform.dom.js index 554e4aa704b..970c4bc72f2 100644 --- a/src/platform/platform.dom.js +++ b/src/platform/platform.dom.js @@ -278,6 +278,7 @@ function unlistenForResize(proxies) { */ function listenForResize(chart, proxies, listener) { const canvas = chart.canvas; + chart.attached = false; // First make sure all observers are removed unlistenForResize(proxies); @@ -290,23 +291,18 @@ function listenForResize(chart, proxies, listener) { }; const attached = () => { - chart.attached = true; - listener(); proxies.resize = watchForResize(container, listener); proxies.detach = watchForDetachment(canvas, detached); }; if (container) { - const upper = _getParentNode(container); - if (upper) { + if (_getParentNode(container)) { attached(); } else { - chart.attached = false; proxies.attach = watchForAttachment(container, attached); } } else { - chart.attached = false; proxies.attach = watchForAttachment(canvas, attached); } } From eba1efaeeaa37a83c066d78208b300966a121bcc Mon Sep 17 00:00:00 2001 From: Jukka Kurkela Date: Mon, 18 May 2020 23:31:46 +0300 Subject: [PATCH 04/10] Allow 0 size again --- src/helpers/helpers.dom.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/helpers/helpers.dom.js b/src/helpers/helpers.dom.js index 443dcaf50da..a0c29a7c493 100644 --- a/src/helpers/helpers.dom.js +++ b/src/helpers/helpers.dom.js @@ -127,7 +127,7 @@ export function getRelativePosition(evt, chart) { } function fallbackIfNotValid(measure, fallback) { - return typeof measure === 'number' && measure > 0 ? measure : fallback; + return typeof measure === 'number' ? measure : fallback; } export function getMaximumWidth(domNode) { From 2416c209993ce205bbfa822cbae77c6c20b93768 Mon Sep 17 00:00:00 2001 From: Jukka Kurkela Date: Tue, 19 May 2020 00:10:57 +0300 Subject: [PATCH 05/10] Fix tests, stop exposing new internals --- src/core/core.controller.js | 2 +- src/platform/platform.dom.js | 25 ++++++++++--------------- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/src/core/core.controller.js b/src/core/core.controller.js index b9f39ee0b1a..1502a8a2e47 100644 --- a/src/core/core.controller.js +++ b/src/core/core.controller.js @@ -961,7 +961,7 @@ export default class Chart { } }; - me.platform.addEventListener(me, 'resize', listener); + me.attached = me.platform.addEventListener(me, 'resize', listener); listeners.resize = listener; } else { me.attached = true; diff --git a/src/platform/platform.dom.js b/src/platform/platform.dom.js index 970c4bc72f2..49139f2f55d 100644 --- a/src/platform/platform.dom.js +++ b/src/platform/platform.dom.js @@ -278,33 +278,28 @@ function unlistenForResize(proxies) { */ function listenForResize(chart, proxies, listener) { const canvas = chart.canvas; - chart.attached = false; - - // First make sure all observers are removed - unlistenForResize(proxies); - const container = _getParentNode(canvas); const detached = () => { - chart.attached = false; listenForResize(chart, proxies, listener); }; const attached = () => { - proxies.resize = watchForResize(container, listener); + const parent = container || _getParentNode(canvas); + proxies.resize = watchForResize(parent, listener); proxies.detach = watchForDetachment(canvas, detached); }; + unlistenForResize(proxies); - if (container) { - if (_getParentNode(container)) { - attached(); - } else { - proxies.attach = watchForAttachment(container, attached); - } - } else { - proxies.attach = watchForAttachment(canvas, attached); + if (container && _getParentNode(container)) { + attached(); + return true; } + + proxies.attach = watchForAttachment(container || canvas, attached); + + return false; } /** From 345dafd3a9dd0f8dae697764fa1efc2af262c7e7 Mon Sep 17 00:00:00 2001 From: Jukka Kurkela Date: Tue, 19 May 2020 00:21:51 +0300 Subject: [PATCH 06/10] Revert unnecessary changes --- src/platform/platform.dom.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/platform/platform.dom.js b/src/platform/platform.dom.js index 49139f2f55d..e132cc7273a 100644 --- a/src/platform/platform.dom.js +++ b/src/platform/platform.dom.js @@ -272,16 +272,15 @@ function unlistenForResize(proxies) { } /** - * @param {Chart} chart + * @param {HTMLCanvasElement} canvas * @param {{ resize?: any; detach?: MutationObserver; attach?: MutationObserver; }} proxies * @param {function} listener */ -function listenForResize(chart, proxies, listener) { - const canvas = chart.canvas; +function listenForResize(canvas, proxies, listener) { const container = _getParentNode(canvas); const detached = () => { - listenForResize(chart, proxies, listener); + listenForResize(canvas, proxies, listener); }; const attached = () => { @@ -384,7 +383,7 @@ export default class DomPlatform extends BasePlatform { const canvas = chart.canvas; const proxies = chart.$proxies || (chart.$proxies = {}); if (type === 'resize') { - return listenForResize(chart, proxies, listener); + return listenForResize(canvas, proxies, listener); } const proxy = proxies[type] = throttled((event) => { From 3930a3f4ceec6ecb2e113012cb6725e56eb2aac6 Mon Sep 17 00:00:00 2001 From: Jukka Kurkela Date: Tue, 19 May 2020 22:55:21 +0300 Subject: [PATCH 07/10] Document --- docs/docs/getting-started/v3-migration.md | 4 +++- src/platform/platform.dom.js | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/docs/getting-started/v3-migration.md b/docs/docs/getting-started/v3-migration.md index ebba06adde0..8c5d3b4941a 100644 --- a/docs/docs/getting-started/v3-migration.md +++ b/docs/docs/getting-started/v3-migration.md @@ -141,7 +141,6 @@ options: { Animation system was completely rewritten in Chart.js v3. Each property can now be animated separately. Please see [animations](../configuration/animations.md) docs for details. - #### Customizability * `custom` attribute of elements was removed. Please use scriptable options @@ -169,6 +168,7 @@ Animation system was completely rewritten in Chart.js v3. Each property can now While the end-user migration for Chart.js 3 is fairly straight-forward, the developer migration can be more complicated. Please reach out for help in the #dev [Slack](https://chartjs-slack.herokuapp.com/) channel if tips on migrating would be helpful. Some of the biggest things that have changed: + * There is a completely rewritten and more performant animation system. * `Element._model` and `Element._view` are no longer used and properties are now set directly on the elements. You will have to use the method `getProps` to access these properties inside most methods such as `inXRange`/`inYRange` and `getCenterPoint`. Please take a look at [the Chart.js-provided elements](https://github.com/chartjs/Chart.js/tree/master/src/elements) for examples. * When building the elements in a controller, it's now suggested to call `updateElement` to provide the element properties. There are also methods such as `getSharedOptions` and `includeOptions` that have been added to skip redundant computation. Please take a look at [the Chart.js-provided controllers](https://github.com/chartjs/Chart.js/tree/master/src/controllers) for examples. @@ -187,6 +187,7 @@ A few changes were made to controllers that are more straight-forward, but will The following properties and methods were removed: #### Chart + * `Chart.borderWidth` * `Chart.chart.chart` * `Chart.Bar`. New charts are created via `new Chart` and providing the appropriate `type` parameter @@ -411,3 +412,4 @@ The APIs listed in this section have changed in signature or behaviour from vers * `Chart.platform` is no longer the platform object used by charts. Every chart instance now has a separate platform instance. * `Chart.platforms` is an object that contains two usable platform classes, `BasicPlatform` and `DomPlatform`. It also contains `BasePlatform`, a class that all platforms must extend from. * If the canvas passed in is an instance of `OffscreenCanvas`, the `BasicPlatform` is automatically used. +* `DomPlatform.addEventListener` for `'resize'` `type` now returns `boolean` indicating whether the element is attached to DOM or not. diff --git a/src/platform/platform.dom.js b/src/platform/platform.dom.js index e132cc7273a..6a19066875d 100644 --- a/src/platform/platform.dom.js +++ b/src/platform/platform.dom.js @@ -275,6 +275,7 @@ function unlistenForResize(proxies) { * @param {HTMLCanvasElement} canvas * @param {{ resize?: any; detach?: MutationObserver; attach?: MutationObserver; }} proxies * @param {function} listener + * @return {boolean} `true` if canvas and its parent is attached to DOM */ function listenForResize(canvas, proxies, listener) { const container = _getParentNode(canvas); From 8619e982355d6944e5b92356c470c395679eb326 Mon Sep 17 00:00:00 2001 From: Jukka Kurkela Date: Wed, 20 May 2020 11:41:40 +0300 Subject: [PATCH 08/10] Refactor --- docs/docs/getting-started/v3-migration.md | 2 +- src/core/core.controller.js | 51 +++++++-- src/platform/platform.base.js | 8 ++ src/platform/platform.dom.js | 120 ++++++++++------------ test/specs/platform.dom.tests.js | 22 ++++ 5 files changed, 128 insertions(+), 75 deletions(-) diff --git a/docs/docs/getting-started/v3-migration.md b/docs/docs/getting-started/v3-migration.md index 8c5d3b4941a..14f2e2d21eb 100644 --- a/docs/docs/getting-started/v3-migration.md +++ b/docs/docs/getting-started/v3-migration.md @@ -412,4 +412,4 @@ The APIs listed in this section have changed in signature or behaviour from vers * `Chart.platform` is no longer the platform object used by charts. Every chart instance now has a separate platform instance. * `Chart.platforms` is an object that contains two usable platform classes, `BasicPlatform` and `DomPlatform`. It also contains `BasePlatform`, a class that all platforms must extend from. * If the canvas passed in is an instance of `OffscreenCanvas`, the `BasicPlatform` is automatically used. -* `DomPlatform.addEventListener` for `'resize'` `type` now returns `boolean` indicating whether the element is attached to DOM or not. +* `isAttached` method was added to platform. diff --git a/src/core/core.controller.js b/src/core/core.controller.js index 1502a8a2e47..8feb3d3e1ae 100644 --- a/src/core/core.controller.js +++ b/src/core/core.controller.js @@ -214,7 +214,7 @@ export default class Chart { this.active = undefined; this.lastActive = []; this._lastEvent = undefined; - /** @type {{resize?: function}} */ + /** @type {{attach?: function, detach?: function, resize?: function}} */ this._listeners = {}; this._sortedMetasets = []; this._updating = false; @@ -346,12 +346,8 @@ export default class Chart { options.onResize(me, newSize); } - if (me.attached) { - me.update('resize'); - } else { - me.attached = true; - me.update(); - } + // Only apply 'resize' mode if we are attached, else do a regular update. + me.update(me.attached && 'resize'); } } @@ -945,24 +941,57 @@ export default class Chart { bindEvents() { const me = this; const listeners = me._listeners; + const platform = me.platform; + let listener = function(e) { me._eventHandler(e); }; helpers.each(me.options.events, (type) => { - me.platform.addEventListener(me, type, listener); + platform.addEventListener(me, type, listener); listeners[type] = listener; }); if (me.options.responsive) { - listener = function(width, height) { + listener = (width, height) => { if (me.canvas) { me.resize(false, width, height); } }; - me.attached = me.platform.addEventListener(me, 'resize', listener); - listeners.resize = listener; + let detached; // eslint-disable-line prefer-const + const attached = () => { + if (listeners.attach) { + platform.removeEventListener(me, 'attach', attached); + delete listeners.attach; + } + + me.resize(); + me.attached = true; + + platform.addEventListener(me, 'resize', listener); + listeners.resize = listener; + + platform.addEventListener(me, 'detach', detached); + listeners.detach = detached; + }; + + detached = () => { + me.attached = false; + + if (listeners.resize) { + platform.removeEventListener(me, 'resize', listener); + delete listeners.resize; + } + + platform.addEventListener(me, 'attach', attached); + listeners.attach = attached; + }; + if (platform.isAttached(me.canvas)) { + attached(); + } else { + detached(); + } } else { me.attached = true; } diff --git a/src/platform/platform.base.js b/src/platform/platform.base.js index 801eecb4e20..434ede1c780 100644 --- a/src/platform/platform.base.js +++ b/src/platform/platform.base.js @@ -48,6 +48,14 @@ export default class BasePlatform { getDevicePixelRatio() { return 1; } + + /** + * @param {HTMLCanvasElement} canvas + * @returns {boolean} true if the canvas is attached to the platform, false if not. + */ + isAttached(canvas) { // eslint-disable-line no-unused-vars + return true; + } } /** diff --git a/src/platform/platform.dom.js b/src/platform/platform.dom.js index 6a19066875d..5b59f7837d1 100644 --- a/src/platform/platform.dom.js +++ b/src/platform/platform.dom.js @@ -250,56 +250,47 @@ function watchForDetachment(element, fn) { return observer; } -/** - * @param {{ [x: string]: any; resize?: any; detach?: MutationObserver; attach?: MutationObserver; }} proxies - * @param {string} type - */ -function removeObserver(proxies, type) { - const observer = proxies[type]; - if (observer) { - observer.disconnect(); - proxies[type] = undefined; - } +function createAttachObserver(chart, type, listener) { + const canvas = chart.canvas; + const container = canvas && _getParentNode(canvas); + return watchForAttachment(container || canvas, listener); } -/** - * @param {{ resize?: any; detach?: MutationObserver; attach?: MutationObserver; }} proxies - */ -function unlistenForResize(proxies) { - removeObserver(proxies, 'attach'); - removeObserver(proxies, 'detach'); - removeObserver(proxies, 'resize'); +function createDetachObserver(chart, type, listener) { + const canvas = chart.canvas; + if (canvas) { + return watchForDetachment(canvas, listener); + } } -/** - * @param {HTMLCanvasElement} canvas - * @param {{ resize?: any; detach?: MutationObserver; attach?: MutationObserver; }} proxies - * @param {function} listener - * @return {boolean} `true` if canvas and its parent is attached to DOM - */ -function listenForResize(canvas, proxies, listener) { - const container = _getParentNode(canvas); - - const detached = () => { - listenForResize(canvas, proxies, listener); - }; - - const attached = () => { - const parent = container || _getParentNode(canvas); - proxies.resize = watchForResize(parent, listener); - proxies.detach = watchForDetachment(canvas, detached); - }; - - unlistenForResize(proxies); +function createResizeObserver(chart, type, listener) { + const canvas = chart.canvas; + const container = canvas && _getParentNode(canvas); + if (container) { + return watchForResize(container, listener); + } +} - if (container && _getParentNode(container)) { - attached(); - return true; +function releaseObserver(canvas, type, observer) { + if (observer) { + observer.disconnect(); } +} + +function createProxyForNative(chart, type, listener) { + const canvas = chart.canvas; + const proxy = throttled((event) => { + // This case can occur if the chart is destroyed while waiting + // for the throttled function to occur. We prevent crashes by checking + // for a destroyed chart + if (chart.ctx !== null) { + listener(fromNativeEvent(event, chart)); + } + }, chart); - proxies.attach = watchForAttachment(container || canvas, attached); + addListener(canvas, type, proxy); - return false; + return proxy; } /** @@ -381,22 +372,14 @@ export default class DomPlatform extends BasePlatform { // Can have only one listener per type, so make sure previous is removed this.removeEventListener(chart, type); - const canvas = chart.canvas; const proxies = chart.$proxies || (chart.$proxies = {}); - if (type === 'resize') { - return listenForResize(canvas, proxies, listener); - } - - const proxy = proxies[type] = throttled((event) => { - // This case can occur if the chart is destroyed while waiting - // for the throttled function to occur. We prevent crashes by checking - // for a destroyed chart - if (chart.ctx !== null) { - listener(fromNativeEvent(event, chart)); - } - }, chart); - - addListener(canvas, type, proxy); + const handlers = { + attach: createAttachObserver, + detach: createDetachObserver, + resize: createResizeObserver + }; + const handler = handlers[type] || createProxyForNative; + proxies[type] = handler(chart, type, listener); } @@ -407,21 +390,32 @@ export default class DomPlatform extends BasePlatform { removeEventListener(chart, type) { const canvas = chart.canvas; const proxies = chart.$proxies || (chart.$proxies = {}); - - if (type === 'resize') { - return unlistenForResize(proxies); - } - const proxy = proxies[type]; + if (!proxy) { return; } - removeListener(canvas, type, proxy); + const handlers = { + attach: releaseObserver, + detach: releaseObserver, + resize: releaseObserver + }; + const handler = handlers[type] || removeListener; + handler(canvas, type, proxy); proxies[type] = undefined; } getDevicePixelRatio() { return window.devicePixelRatio; } + + + /** + * @param {HTMLCanvasElement} canvas + */ + isAttached(canvas) { + const container = _getParentNode(canvas); + return !!(container && _getParentNode(container)); + } } diff --git a/test/specs/platform.dom.tests.js b/test/specs/platform.dom.tests.js index 78846cfb976..263b031682b 100644 --- a/test/specs/platform.dom.tests.js +++ b/test/specs/platform.dom.tests.js @@ -1,3 +1,5 @@ +import {DomPlatform} from '../../src/platform/platforms'; + describe('Platform.dom', function() { describe('context acquisition', function() { @@ -405,4 +407,24 @@ describe('Platform.dom', function() { }); }); }); + + describe('isAttached', function() { + it('should detect detached when canvas is attached to DOM', function() { + var platform = new DomPlatform(); + var canvas = document.createElement('canvas'); + var div = document.createElement('div'); + + expect(platform.isAttached(canvas)).toEqual(false); + div.appendChild(canvas); + expect(platform.isAttached(canvas)).toEqual(false); + document.body.appendChild(div); + + expect(platform.isAttached(canvas)).toEqual(true); + + div.removeChild(canvas); + expect(platform.isAttached(canvas)).toEqual(false); + document.body.removeChild(div); + expect(platform.isAttached(canvas)).toEqual(false); + }); + }); }); From 4a59028aed62e9201b4d4bed35b53c93a6122d12 Mon Sep 17 00:00:00 2001 From: Jukka Kurkela Date: Wed, 20 May 2020 12:17:32 +0300 Subject: [PATCH 09/10] Refactor some more --- src/core/core.controller.js | 38 ++++++------- src/platform/platform.dom.js | 106 +++++++++++++---------------------- 2 files changed, 56 insertions(+), 88 deletions(-) diff --git a/src/core/core.controller.js b/src/core/core.controller.js index 8feb3d3e1ae..f6a044f536e 100644 --- a/src/core/core.controller.js +++ b/src/core/core.controller.js @@ -943,14 +943,22 @@ export default class Chart { const listeners = me._listeners; const platform = me.platform; + const _add = (type, listener) => { + platform.addEventListener(me, type, listener); + listeners[type] = listener; + }; + const _remove = (type, listener) => { + if (listeners[type]) { + platform.removeEventListener(me, type, listener); + delete listeners[type]; + } + }; + let listener = function(e) { me._eventHandler(e); }; - helpers.each(me.options.events, (type) => { - platform.addEventListener(me, type, listener); - listeners[type] = listener; - }); + helpers.each(me.options.events, (type) => _add(type, listener)); if (me.options.responsive) { listener = (width, height) => { @@ -961,32 +969,22 @@ export default class Chart { let detached; // eslint-disable-line prefer-const const attached = () => { - if (listeners.attach) { - platform.removeEventListener(me, 'attach', attached); - delete listeners.attach; - } + _remove('attach', attached); me.resize(); me.attached = true; - platform.addEventListener(me, 'resize', listener); - listeners.resize = listener; - - platform.addEventListener(me, 'detach', detached); - listeners.detach = detached; + _add('resize', listener); + _add('detach', detached); }; detached = () => { me.attached = false; - if (listeners.resize) { - platform.removeEventListener(me, 'resize', listener); - delete listeners.resize; - } - - platform.addEventListener(me, 'attach', attached); - listeners.attach = attached; + _remove('resize', listener); + _add('attach', attached); }; + if (platform.isAttached(me.canvas)) { attached(); } else { diff --git a/src/platform/platform.dom.js b/src/platform/platform.dom.js index 5b59f7837d1..07f807431af 100644 --- a/src/platform/platform.dom.js +++ b/src/platform/platform.dom.js @@ -172,51 +172,17 @@ function throttled(fn, thisArg) { }; } -/** - * Watch for resize of `element`. - * Calling `fn` is limited to once per animation frame - * @param {Element} element - The element to monitor - * @param {function} fn - Callback function to call when resized - */ -function watchForResize(element, fn) { - const resize = throttled((width, height) => { - const w = element.clientWidth; - fn(width, height); - if (w < element.clientWidth) { - // 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) - fn(); - } - }, window); - - // @ts-ignore until https://github.com/Microsoft/TypeScript/issues/28502 implemented - const observer = new ResizeObserver(entries => { - const entry = entries[0]; - resize(entry.contentRect.width, entry.contentRect.height); - }); - observer.observe(element); - return observer; -} - -/** - * Detect attachment of `element` or its direct `parent` to DOM - * @param {Element} element - The element to watch for - * @param {function} fn - Callback function to call when attachment is detected - * @return {MutationObserver} - */ -function watchForAttachment(element, fn) { +function createAttachObserver(chart, type, listener) { + const canvas = chart.canvas; + const container = canvas && _getParentNode(canvas); + const element = container || canvas; const observer = new MutationObserver(entries => { const parent = _getParentNode(element); entries.forEach(entry => { for (let i = 0; i < entry.addedNodes.length; i++) { const added = entry.addedNodes[i]; if (added === element || added === parent) { - fn(entry.target); + listener(entry.target); } } }); @@ -225,50 +191,54 @@ function watchForAttachment(element, fn) { return observer; } -/** - * Watch for detachment of `element` from its direct `parent`. - * @param {Element} element - The element to watch - * @param {function} fn - Callback function to call when detached. - * @return {MutationObserver=} - */ -function watchForDetachment(element, fn) { - const parent = _getParentNode(element); - if (!parent) { +function createDetachObserver(chart, type, listener) { + const canvas = chart.canvas; + const container = canvas && _getParentNode(canvas); + if (!container) { return; } const observer = new MutationObserver(entries => { entries.forEach(entry => { for (let i = 0; i < entry.removedNodes.length; i++) { - if (entry.removedNodes[i] === element) { - fn(); + if (entry.removedNodes[i] === canvas) { + listener(); break; } } }); }); - observer.observe(parent, {childList: true}); + observer.observe(container, {childList: true}); return observer; } -function createAttachObserver(chart, type, listener) { - const canvas = chart.canvas; - const container = canvas && _getParentNode(canvas); - return watchForAttachment(container || canvas, listener); -} - -function createDetachObserver(chart, type, listener) { - const canvas = chart.canvas; - if (canvas) { - return watchForDetachment(canvas, listener); - } -} - function createResizeObserver(chart, type, listener) { const canvas = chart.canvas; const container = canvas && _getParentNode(canvas); - if (container) { - return watchForResize(container, listener); + if (!container) { + return; } + const resize = throttled((width, height) => { + const w = container.clientWidth; + listener(width, height); + if (w < container.clientWidth) { + // 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(); + } + }, window); + + // @ts-ignore until https://github.com/Microsoft/TypeScript/issues/28502 implemented + const observer = new ResizeObserver(entries => { + const entry = entries[0]; + resize(entry.contentRect.width, entry.contentRect.height); + }); + observer.observe(container); + return observer; } function releaseObserver(canvas, type, observer) { @@ -277,7 +247,7 @@ function releaseObserver(canvas, type, observer) { } } -function createProxyForNative(chart, type, listener) { +function createProxyAndListen(chart, type, listener) { const canvas = chart.canvas; const proxy = throttled((event) => { // This case can occur if the chart is destroyed while waiting @@ -378,7 +348,7 @@ export default class DomPlatform extends BasePlatform { detach: createDetachObserver, resize: createResizeObserver }; - const handler = handlers[type] || createProxyForNative; + const handler = handlers[type] || createProxyAndListen; proxies[type] = handler(chart, type, listener); } From 7a33d33c35e3ef8bd52d4df649d5754d50f0cf6c Mon Sep 17 00:00:00 2001 From: Jukka Kurkela Date: Wed, 20 May 2020 20:30:25 +0300 Subject: [PATCH 10/10] Update TypeScript issue URL --- src/platform/platform.dom.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/platform/platform.dom.js b/src/platform/platform.dom.js index 07f807431af..e2c862370c0 100644 --- a/src/platform/platform.dom.js +++ b/src/platform/platform.dom.js @@ -232,7 +232,7 @@ function createResizeObserver(chart, type, listener) { } }, window); - // @ts-ignore until https://github.com/Microsoft/TypeScript/issues/28502 implemented + // @ts-ignore until https://github.com/microsoft/TypeScript/issues/37861 implemented const observer = new ResizeObserver(entries => { const entry = entries[0]; resize(entry.contentRect.width, entry.contentRect.height);