Class instead of closure

Signed-off-by: Colin Grant <[email protected]>

packages/core/src/browser/core-preferences.ts

@@ -97,11 +97,19 @@ export const corePreferenceSchema: PreferenceSchema = {
             default: 'code',
             description: 'Whether to interpret keypresses by the `code` of the physical key, or by the `keyCode` provided by the OS.'
         },
+        'application.clickTime': {
+            type: 'number',
+            minimum: 0,
+            default: 500, // This is Windows' default.
+            description: 'The number of milliseconds within which a second click should trigger a double-click action'
+        }
     }
 };
 
 export interface CoreConfiguration {
     'application.confirmExit': 'never' | 'ifRequired' | 'always';
+    'application.clickTime': number;
+    'files.encoding': string
     'keyboard.dispatch': 'code' | 'keyCode';
     'workbench.list.openMode': 'singleClick' | 'doubleClick';
     'workbench.commandPalette.history': number;
@@ -110,7 +118,6 @@ export interface CoreConfiguration {
     'workbench.colorTheme': string;
     'workbench.iconTheme': string | null;
     'workbench.silentNotifications': boolean;
-    'files.encoding': string
     'workbench.tree.renderIndentGuides': 'onHover' | 'none' | 'always';
 }
 

packages/core/src/browser/frontend-application-module.ts

@@ -104,6 +104,7 @@ import {
 } from './quick-input';
 import { QuickAccessContribution } from './quick-input/quick-access';
 import { QuickCommandService } from './quick-input/quick-command-service';
+import { ClickEventHandler, ClickEventHandlerFactory, ClickEventHandlerOptions } from './widgets/event-utils';
 
 export { bindResourceProvider, bindMessageService, bindPreferenceService };
 
@@ -353,4 +354,11 @@ export const frontendApplicationModule = new ContainerModule((bind, unbind, isBo
     bind(CredentialsService).to(CredentialsServiceImpl);
 
     bind(ContributionFilterRegistry).to(ContributionFilterRegistryImpl).inSingletonScope();
+
+    bind(ClickEventHandler).toSelf();
+    bind(ClickEventHandlerFactory).toFactory(({ container }) => (options: ClickEventHandlerOptions) => {
+        const child = container.createChild();
+        child.bind(ClickEventHandlerOptions).toConstantValue(options);
+        return child.get(ClickEventHandler);
+    });
 });

packages/core/src/browser/tree/tree-widget.tsx

@@ -39,7 +39,7 @@ import { TreeWidgetSelection } from './tree-widget-selection';
 import { MaybePromise } from '../../common/types';
 import { LabelProvider } from '../label-provider';
 import { CorePreferences } from '../core-preferences';
-import { createClickEventHandler } from '../widgets/event-utils';
+import { ClickEventHandler, ClickEventHandlerFactory } from '../widgets/event-utils';
 
 const debounce = require('lodash.debounce');
 
@@ -148,14 +148,14 @@ export namespace TreeWidget {
          */
         readonly shiftKey: boolean;
     }
-
 }
 
 @injectable()
 export class TreeWidget extends ReactWidget implements StatefulWidget {
 
     protected searchBox: SearchBox;
     protected searchHighlights: Map<string, TreeDecoration.CaptionHighlight>;
+    protected nodeClickEventHandler: ClickEventHandler<React.MouseEvent>;
 
     @inject(TreeDecoratorService)
     protected readonly decoratorService: TreeDecoratorService;
@@ -175,6 +175,8 @@ export class TreeWidget extends ReactWidget implements StatefulWidget {
     @inject(CorePreferences)
     protected readonly corePreferences: CorePreferences;
 
+    @inject(ClickEventHandlerFactory) protected readonly clickEventHandlerFactory: ClickEventHandlerFactory;
+
     protected shouldScrollToRow = true;
 
     constructor(
@@ -238,7 +240,14 @@ export class TreeWidget extends ReactWidget implements StatefulWidget {
                 }),
             ]);
         }
