Remove dblclick hook to prepare chart redrawing from an event hook (#716

)

* Remove dblclick event hook to prepare chart redrawing

* adds note about breaking change to migration guide

* removes click hooks array not needed anymore

* removes inner if statement because useless

docs/guide/configuration.md

@@ -10,7 +10,6 @@ The following options are available at the top level. They apply to all annotati
 | ---- | ---- | :----: | ---- | ----
 | [`animations`](#animations) | `object` | No | [see here](#default-animations) | To configure which element properties are animated and how.
 | `clip` | `boolean` | No | `true` | Are the annotations clipped to the chartArea.
-| `dblClickSpeed` | `number` | Yes | `350` | Time to detect a double click in ms.
 | `drawTime` | `string` | Yes | `'afterDatasetsDraw'` | See [drawTime](options#draw-time).
 | [`interaction`](options#interaction) | `Object` | No | `options.interaction` | To configure which events trigger plugin interactions
 
@@ -65,6 +64,5 @@ The following options are available for all annotation types. These options can
 | Name | Type | [Scriptable](options#scriptable-options) | Notes
 | ---- | ---- | :----: | ----
 | `click` | `(context, event) => void` | No | Called when a single click occurs on the annotation.
-| `dblClick` | `(context, event) => void` | No | Called when a double click occurs on the annotation.
 | `enter` | `(context, event) => void` | No | Called when the mouse enters the annotation.
 | `leave` | `(context, event) => void` | No | Called when the mouse leaves the annotation.

docs/guide/migrationV2.md

@@ -12,6 +12,7 @@ A number of changes were made to the configuration options passed to the plugin
  * `cornerRadius` option was replaced by `borderRadius` in the box annotation configuration and in the label configuration of line annotation to align with Chart.js options.
  * `xPadding` and `yPadding` options were merged into a single `padding` object in the label configuration of line annotation to align with Chart.js options.
  * `enabled` option was replaced by `display` in the callout configuration of label annotation, in the label configuration of line and box annotations and in the arrow heads configuration of line annotation to have the same option on all elements.
+ * `dblClickSpeed` option was removed from the plugin options because `dblclick` event hook is not available anymore.
 
 ## Elements
 
@@ -58,3 +59,5 @@ The following diagram is showing the element properties about a `'polygon'` anno
 `chartjs-plugin-annotation` plugin version 2 introduces the [`interaction`](options#interaction) options, to configure which events trigger annotation interactions. By default, the plugin uses the [chart interaction configuration](https://www.chartjs.org/docs/latest/configuration/interactions.html#interactions).
 
  * When [scatter charts](https://www.chartjs.org/docs/latest/charts/scatter.html) are used, the interaction default `mode` in Chart.js is `point`, while, in the previous plugin version, the default was `nearest`.
+
+The `dblclick` event hook was removed from annotations options because, being executed asynchronously, it can not enable the chart re-rendering, automatically after processing the event completely. This is important when the user requires re-draws. It gets slow and messy if every event hook does the draw (or update!).

src/annotation.js

@@ -120,7 +120,6 @@ export default {
       },
     },
     clip: true,
-    dblClickSpeed: 350, // ms
     drawTime: 'afterDatasetsDraw',
     interaction: {
       mode: undefined,

src/events.js

@@ -1,9 +1,8 @@
 import {defined, callback} from 'chart.js/helpers';
 import {getElements} from './interaction';
 
-const clickHooks = ['click', 'dblclick'];
 const moveHooks = ['enter', 'leave'];
-export const hooks = clickHooks.concat(moveHooks);
+export const hooks = moveHooks.concat('click');
 
 export function updateListeners(chart, state, options) {
   state.listened = false;
@@ -26,12 +25,8 @@ export function updateListeners(chart, state, options) {
 
   if (!state.listened || !state.moveListened) {
     state.annotations.forEach(scope => {
-      if (!state.listened) {
-        clickHooks.forEach(hook => {
-          if (typeof scope[hook] === 'function') {
-            state.listened = true;
-          }
-        });
+      if (!state.listened && typeof scope.click === 'function') {
+        state.listened = true;
       }
       if (!state.moveListened) {
         moveHooks.forEach(hook => {
@@ -93,24 +88,7 @@ function handleClickEvents(state, event, options) {
   const listeners = state.listeners;
   const elements = getElements(state, event, options.interaction);
   for (const element of elements) {
-    const elOpts = element.options;
-    const dblclick = elOpts.dblclick || listeners.dblclick;
-    const click = elOpts.click || listeners.click;
-    if (element.clickTimeout) {
-      // 2nd click before timeout, so its a double click
-      clearTimeout(element.clickTimeout);
-      delete element.clickTimeout;
-      dispatchEvent(dblclick, element, event);
-    } else if (dblclick) {
-      // if there is a dblclick handler, wait for dblClickSpeed ms before deciding its a click
-      element.clickTimeout = setTimeout(() => {
-        delete element.clickTimeout;
-        dispatchEvent(click, element, event);
-      }, options.dblClickSpeed);
-    } else {
-      // no double click handler, just call the click handler directly
-      dispatchEvent(click, element, event);
-    }
+    dispatchEvent(element.options.click || listeners.click, element, event);
   }
 }
 

test/events.js

@@ -99,31 +99,6 @@ export function testEvents(options) {
         });
         window.triggerMouseEvent(chart, 'click', getCenterPoint(chart));
       });
-
-      it('should detect dbl click event', function(done) {
-        const dblClickSpy = jasmine.createSpy('dblclick');
-
-        targetOptions.dblclick = dblClickSpy;
-        pluginOpts.dblClickSpeed = 1000;
-        pluginOpts.annotations = [options];
-
-        const chart = window.acquireChart(chartConfig);
-        const eventPoint = getCenterPoint(chart);
-
-        let dblClick = false;
-        window.afterEvent(chart, 'click', function() {
-          if (!dblClick) {
-            dblClick = true;
-            window.triggerMouseEvent(chart, 'click', eventPoint);
-          } else {
-            expect(dblClickSpy.calls.count()).toBe(1);
-            delete targetOptions.dblclick;
-            delete pluginOpts.dblClickSpeed;
-            done();
-          }
-        });
-        window.triggerMouseEvent(chart, 'click', eventPoint);
-      });
     });
   });
 }

test/specs/events.spec.js

@@ -93,28 +93,5 @@ describe('Common', function() {
         });
       });
     });
-
-    it('should detect a click event even if 2 clicks are fired', function(done) {
-      const dblClickSpy = jasmine.createSpy('dblclick');
-      annotation.dblclick = dblClickSpy;
-
-      const chart = window.scatterChart(10, 10, {annotation});
-      const eventPoint = window.getCenterPoint(chart);
-
-      let dblClick = false;
-      window.afterEvent(chart, 'click', function() {
-        if (!dblClick) {
-          dblClick = true;
-          setTimeout(() => {
-            window.triggerMouseEvent(chart, 'click', eventPoint);
-          }, 500);
-        } else {
-          expect(dblClickSpy.calls.count()).toBe(0);
-          delete annotation.dblclick;
-          done();
-        }
-      });
-      window.triggerMouseEvent(chart, 'click', eventPoint);
-    });
   });
 });

types/events.d.ts

@@ -23,5 +23,4 @@ export interface AnnotationEvents {
   enter?(context: EventContext, event: ChartEvent): void,
   leave?(context: EventContext, event: ChartEvent): void,
   click?(context: EventContext, event: ChartEvent): void,
-  dblclick?(context: EventContext, event: ChartEvent): void,
 }

types/options.d.ts

@@ -155,7 +155,6 @@ export interface AnnotationPluginOptions extends AnnotationEvents {
   animations?: Record<string, unknown>,
   annotations: AnnotationOptions[] | Record<string, AnnotationOptions>,
   clip?: boolean,
-  dblClickSpeed?: Scriptable<number, PartialEventContext>,
   drawTime?: Scriptable<DrawTime, PartialEventContext>,
   interaction?: CoreInteractionOptions
 }