]> git.ipfire.org Git - thirdparty/Chart.js.git/commitdiff
Fix shared option handling (#7731)
authorJukka Kurkela <jukka.kurkela@gmail.com>
Thu, 27 Aug 2020 13:14:08 +0000 (16:14 +0300)
committerGitHub <noreply@github.com>
Thu, 27 Aug 2020 13:14:08 +0000 (09:14 -0400)
Fix shared option handling

14 files changed:
package-lock.json
package.json
rollup.config.js
src/controllers/controller.bar.js
src/controllers/controller.bubble.js
src/controllers/controller.doughnut.js
src/controllers/controller.line.js
src/core/core.animation.js
src/core/core.animations.js
src/core/core.animator.js
src/core/core.controller.js
src/core/core.datasetController.js
test/specs/core.animations.tests.js
types/core/index.d.ts

index 374f637ee0659763daac3a6567d41c162f3caad2..0bedf73d1b7f694f85b4f7b3cac97fbb65da858c 100644 (file)
       "integrity": "sha512-7PiHtLll5LdnKIMw100I+8xJXR5gW2QwWYkT6iJva0bXitZKa/XMrSbdmg3r2Xnaidz9Qumd0VPaMrZlF9V9sA==",
       "dev": true
     },
+    "promise-polyfill": {
+      "version": "8.1.3",
+      "resolved": "https://registry.npmjs.org/promise-polyfill/-/promise-polyfill-8.1.3.tgz",
+      "integrity": "sha512-MG5r82wBzh7pSKDRa9y+vllNHz3e3d4CNj1PQE4BQYxLme0gKYYBm9YENq+UkEikyZ0XbiGWxYlVw3Rl9O/U8g==",
+      "dev": true
+    },
     "psl": {
       "version": "1.8.0",
       "resolved": "https://registry.npmjs.org/psl/-/psl-1.8.0.tgz",
index c37fa2a953c6cf6f43354e1209e930dc9b8dc0a0..5e7cb30616a3de3d7046ae941c504130364e4bd3 100644 (file)
@@ -74,6 +74,7 @@
     "karma-safari-private-launcher": "^1.0.0",
     "moment": "^2.27.0",
     "pixelmatch": "^5.2.1",
+    "promise-polyfill": "^8.1.3",
     "resize-observer-polyfill": "^1.5.1",
     "rollup": "^2.25.0",
     "rollup-plugin-babel": "^4.4.0",
index b65a2864a3c64f38704c7993a51e948284a1e00a..f30161bc1513c218a3add11b7c7025c9868e9378 100644 (file)
@@ -40,7 +40,8 @@ module.exports = [
                input,
                plugins: [
                        inject({
-                               ResizeObserver: 'resize-observer-polyfill'
+                               ResizeObserver: 'resize-observer-polyfill',
+                               Promise: 'promise-polyfill'
                        }),
                        json(),
                        resolve(),
@@ -61,7 +62,8 @@ module.exports = [
                input,
                plugins: [
                        inject({
-                               ResizeObserver: 'resize-observer-polyfill'
+                               ResizeObserver: 'resize-observer-polyfill',
+                               Promise: 'promise-polyfill'
                        }),
                        json(),
                        resolve(),
index 32769d59ba5792d91ad062a38e6908671e1f57ec..0e663c983c79084a84c6cd356079f2ae159ac285 100644 (file)
@@ -219,6 +219,7 @@ export default class BarController extends DatasetController {
 
        initialize() {
                const me = this;
+               me.enableOptionSharing = true;
 
                super.initialize();
 
@@ -241,14 +242,14 @@ export default class BarController extends DatasetController {
                const horizontal = vscale.isHorizontal();
                const ruler = me._getRuler();
                const firstOpts = me.resolveDataElementOptions(start, mode);
-               const sharedOptions = me.getSharedOptions(mode, rectangles[start], firstOpts);
+               const sharedOptions = me.getSharedOptions(firstOpts);
                const includeOptions = me.includeOptions(mode, sharedOptions);
 
-               let i;
+               me.updateSharedOptions(sharedOptions, mode, firstOpts);
 
-               for (i = 0; i < rectangles.length; i++) {
+               for (let i = 0; i < rectangles.length; i++) {
                        const index = start + i;
-                       const options = me.resolveDataElementOptions(index, mode);
+                       const options = sharedOptions || me.resolveDataElementOptions(index, mode);
                        const vpixels = me._calculateBarValuePixels(index, options);
                        const ipixels = me._calculateBarIndexPixels(index, ruler, options);
 
@@ -261,19 +262,11 @@ export default class BarController extends DatasetController {
                                width: horizontal ? undefined : ipixels.size
                        };
 
-                       // all borders are drawn for floating bar
-                       /* TODO: float bars border skipping magic
-                       if (me.getParsed(i)._custom) {
-                               model.borderSkipped = null;
-                       }
-                       */
                        if (includeOptions) {
                                properties.options = options;
                        }
                        me.updateElement(rectangles[i], index, properties, mode);
                }
-
-               me.updateSharedOptions(sharedOptions, mode);
        }
 
        /**
index b7be677bcbaff5fefcfea71b6fda4bc37cb3770e..386317b10a034e0891655f94a2ad1adc38986714 100644 (file)
@@ -3,6 +3,10 @@ import {resolve} from '../helpers/helpers.options';
 import {resolveObjectKey} from '../helpers/helpers.core';
 
 export default class BubbleController extends DatasetController {
+       initialize() {
+               this.enableOptionSharing = true;
+               super.initialize();
+       }
 
        /**
         * Parse array of objects
@@ -69,7 +73,7 @@ export default class BubbleController extends DatasetController {
                const reset = mode === 'reset';
                const {xScale, yScale} = me._cachedMeta;
                const firstOpts = me.resolveDataElementOptions(start, mode);
-               const sharedOptions = me.getSharedOptions(mode, points[start], firstOpts);
+               const sharedOptions = me.getSharedOptions(firstOpts);
                const includeOptions = me.includeOptions(mode, sharedOptions);
 
                for (let i = 0; i < points.length; i++) {
@@ -95,7 +99,7 @@ export default class BubbleController extends DatasetController {
                        me.updateElement(point, index, properties, mode);
                }
 
-               me.updateSharedOptions(sharedOptions, mode);
+               me.updateSharedOptions(sharedOptions, mode, firstOpts);
        }
 
        /**
index bf6cdaef50c7d1e5053553317c2d856266d17877..78e79a4fe9f9a644640306a501ba57b4b580f39e 100644 (file)
@@ -44,6 +44,7 @@ export default class DoughnutController extends DatasetController {
        constructor(chart, datasetIndex) {
                super(chart, datasetIndex);
 
+               this.enableOptionSharing = true;
                this.innerRadius = undefined;
                this.outerRadius = undefined;
                this.offsetX = undefined;
@@ -129,7 +130,7 @@ export default class DoughnutController extends DatasetController {
                const innerRadius = animateScale ? 0 : me.innerRadius;
                const outerRadius = animateScale ? 0 : me.outerRadius;
                const firstOpts = me.resolveDataElementOptions(start, mode);
-               const sharedOptions = me.getSharedOptions(mode, arcs[start], firstOpts);
+               const sharedOptions = me.getSharedOptions(firstOpts);
                const includeOptions = me.includeOptions(mode, sharedOptions);
                let startAngle = opts.rotation;
                let i;
@@ -152,13 +153,13 @@ export default class DoughnutController extends DatasetController {
                                innerRadius
                        };
                        if (includeOptions) {
-                               properties.options = me.resolveDataElementOptions(index, mode);
+                               properties.options = sharedOptions || me.resolveDataElementOptions(index, mode);
                        }
                        startAngle += circumference;
 
                        me.updateElement(arc, index, properties, mode);
                }
-               me.updateSharedOptions(sharedOptions, mode);
+               me.updateSharedOptions(sharedOptions, mode, firstOpts);
        }
 
        calculateTotal() {
index b7202dca76b5e4588be98ad9b7af0baab19c549f..fae5d17a143e6d0384505fe1ec5f60dc3c29c3e5 100644 (file)
@@ -5,6 +5,11 @@ import {resolve} from '../helpers/helpers.options';
 
 export default class LineController extends DatasetController {
 
+       initialize() {
+               this.enableOptionSharing = true;
+               super.initialize();
+       }
+
        update(mode) {
                const me = this;
                const meta = me._cachedMeta;
@@ -31,7 +36,7 @@ export default class LineController extends DatasetController {
                const reset = mode === 'reset';
                const {xScale, yScale, _stacked} = me._cachedMeta;
                const firstOpts = me.resolveDataElementOptions(start, mode);
-               const sharedOptions = me.getSharedOptions(mode, points[start], firstOpts);
+               const sharedOptions = me.getSharedOptions(firstOpts);
                const includeOptions = me.includeOptions(mode, sharedOptions);
                const spanGaps = valueOrDefault(me._config.spanGaps, me.chart.options.spanGaps);
                const maxGapLength = isNumber(spanGaps) ? spanGaps : Number.POSITIVE_INFINITY;
@@ -51,7 +56,7 @@ export default class LineController extends DatasetController {
                        };
 
                        if (includeOptions) {
-                               properties.options = me.resolveDataElementOptions(index, mode);
+                               properties.options = sharedOptions || me.resolveDataElementOptions(index, mode);
                        }
 
                        me.updateElement(point, index, properties, mode);
@@ -59,7 +64,7 @@ export default class LineController extends DatasetController {
                        prevParsed = parsed;
                }
 
-               me.updateSharedOptions(sharedOptions, mode);
+               me.updateSharedOptions(sharedOptions, mode, firstOpts);
        }
 
        /**
index 09e7e03a6673e2534aec36b4e8ade6f1f7034602..f2dcff009ecc381e49f65bda37b02d0717d238f0 100644 (file)
@@ -36,6 +36,7 @@ export default class Animation {
                this._prop = prop;
                this._from = from;
                this._to = to;
+               this._promises = undefined;
        }
 
        active() {
@@ -62,6 +63,7 @@ export default class Animation {
                        // update current evaluated value, for smoother animations
                        me.tick(Date.now());
                        me._active = false;
+                       me._notify(false);
                }
        }
 
@@ -79,6 +81,7 @@ export default class Animation {
 
                if (!me._active) {
                        me._target[prop] = to;
+                       me._notify(true);
                        return;
                }
 
@@ -93,4 +96,19 @@ export default class Animation {
 
                me._target[prop] = me._fn(from, to, factor);
        }
+
+       wait() {
+               const promises = this._promises || (this._promises = []);
+               return new Promise((res, rej) => {
+                       promises.push({res, rej});
+               });
+       }
+
+       _notify(resolved) {
+               const method = resolved ? 'res' : 'rej';
+               const promises = this._promises || [];
+               for (let i = 0; i < promises.length; i++) {
+                       promises[i][method]();
+               }
+       }
 }
index c1b17687ff23d95ac8d9b4321b9a81e15c3bd4f2..36549aa9af9a1b1870712e37e2ad938ca47bbf01 100644 (file)
@@ -57,10 +57,10 @@ defaults.set('animation', {
 function copyOptions(target, values) {
        const oldOpts = target.options;
        const newOpts = values.options;
-       if (!oldOpts || !newOpts || newOpts.$shared) {
+       if (!oldOpts || !newOpts) {
                return;
        }
-       if (oldOpts.$shared) {
+       if (oldOpts.$shared && !newOpts.$shared) {
                target.options = Object.assign({}, oldOpts, newOpts, {$shared: false});
        } else {
                Object.assign(oldOpts, newOpts);
@@ -115,29 +115,25 @@ export default class Animations {
 
        /**
         * Utility to handle animation of `options`.
-        * This should not be called, when animating $shared options to $shared new options.
         * @private
-        * @todo if new options are $shared, target.options should be replaced with those new shared
-        *  options after all animations have completed
         */
        _animateOptions(target, values) {
                const newOptions = values.options;
-               let animations = [];
-
-               if (!newOptions) {
-                       return animations;
+               const options = resolveTargetOptions(target, newOptions);
+               if (!options) {
+                       return [];
                }
-               let options = target.options;
-               if (options) {
-                       if (options.$shared) {
-                               // If the current / old options are $shared, meaning other elements are
-                               // using the same options, we need to clone to become unique.
-                               target.options = options = Object.assign({}, options, {$shared: false, $animations: {}});
-                       }
-                       animations = this._createAnimations(options, newOptions);
-               } else {
-                       target.options = newOptions;
+
+               const animations = this._createAnimations(options, newOptions);
+               if (newOptions.$shared && !options.$shared) {
+                       // Going from distinct options to shared options:
+                       // After all animations are done, assing the shared options object to the element
+                       // So any new updates to the shared options are observed
+                       awaitAll(target.$animations, newOptions).then(() => {
+                               target.options = newOptions;
+                       });
                }
+
                return animations;
        }
 
@@ -214,3 +210,32 @@ export default class Animations {
        }
 }
 
+function awaitAll(animations, properties) {
+       const running = [];
+       const keys = Object.keys(properties);
+       for (let i = 0; i < keys.length; i++) {
+               const anim = animations[keys[i]];
+               if (anim && anim.active()) {
+                       running.push(anim.wait());
+               }
+       }
+       // @ts-ignore
+       return Promise.all(running);
+}
+
+function resolveTargetOptions(target, newOptions) {
+       if (!newOptions) {
+               return;
+       }
+       let options = target.options;
+       if (!options) {
+               target.options = newOptions;
+               return;
+       }
+       if (options.$shared && !newOptions.$shared) {
+               // Going from shared options to distinct one:
+               // Create new options object containing the old shared values and start updating that.
+               target.options = options = Object.assign({}, options, {$shared: false, $animations: {}});
+       }
+       return options;
+}
index bed5e2854b10a469d4b4dd0fb4275efd152cc868..64771ff5f245df00ea41404bd051f023e36a3a13 100644 (file)
@@ -1,6 +1,7 @@
 import {requestAnimFrame} from '../helpers/helpers.extras';
 
 /**
+ * @typedef { import("./core.animation").default } Animation
  * @typedef { import("./core.controller").default } Chart
  */
 
index 490c4029f460c71005419c28e8bbb75d4f25e275..1bd8d39de8164737cf4c283cc2a6fd194b62e5e8 100644 (file)
@@ -608,11 +608,9 @@ class Chart {
                me._updateLayout();
 
                // Can only reset the new controllers after the scales have been updated
-               if (me.options.animation) {
-                       each(newControllers, (controller) => {
-                               controller.reset();
-                       });
-               }
+               each(newControllers, (controller) => {
+                       controller.reset();
+               });
 
                me._updateDatasets(mode);
 
index f79efba872e5c7c63f59e6d2135e4e5310b0c6bc..7f5a958ccd9523980a2b5ede33501c2d8cf823ae 100644 (file)
@@ -155,6 +155,10 @@ function optionKey(key, active) {
        return active ? 'hover' + _capitalize(key) : key;
 }
 
+function isDirectUpdateMode(mode) {
+       return mode === 'reset' || mode === 'none';
+}
+
 export default class DatasetController {
 
        /**
@@ -174,6 +178,8 @@ export default class DatasetController {
                this._parsing = false;
                this._data = undefined;
                this._objectData = undefined;
+               this._sharedOptions = undefined;
+               this.enableOptionSharing = false;
 
                this.initialize();
        }
@@ -610,7 +616,7 @@ export default class DatasetController {
                me.configure();
                me._cachedAnimations = {};
                me._cachedDataOpts = {};
-               me.update(mode);
+               me.update(mode || 'default');
                meta._clip = toClip(valueOrDefault(me._config.clip, defaultClip(meta.xScale, meta.yScale, me.getMaxOverflow())));
        }
 
@@ -684,6 +690,7 @@ export default class DatasetController {
                if (active) {
                        me._addAutomaticHoverColors(index, options);
                }
+
                return options;
        }
 
@@ -718,9 +725,11 @@ export default class DatasetController {
         * @protected
         */
        resolveDataElementOptions(index, mode) {
+               mode = mode || 'default';
                const me = this;
                const active = mode === 'active';
                const cached = me._cachedDataOpts;
+               const sharing = me.enableOptionSharing;
                if (cached[mode]) {
                        return cached[mode];
                }
@@ -736,12 +745,12 @@ export default class DatasetController {
                if (info.cacheable) {
                        // `$shared` indicades this set of options can be shared between multiple elements.
                        // Sharing is used to reduce number of properties to change during animation.
-                       values.$shared = true;
+                       values.$shared = sharing;
 
                        // We cache options by `mode`, which can be 'active' for example. This enables us
                        // to have the 'active' element options and 'default' options to switch between
                        // when interacting.
-                       cached[mode] = values;
+                       cached[mode] = sharing ? Object.freeze(values) : values;
                }
 
                return values;
@@ -809,17 +818,14 @@ export default class DatasetController {
        }
 
        /**
-        * Utility for checking if the options are shared and should be animated separately.
+        * Utility for getting the options object shared between elements
         * @protected
         */
-       getSharedOptions(mode, el, options) {
-               if (!mode) {
-                       // store element option sharing status for usage in interactions
-                       this._sharedOptions = options && options.$shared;
-               }
-               if (mode !== 'reset' && options && options.$shared && el && el.options && el.options.$shared) {
-                       return {target: el.options, options};
+       getSharedOptions(options) {
+               if (!options.$shared) {
+                       return;
                }
+               return this._sharedOptions || (this._sharedOptions = Object.assign({}, options));
        }
 
        /**
@@ -827,10 +833,7 @@ export default class DatasetController {
         * @protected
         */
        includeOptions(mode, sharedOptions) {
-               if (mode === 'hide' || mode === 'show') {
-                       return true;
-               }
-               return mode !== 'resize' && !sharedOptions;
+               return !sharedOptions || isDirectUpdateMode(mode);
        }
 
        /**
@@ -838,7 +841,7 @@ export default class DatasetController {
         * @protected
         */
        updateElement(element, index, properties, mode) {
-               if (mode === 'reset' || mode === 'none') {
+               if (isDirectUpdateMode(mode)) {
                        Object.assign(element, properties);
                } else {
                        this._resolveAnimations(index, mode).update(element, properties);
@@ -849,9 +852,9 @@ export default class DatasetController {
         * Utility to animate the shared options, that are potentially affecting multiple elements.
         * @protected
         */
-       updateSharedOptions(sharedOptions, mode) {
+       updateSharedOptions(sharedOptions, mode, newOptions) {
                if (sharedOptions) {
-                       this._resolveAnimations(undefined, mode).update(sharedOptions.target, sharedOptions.options);
+                       this._resolveAnimations(undefined, mode).update({options: sharedOptions}, {options: newOptions});
                }
        }
 
@@ -860,7 +863,8 @@ export default class DatasetController {
         */
        _setStyle(element, index, mode, active) {
                element.active = active;
-               this._resolveAnimations(index, mode, active).update(element, {options: this.getStyle(index, active)});
+               const options = this.getStyle(index, active);
+               this._resolveAnimations(index, mode, active).update(element, {options: this.getSharedOptions(options) || options});
        }
 
        removeHoverStyle(element, datasetIndex, index) {
index cc668291c64a711aabfade1d8ea1b6484b872823..92cd0e276e2b4dfedce1f77ce7175c77646b7d8a 100644 (file)
@@ -43,4 +43,40 @@ describe('Chart.animations', function() {
                        options: undefined
                })).toBeUndefined();
        });
+
+       it('should assign shared options to target after animations complete', function(done) {
+               const chart = {
+                       draw: function() {},
+                       options: {
+                               animation: {
+                                       debug: false
+                               }
+                       }
+               };
+               const anims = new Chart.Animations(chart, {value: {duration: 100}, option: {duration: 200}});
+
+               const target = {
+                       value: 1,
+                       options: {
+                               option: 2
+                       }
+               };
+               const sharedOpts = {option: 10, $shared: true};
+
+               expect(anims.update(target, {
+                       options: sharedOpts
+               })).toBeTrue();
+
+               expect(target.options !== sharedOpts).toBeTrue();
+
+               Chart.animator.start(chart);
+
+               setTimeout(function() {
+                       expect(Chart.animator.running(chart)).toBeFalse();
+                       expect(target.options === sharedOpts).toBeTrue();
+
+                       Chart.animator.remove(chart);
+                       done();
+               }, 300);
+       });
 });
index 1a487965d76094c288795387738aebe490b1791e..820d34e46eca8b24b23b7a938c7442972e7dd00c 100644 (file)
@@ -320,6 +320,7 @@ export class DatasetController<E extends Element = Element, DSE extends Element
   readonly chart: Chart;
   readonly index: number;
   readonly _cachedMeta: IChartMeta<E, DSE>;
+  enableOptionSharing: boolean;
 
   linkScales(): void;
   getAllParsedValues(scale: Scale): number[];
@@ -345,7 +346,7 @@ export class DatasetController<E extends Element = Element, DSE extends Element
    * Utility for checking if the options are shared and should be animated separately.
    * @protected
    */
-  protected getSharedOptions(mode: UpdateMode, el: E, options: any): undefined | { target: any; options: any };
+  protected getSharedOptions(options: any): undefined | { any };
   /**
    * Utility for determining if `options` should be included in the updated properties
    * @protected
@@ -362,7 +363,7 @@ export class DatasetController<E extends Element = Element, DSE extends Element
    * @protected
    */
 
-  protected updateSharedOptions(sharedOptions: any, mode: UpdateMode): void;
+  protected updateSharedOptions(sharedOptions: any, mode: UpdateMode, newOptions: any): void;
   removeHoverStyle(element: E, datasetIndex: number, index: number): void;
   setHoverStyle(element: E, datasetIndex: number, index: number): void;