+        this.nodeClickEventHandler = this.clickEventHandlerFactory({
+            actions: [
+                (event: React.MouseEvent<HTMLElement>, node: TreeNode) => this.handleClickEvent(node, event),
+                (event: React.MouseEvent<HTMLElement>, node: TreeNode) => this.handleDblClickEvent(node, event),
+            ]
+        });
         this.toDispose.pushAll([
+            this.nodeClickEventHandler,
             this.model,
             this.model.onChanged(() => this.updateRows()),
             this.model.onSelectionChanged(() => this.updateScrollToRow({ resize: false })),
@@ -887,17 +896,10 @@ export class TreeWidget extends ReactWidget implements StatefulWidget {
     protected createNodeAttributes(node: TreeNode, props: NodeProps): React.Attributes & React.HTMLAttributes<HTMLElement> {
         const className = this.createNodeClassNames(node, props).join(' ');
         const style = this.createNodeStyle(node, props);
-        const clickListener = createClickEventHandler<React.MouseEvent<HTMLElement>>(
-            event => this.handleClickEvent(node, event),
-            event => this.handleDblClickEvent(node, event),
-            // This component can't use invokeSingle because running the handler causes the tree to refresh, replacing the listener and its closure
-            { timeout: 250, invokeImmediately: false }
-        );
         return {
             className,
             style,
-            onClick: clickListener,
-            onDoubleClick: clickListener,
+            onClick: event => this.nodeClickEventHandler.invoke(event, node),
             onContextMenu: event => this.handleContextMenuEvent(node, event)
         };
     }

packages/core/src/browser/widgets/event-utils.ts

@@ -14,71 +14,98 @@
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
 
-export interface Handler<T> { (stimulus: T): unknown; }
-export const isReactEvent = (event?: object): event is { persist(): unknown } => !!event && 'persist' in event;
-export interface MinimalEvent {
-    type: string;
-    __superseded?: boolean;
+import { Disposable } from '../../common';
+import { inject, injectable } from 'inversify';
+import * as React from 'react';
+import { CorePreferences } from '../core-preferences';
+import { wait } from '../../common/promise-util';
+
+export const isReactEvent = (event: MouseEvent | React.MouseEvent): event is React.MouseEvent => 'nativeEvent' in event;
+
+interface MouseEventHandlerPlus<T extends MouseEvent | React.MouseEvent> {
+    (event: T, ...additionalArgs: unknown[]): unknown;
 }
-export interface ClickHandlerOptions {
+
+export interface ClickEventHandlerOptions<T extends MouseEvent | React.MouseEvent = MouseEvent> {
     /**
-     * Time in milliseconds to wait for a second click.
+     * A function to invoke on every click, regardless of other conditions.
      */
-    timeout: number;
+    immediateAction?: MouseEventHandlerPlus<T>;
     /**
-     * If `true`, the single-click handler will be invoked immediately on the first click. If true, `invokeSingleOnDouble` is ignored.
-     * Use this option with caution: if the single-click handler triggers a rerender, it may invalidate or ignore the state of the handlers
-     * created here.
+     * Functions to invoke on a given number of clicks, and no more.
+     * E.g. the action at index 0 will be invoked on a single click if no additional click
+     * comes within a set interval.
      */
-    invokeImmediately?: boolean;
+    actions: MouseEventHandlerPlus<T>[];
+}
+
+export const ClickEventHandlerOptions = Symbol('ClickEventHandlerOptions');
+export interface ClickEventHandlerFactory {
+    <T extends MouseEvent | React.MouseEvent>(options: ClickEventHandlerOptions<T>): ClickEventHandler<T>;
+}
+export const ClickEventHandlerFactory = Symbol('ClickEventHandlerFactory');
+
+@injectable()
+export class ClickEventHandler<T extends MouseEvent | React.MouseEvent> implements Disposable {
+
+    @inject(CorePreferences) readonly preferences: CorePreferences;
+    @inject(ClickEventHandlerOptions) protected readonly options: ClickEventHandlerOptions<T>;
+
+    protected disposed = false;
+
     /**
-     * If `true`, the single click handler will be invoked and awaited before the double-click handler is invoked.
-     * Ignored if `invokeImmediately` is `true`.
+     * Setting `.canceled` to true will prevent the handler from running.
+     * Calling `.run()` will invoke the handler immediately and prevent future invocations.
      */
-    invokeSingleOnDouble?: boolean;
-}
+    protected queuedInvocation: { event: T, canceled: boolean, run: () => unknown } | undefined;
 
-/**
- * @returns a single handler that should be applied to be both 'click' and 'dblclick' events for a given element.
- * If the user clicks once in the interval specified, the single-click handler will be invoked.
- * If the user clicks twice in the interval, *only* the double-click handler will be invoked.
- */
-export function createClickEventHandler<T extends MinimalEvent>(
-    singleClickHandler: Handler<T>,
-    doubleClickHandler: Handler<T>,
-    options: ClickHandlerOptions,
-): Handler<T> {
-    const { timeout, invokeImmediately, invokeSingleOnDouble } = options;
-    const shouldInvokeSingleOnDouble = invokeSingleOnDouble && !invokeImmediately;
-    let deferredEvent: T | undefined;
-    return async (event: T): Promise<void> => {
+    async invoke(event: T, ...additionalArguments: unknown[]): Promise<void> {
+        if (this.disposed) {
+            return;
+        }
         if (isReactEvent(event)) {
             event.persist();
         }
-        if (!deferredEvent && event.type === 'click') {
-            deferredEvent = event;
-            if (invokeImmediately) {
-                singleClickHandler(event);
+        const { immediateAction, actions } = this.options;
+        if (immediateAction) {
+            immediateAction(event, ...additionalArguments);
+        }
+        const { currentTarget, detail } = event;
+        const handler = actions[detail - 1];
+        if (currentTarget && !!handler) {
+            if (this.queuedInvocation && detail > this.queuedInvocation.event.detail) { // Click is on same thing as before. Cancel that invocation.
+                this.queuedInvocation.canceled = true;
+            } else if (this.queuedInvocation) { // Clicking somewhere else or OS timer has run out. Run handler for last invocation immediately.
+                this.queuedInvocation.run();
             }
-            await new Promise(resolve => setTimeout(resolve, timeout));
-            if (!event.__superseded) { // No double click has occurred.
-                deferredEvent = undefined;
-                if (!invokeImmediately) { // We haven't run it yet.
-                    singleClickHandler(event);
+            const thisCall = {
+                event,
+                canceled: false,
+                run(): void {
+                    if (!this.canceled) {
+                        this.canceled = true;
+                        handler(event, ...additionalArguments);
+                    }
                 }
+            };
+            this.queuedInvocation = thisCall;
+            // If detail == actions.length, it's the last defined handler.
+            // In that case, we run it immediately. If >, we don't reach this code.
+            if (detail < actions.length) {
+                await wait(this.preferences.get('application.clickTime', 500));
             }
-        } else if (event.type === 'dblclick') {
-            if (deferredEvent) {
-                deferredEvent.__superseded = true;
-                if (shouldInvokeSingleOnDouble) {
-                    const eventToHandle = deferredEvent;
-                    deferredEvent = undefined; // Clear state immediately in case of triple click.
-                    try {
-                        await singleClickHandler(eventToHandle);
-                    } catch { }
-                }
+            thisCall.run();
+            if (this.queuedInvocation === thisCall) {
+                this.queuedInvocation = undefined;
             }
-            doubleClickHandler(event);
         }
-    };
+    }
+
+    dispose(): void {
+        this.disposed = true;
+        if (this.queuedInvocation) {
+            this.queuedInvocation.canceled = true;
+            this.queuedInvocation = undefined;
+        }
+    }
 }