From 4f756fc6ec8f783173df9ed864c47b0f0d36d774 Mon Sep 17 00:00:00 2001 From: Chris Lewis Date: Thu, 29 Jun 2017 11:44:48 -0700 Subject: [PATCH 1/4] Hide dialog after delay on 'shift + ?' press --- .../src/components/hotkeys/hotkeysDialog.tsx | 16 +++++++++++++--- .../core/src/components/hotkeys/hotkeysEvents.ts | 12 +++++++++--- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/packages/core/src/components/hotkeys/hotkeysDialog.tsx b/packages/core/src/components/hotkeys/hotkeysDialog.tsx index c2632595344..88e15576160 100644 --- a/packages/core/src/components/hotkeys/hotkeysDialog.tsx +++ b/packages/core/src/components/hotkeys/hotkeysDialog.tsx @@ -30,7 +30,8 @@ class HotkeysDialog { private container: HTMLElement; private hotkeysQueue = [] as IHotkeyProps[][]; private isDialogShowing = false; - private timeoutToken = 0; + private showTimeoutToken = 0; + private hideTimeoutToken = 0; public render() { if (this.container == null) { @@ -59,8 +60,13 @@ class HotkeysDialog { this.hotkeysQueue.push(hotkeys); // reset timeout for debounce - clearTimeout(this.timeoutToken); - this.timeoutToken = setTimeout(this.show, 10); + clearTimeout(this.showTimeoutToken); + this.showTimeoutToken = setTimeout(this.show, 10); + } + + public hideAfterDelay() { + clearTimeout(this.hideTimeoutToken); + this.hideTimeoutToken = setTimeout(this.hide, 10); } public show = () => { @@ -141,3 +147,7 @@ export function showHotkeysDialog(hotkeys: IHotkeyProps[]) { export function hideHotkeysDialog() { HOTKEYS_DIALOG.hide(); } + +export function hideHotkeysDialogAfterDelay() { + HOTKEYS_DIALOG.hideAfterDelay(); +} diff --git a/packages/core/src/components/hotkeys/hotkeysEvents.ts b/packages/core/src/components/hotkeys/hotkeysEvents.ts index f6959132195..73d3f288c40 100644 --- a/packages/core/src/components/hotkeys/hotkeysEvents.ts +++ b/packages/core/src/components/hotkeys/hotkeysEvents.ts @@ -11,7 +11,7 @@ import { safeInvoke } from "../../common/utils"; import { Hotkey, IHotkeyProps } from "./hotkey"; import { comboMatches, getKeyCombo, IKeyCombo, parseKeyCombo } from "./hotkeyParser"; import { IHotkeysProps } from "./hotkeys"; -import { isHotkeysDialogShowing, showHotkeysDialog } from "./hotkeysDialog"; +import { hideHotkeysDialogAfterDelay, isHotkeysDialogShowing, showHotkeysDialog } from "./hotkeysDialog"; const SHOW_DIALOG_KEY = "?"; @@ -53,14 +53,20 @@ export class HotkeysEvents { } public handleKeyDown = (e: KeyboardEvent) => { - if (this.isTextInput(e) || isHotkeysDialogShowing()) { + if (this.isTextInput(e)) { return; } const combo = getKeyCombo(e); if (comboMatches(parseKeyCombo(SHOW_DIALOG_KEY), combo)) { - showHotkeysDialog(this.actions.map((action) => action.props)); + if (isHotkeysDialogShowing()) { + hideHotkeysDialogAfterDelay(); + } else { + showHotkeysDialog(this.actions.map((action) => action.props)); + } + return; + } else if (isHotkeysDialogShowing()) { return; } From 476a0b62b47a33ff43e2570adcb94ed478971726 Mon Sep 17 00:00:00 2001 From: Chris Lewis Date: Mon, 10 Jul 2017 15:59:36 -0700 Subject: [PATCH 2/4] Respond to CR feedback --- .../src/components/hotkeys/hotkeysDialog.tsx | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/packages/core/src/components/hotkeys/hotkeysDialog.tsx b/packages/core/src/components/hotkeys/hotkeysDialog.tsx index 88e15576160..a94a3d1e42c 100644 --- a/packages/core/src/components/hotkeys/hotkeysDialog.tsx +++ b/packages/core/src/components/hotkeys/hotkeysDialog.tsx @@ -22,6 +22,12 @@ export interface IHotkeysDialogProps extends IDialogProps { globalHotkeysGroup?: string; } +/** + * The delay before showing or hiding the dialog. Should be long enough to + * allow all registered hotkey listeners to execute first. + */ +const DELAY_IN_MS = 10; + class HotkeysDialog { public componentProps = { globalHotkeysGroup: "Global hotkeys", @@ -30,8 +36,8 @@ class HotkeysDialog { private container: HTMLElement; private hotkeysQueue = [] as IHotkeyProps[][]; private isDialogShowing = false; - private showTimeoutToken = 0; - private hideTimeoutToken = 0; + private showTimeoutToken: number; + private hideTimeoutToken: number; public render() { if (this.container == null) { @@ -61,12 +67,17 @@ class HotkeysDialog { // reset timeout for debounce clearTimeout(this.showTimeoutToken); - this.showTimeoutToken = setTimeout(this.show, 10); + this.showTimeoutToken = setTimeout(this.show, DELAY_IN_MS); } + /** + * Since one hotkey toggles the dialog open and closed, we need to + * introduce a delay to ensure that all hotkey listeners fire with the + * dialog in a consistent state. + */ public hideAfterDelay() { clearTimeout(this.hideTimeoutToken); - this.hideTimeoutToken = setTimeout(this.hide, 10); + this.hideTimeoutToken = setTimeout(this.hide, DELAY_IN_MS); } public show = () => { From 01f747bf79b29b6148e1210405442d178c4b59dd Mon Sep 17 00:00:00 2001 From: Chris Lewis Date: Mon, 17 Jul 2017 10:17:02 -0700 Subject: [PATCH 3/4] Fix build --- packages/core/src/components/hotkeys/hotkeysEvents.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/core/src/components/hotkeys/hotkeysEvents.ts b/packages/core/src/components/hotkeys/hotkeysEvents.ts index 4b6911a59ec..d848d122a47 100644 --- a/packages/core/src/components/hotkeys/hotkeysEvents.ts +++ b/packages/core/src/components/hotkeys/hotkeysEvents.ts @@ -53,10 +53,6 @@ export class HotkeysEvents { } public handleKeyDown = (e: KeyboardEvent) => { - if (this.isTextInput(e)) { - return; - } - const combo = getKeyCombo(e); const isTextInput = this.isTextInput(e); From 8a3a015db12553fdba10aec86df9529bb5195982 Mon Sep 17 00:00:00 2001 From: Chris Lewis Date: Mon, 24 Jul 2017 15:27:08 -0700 Subject: [PATCH 4/4] Move function comment --- packages/core/src/components/hotkeys/hotkeysDialog.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/core/src/components/hotkeys/hotkeysDialog.tsx b/packages/core/src/components/hotkeys/hotkeysDialog.tsx index a94a3d1e42c..e48d3e974e3 100644 --- a/packages/core/src/components/hotkeys/hotkeysDialog.tsx +++ b/packages/core/src/components/hotkeys/hotkeysDialog.tsx @@ -70,11 +70,6 @@ class HotkeysDialog { this.showTimeoutToken = setTimeout(this.show, DELAY_IN_MS); } - /** - * Since one hotkey toggles the dialog open and closed, we need to - * introduce a delay to ensure that all hotkey listeners fire with the - * dialog in a consistent state. - */ public hideAfterDelay() { clearTimeout(this.hideTimeoutToken); this.hideTimeoutToken = setTimeout(this.hide, DELAY_IN_MS); @@ -159,6 +154,11 @@ export function hideHotkeysDialog() { HOTKEYS_DIALOG.hide(); } +/** + * Use this function instead of `hideHotkeysDialog` if you need to ensure that all hotkey listeners + * have time to execute with the dialog in a consistent open state. This can avoid flickering the + * dialog between open and closed states as successive listeners fire. + */ export function hideHotkeysDialogAfterDelay() { HOTKEYS_DIALOG.hideAfterDelay(); }