Fix open CommandDialog (#5818)

Signed-off-by: Sebastian Malton <sebastian@malton.name>

src/behaviours/command-pallet/keyboard-shortcuts.test.tsx

@@ -0,0 +1,148 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+
+import type { RenderResult } from "@testing-library/react";
+import userEvent from "@testing-library/user-event";
+import platformInjectable from "../../common/vars/platform.injectable";
+import { type ApplicationBuilder, getApplicationBuilder } from "../../renderer/components/test-utils/get-application-builder";
+
+describe("Command Pallet: keyboard shortcut tests", () => {
+  let applicationBuilder: ApplicationBuilder;
+  let rendered: RenderResult;
+
+  beforeEach(async () => {
+    applicationBuilder = getApplicationBuilder();
+  });
+
+  describe("when on macOS", () => {
+    beforeEach(async () => {
+      applicationBuilder.dis.rendererDi.override(platformInjectable, () => "darwin");
+      rendered = await applicationBuilder.render();
+    });
+
+    it("renders", () => {
+      expect(rendered.baseElement).toMatchSnapshot();
+    });
+
+    it("does not show the command pallet yet", () => {
+      const actual = rendered.queryByTestId("command-container");
+
+      expect(actual).toBeNull();
+    });
+
+    describe("when pressing ESC", () => {
+      beforeEach(() => {
+        userEvent.keyboard("{Escape}");
+      });
+
+      it("renders", () => {
+        expect(rendered.baseElement).toMatchSnapshot();
+      });
+
+      it("does not show the command pallet yet", () => {
+        const actual = rendered.queryByTestId("command-container");
+
+        expect(actual).toBeNull();
+      });
+    });
+
+    describe("when pressing SHIFT+CMD+P", () => {
+      beforeEach(() => {
+        userEvent.keyboard("{Shift>}{Meta>}P{/Meta}{/Shift}");
+      });
+
+      it("renders", () => {
+        expect(rendered.baseElement).toMatchSnapshot();
+      });
+
+      it("shows the command pallet", () => {
+        const actual = rendered.queryByTestId("command-container");
+
+        expect(actual).toBeInTheDocument();
+      });
+
+      describe("when pressing ESC", () => {
+        beforeEach(() => {
+          userEvent.keyboard("{Escape}");
+        });
+
+        it("renders", () => {
+          expect(rendered.baseElement).toMatchSnapshot();
+        });
+
+        it("no longer shows the command pallet", () => {
+          const actual = rendered.queryByTestId("command-container");
+
+          expect(actual).toBeNull();
+        });
+      });
+    });
+  });
+
+  describe("when on linux", () => {
+    beforeEach(async () => {
+      applicationBuilder.dis.rendererDi.override(platformInjectable, () => "linux");
+      rendered = await applicationBuilder.render();
+    });
+
+    it("renders", () => {
+      expect(rendered.baseElement).toMatchSnapshot();
+    });
+
+    it("does not show the command pallet yet", () => {
+      const actual = rendered.queryByTestId("command-container");
+
+      expect(actual).toBeNull();
+    });
+
+    describe("when pressing ESC", () => {
+      beforeEach(() => {
+        userEvent.keyboard("{Escape}");
+      });
+
+      it("renders", () => {
+        expect(rendered.baseElement).toMatchSnapshot();
+      });
+
+      it("does not show the command pallet yet", () => {
+        const actual = rendered.queryByTestId("command-container");
+
+        expect(actual).toBeNull();
+      });
+    });
+
+    describe("when pressing SHIFT+CTRL+P", () => {
+      beforeEach(() => {
+        userEvent.keyboard("{Shift>}{Control>}P{/Control}{/Shift}");
+      });
+
+      it("renders", () => {
+        expect(rendered.baseElement).toMatchSnapshot();
+      });
+
+      it("shows the command pallet", () => {
+        const actual = rendered.queryByTestId("command-container");
+
+        expect(actual).toBeInTheDocument();
+      });
+
+      describe("when pressing ESC", () => {
+        beforeEach(() => {
+          userEvent.keyboard("{Escape}");
+        });
+
+        it("renders", () => {
+          expect(rendered.baseElement).toMatchSnapshot();
+        });
+
+        it("no longer shows the command pallet", () => {
+          const actual = rendered.queryByTestId("command-container");
+
+          expect(actual).toBeNull();
+        });
+      });
+    });
+  });
+});

src/extensions/registries/entity-setting-registry.ts

@@ -38,7 +38,7 @@ export class EntitySettingRegistry extends BaseRegistry<EntitySettingRegistratio
     };
   }
 
-  getItemsForKind = (kind: string, apiVersion: string, source?: string) => {
+  getItemsForKind(kind: string, apiVersion: string, source?: string) {
     let items = this.getItems().filter((item) => {
       return item.kind === kind && item.apiVersions.includes(apiVersion);
     });
@@ -50,5 +50,5 @@ export class EntitySettingRegistry extends BaseRegistry<EntitySettingRegistratio
     }
 
     return items.sort((a, b) => (b.priority ?? 50) - (a.priority ?? 50));
-  };
+  }
 }

src/renderer/components/command-palette/command-container-cluster-frame-child-component.injectable.ts

@@ -9,15 +9,11 @@ import { clusterFrameChildComponentInjectionToken } from "../../frames/cluster-f
 
 const commandContainerClusterFrameChildComponentInjectable = getInjectable({
   id: "command-container-cluster-frame-child-component",
-
   instantiate: () => ({
     id: "command-container",
     shouldRender: computed(() => true),
     Component: CommandContainer,
   }),
-
-  causesSideEffects: true,
-
   injectionToken: clusterFrameChildComponentInjectionToken,
 });
 

src/renderer/components/command-palette/command-container-root-frame-child-component.injectable.ts

@@ -9,15 +9,11 @@ import { CommandContainer } from "./command-container";
 
 const commandContainerRootFrameChildComponentInjectable = getInjectable({
   id: "command-container-root-frame-child-component",
-
   instantiate: () => ({
     id: "command-container",
     shouldRender: computed(() => true),
     Component: CommandContainer,
   }),
-
-  causesSideEffects: true,
-
   injectionToken: rootFrameChildComponentInjectionToken,
 });
 

src/renderer/components/command-palette/command-container.module.scss

@@ -3,7 +3,7 @@
  * Licensed under MIT License. See LICENSE in root directory for more information.
  */
 
-#command-container {
+.CommandContainer {
   position: absolute;
   top: 20px;
   left: 0;
@@ -17,7 +17,7 @@
   color: var(--settingsColor);
   transition: all 0.3s;
 
-  .Input {
+  :global(.Input) {
     label {
       caret-color: var(--blue);
       color: var(--settingsColor);
@@ -33,20 +33,20 @@
     }
   }
 
-  .hint {
+  :global(.hint) {
     padding: 8px;
     display: block;
   }
 
-  .errors {
+  :global(.errors) {
     padding: 8px;
   }
 
-  .Select__menu {
+  :global(.Select__menu) {
     position: relative;
   }
 
-  .Select__control {
+  :global(.Select__control) {
     padding: var(--padding);
     box-shadow: none;
     border-bottom: 1px solid var(--borderFaintColor);
@@ -58,17 +58,17 @@
     }
   }
 
-  .Select__menu {
+  :global(.Select__menu) {
     box-shadow: none;
     background: transparent;
     margin: 0;
   }
 
-  .Select__menu-list {
+  :global(.Select__menu-list) {
     padding: 0;
   }
 
-  .Select__option {
+  :global(.Select__option) {
     background-color: transparent;
     padding: 10px 18px;
 
@@ -78,14 +78,14 @@
       padding-left: 14px;
     }
 
-    &.Select__option--is-focused {
+    &:global(.Select__option--is-focused) {
       background-color: var(--menuSelectedOptionBgc);
       border-left: 4px solid var(--blue);
       padding-left: 14px;
     }
   }
 
-  .Select__menu-notice--no-options {
+  :global(.Select__menu-notice--no-options) {
     padding: 12px;
   }
 }

src/renderer/components/command-palette/command-container.tsx

@@ -4,7 +4,7 @@
  */
 
 
-import "./command-container.scss";
+import styles from "./command-container.module.scss";
 import { disposeOnUnmount, observer } from "mobx-react";
 import React from "react";
 import { Dialog } from "../dialog";
@@ -22,6 +22,7 @@ import matchedClusterIdInjectable from "../../navigation/matched-cluster-id.inje
 import hostedClusterIdInjectable from "../../cluster-frame-context/hosted-cluster-id.injectable";
 import isMacInjectable from "../../../common/vars/is-mac.injectable";
 import legacyOnChannelListenInjectable from "../../ipc/legacy-channel-listen.injectable";
+import { onKeyboardShortcut } from "../../utils/on-keyboard-shortcut";
 
 interface Dependencies {
   addWindowEventListener: AddWindowEventListener;
@@ -34,47 +35,38 @@ interface Dependencies {
 
 @observer
 class NonInjectedCommandContainer extends React.Component<Dependencies> {
-  private escHandler = (event: KeyboardEvent) => {
-    if (event.key === "Escape") {
-      event.stopPropagation();
-      this.props.commandOverlay.close();
-    }
-  };
-
-  handleCommandPalette = () => {
-    const matchedClusterId = this.props.matchedClusterId.get();
-
-    if (matchedClusterId !== undefined) {
-      broadcastMessage(`command-palette:${matchedClusterId}:open`);
-    } else {
-      this.props.commandOverlay.open(<CommandDialog />);
-    }
-  };
-
-  onKeyboardShortcut(action: () => void) {
-    return ({ key, shiftKey, ctrlKey, altKey, metaKey }: KeyboardEvent) => {
-      const ctrlOrCmd = this.props.isMac ? metaKey && !ctrlKey : !metaKey && ctrlKey;
-
-      if (key === "p" && shiftKey && ctrlOrCmd && !altKey) {
-        action();
-      }
-    };
-  }
-
   componentDidMount() {
-    const { clusterId, addWindowEventListener, commandOverlay } = this.props;
+    const { clusterId, addWindowEventListener, commandOverlay, matchedClusterId, isMac } = this.props;
 
     const action = clusterId
       ? () => commandOverlay.open(<CommandDialog />)
-      : this.handleCommandPalette;
+      : () => {
+        const matchedId = matchedClusterId.get();
+
+        if (matchedId) {
+          broadcastMessage(`command-palette:${matchedClusterId}:open`);
+        } else {
+          commandOverlay.open(<CommandDialog />);
+        }
+      };
     const ipcChannel = clusterId
       ? `command-palette:${clusterId}:open`
       : "command-palette:open";
 
     disposeOnUnmount(this, [
       this.props.legacyOnChannelListen(ipcChannel, action),
-      addWindowEventListener("keydown", this.onKeyboardShortcut(action)),
+      addWindowEventListener("keydown", onKeyboardShortcut(
-      addWindowEventListener("keyup", this.escHandler, true),
+        isMac
+          ? "Shift+Cmd+P"
+          : "Shift+Ctrl+P",
+        action,
+      )),
+      addWindowEventListener("keydown", (event) => {
+        if (event.code === "Escape") {
+          event.stopPropagation();
+          this.props.commandOverlay.close();
+        }
+      }),
     ]);
   }
 
@@ -84,11 +76,11 @@ class NonInjectedCommandContainer extends React.Component<Dependencies> {
     return (
       <Dialog
         isOpen={commandOverlay.isOpen}
-        animated={true}
+        animated={false}
         onClose={commandOverlay.close}
         modal={false}
       >
-        <div id="command-container">
+        <div className={styles.CommandContainer} data-testid="command-container">
           {commandOverlay.component}
         </div>
       </Dialog>

src/renderer/components/command-palette/command-overlay.injectable.ts

@@ -4,7 +4,7 @@
  */
 
 import { getInjectable } from "@ogre-tools/injectable";
-import { observable } from "mobx";
+import { action, observable } from "mobx";
 import React from "react";
 
 export class CommandOverlay {
@@ -14,17 +14,17 @@ export class CommandOverlay {
     return Boolean(this.#component.get());
   }
 
-  open = (component: React.ReactElement) => {
+  open = action((component: React.ReactElement) => {
     if (!React.isValidElement(component)) {
       throw new TypeError("CommandOverlay.open must be passed a valid ReactElement");
     }
 
     this.#component.set(component);
-  };
+  });
 
-  close = () => {
+  close = action(() => {
     this.#component.set(null);
-  };
+  });
 
   get component(): React.ReactElement | null {
     return this.#component.get();

src/renderer/components/command-palette/registered-commands/get-entity-setting-commands.injectable.ts

@@ -0,0 +1,21 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+import { getInjectable } from "@ogre-tools/injectable";
+import type { RegisteredEntitySetting } from "../../../../extensions/registries";
+import { EntitySettingRegistry } from "../../../../extensions/registries";
+
+export type GetEntitySettingCommands = (kind: string, apiVersion: string, source?: string) => RegisteredEntitySetting[];
+
+const getEntitySettingCommandsInjectable = getInjectable({
+  id: "get-entity-setting-commands",
+  instantiate: (): GetEntitySettingCommands => {
+    const reg = EntitySettingRegistry.getInstance();
+
+    return (kind, apiVersion, source) => reg.getItemsForKind(kind, apiVersion, source);
+  },
+  causesSideEffects: true,
+});
+
+export default getEntitySettingCommandsInjectable;

src/renderer/components/command-palette/registered-commands/internal-commands.injectable.tsx

@@ -5,7 +5,6 @@
 
 import React from "react";
 import type { RegisteredEntitySetting } from "../../../../extensions/registries";
-import { EntitySettingRegistry } from "../../../../extensions/registries";
 import { HotbarAddCommand } from "../../hotbar/hotbar-add-command";
 import { HotbarRemoveCommand } from "../../hotbar/hotbar-remove-command";
 import { HotbarSwitchCommand } from "../../hotbar/hotbar-switch-command";
@@ -38,6 +37,7 @@ import navigateToJobsInjectable from "../../../../common/front-end-routing/route
 import navigateToCronJobsInjectable from "../../../../common/front-end-routing/routes/cluster/workloads/cron-jobs/navigate-to-cron-jobs.injectable";
 import navigateToCustomResourcesInjectable from "../../../../common/front-end-routing/routes/cluster/custom-resources/custom-resources/navigate-to-custom-resources.injectable";
 import navigateToEntitySettingsInjectable from "../../../../common/front-end-routing/routes/entity-settings/navigate-to-entity-settings.injectable";
+import getEntitySettingCommandsInjectable from "./get-entity-setting-commands.injectable";
 
 export function isKubernetesClusterActive(context: CommandContext): boolean {
   return context.entity?.kind === "KubernetesCluster";
@@ -247,9 +247,7 @@ const internalCommandsInjectable = getInjectable({
 
   instantiate: (di) => getInternalCommands({
     openCommandDialog: di.inject(commandOverlayInjectable).open,
-    getEntitySettingItems: EntitySettingRegistry
+    getEntitySettingItems: di.inject(getEntitySettingCommandsInjectable),
-      .getInstance()
-      .getItemsForKind,
     createTerminalTab: di.inject(createTerminalTabInjectable),
     navigateToPreferences: di.inject(navigateToPreferencesInjectable),
     navigateToHelmCharts: di.inject(navigateToHelmChartsInjectable),

src/renderer/getDiForUnitTesting.tsx

@@ -57,9 +57,7 @@ import goForwardInjectable from "./components/layout/top-bar/go-forward.injectab
 import closeWindowInjectable from "./components/layout/top-bar/close-window.injectable";
 import maximizeWindowInjectable from "./components/layout/top-bar/maximize-window.injectable";
 import toggleMaximizeWindowInjectable from "./components/layout/top-bar/toggle-maximize-window.injectable";
-import commandContainerRootFrameChildComponentInjectable from "./components/command-palette/command-container-root-frame-child-component.injectable";
 import type { HotbarStore } from "../common/hotbars/store";
-import commandContainerClusterFrameChildComponentInjectable from "./components/command-palette/command-container-cluster-frame-child-component.injectable";
 import cronJobTriggerDialogClusterFrameChildComponentInjectable from "./components/+workloads-cronjobs/cron-job-trigger-dialog-cluster-frame-child-component.injectable";
 import deploymentScaleDialogClusterFrameChildComponentInjectable from "./components/+workloads-deployments/scale/deployment-scale-dialog-cluster-frame-child-component.injectable";
 import replicasetScaleDialogClusterFrameChildComponentInjectable from "./components/+workloads-replicasets/scale-dialog/replicaset-scale-dialog-cluster-frame-child-component.injectable";
@@ -72,6 +70,8 @@ import setupSystemCaInjectable from "./frames/root-frame/setup-system-ca.injecta
 import extensionShouldBeEnabledForClusterFrameInjectable from "./extension-loader/extension-should-be-enabled-for-cluster-frame.injectable";
 import { asyncComputed } from "@ogre-tools/injectable-react";
 import forceUpdateModalRootFrameComponentInjectable from "./application-update/force-update-modal/force-update-modal-root-frame-component.injectable";
+import legacyOnChannelListenInjectable from "./ipc/legacy-channel-listen.injectable";
+import getEntitySettingCommandsInjectable from "./components/command-palette/registered-commands/get-entity-setting-commands.injectable";
 
 export const getDiForUnitTesting = (opts: { doGeneralOverrides?: boolean } = {}) => {
   const {
@@ -110,17 +110,12 @@ export const getDiForUnitTesting = (opts: { doGeneralOverrides?: boolean } = {})
     di.override(appVersionInjectable, () => "1.0.0");
 
     di.override(historyInjectable, () => createMemoryHistory());
-
+    di.override(legacyOnChannelListenInjectable, () => () => noop);
     di.override(requestAnimationFrameInjectable, () => (callback) => callback());
-
     di.override(lensResourcesDirInjectable, () => "/irrelevant");
 
-    // TODO: Remove side-effects and shared global state
+    // TODO: remove when entity settings registry is refactored
-    di.override(commandContainerRootFrameChildComponentInjectable, () => ({
+    di.override(getEntitySettingCommandsInjectable, () => () => []);
-      Component: () => null,
-      id: "command-container",
-      shouldRender: computed(() => false),
-    }));
 
     di.override(forceUpdateModalRootFrameComponentInjectable, () => ({
       id: "force-update-modal",
@@ -135,7 +130,6 @@ export const getDiForUnitTesting = (opts: { doGeneralOverrides?: boolean } = {})
 
     // TODO: Remove side-effects and shared global state
     const clusterFrameChildComponentInjectables: Injectable<any, any, any>[] = [
-      commandContainerClusterFrameChildComponentInjectable,
       cronJobTriggerDialogClusterFrameChildComponentInjectable,
       deploymentScaleDialogClusterFrameChildComponentInjectable,
       replicasetScaleDialogClusterFrameChildComponentInjectable,

src/renderer/utils/on-keyboard-shortcut.ts

@@ -0,0 +1,54 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+
+import type { WindowEventListener } from "../window/event-listener.injectable";
+
+function parseKeyDownDescriptor(descriptor: string): (event: KeyboardEvent) => boolean {
+  const parts = new Set((
+    descriptor
+      .split("+")
+      .filter(Boolean)
+      .map(part => part.toLowerCase())
+  ));
+
+  if (parts.size === 0) {
+    return () => true;
+  }
+
+  const hasShift = parts.delete("shift");
+  const hasAlt = parts.delete("alt");
+
+  const rawHasCtrl = parts.delete("ctrl");
+  const rawHasControl = parts.delete("control");
+  const hasCtrl = rawHasCtrl || rawHasControl;
+
+  const rawHasMeta = parts.delete("meta");
+  const rawHasCmd = parts.delete("cmd");
+  const hasMeta = rawHasCmd || rawHasMeta; // This means either matches
+
+  const [key, ...rest] = [...parts];
+
+  if (rest.length !== 0) {
+    throw new Error("only single key combinations are currently supported");
+  }
+
+  return (event) => {
+    return event.altKey === hasAlt
+      && event.shiftKey === hasShift
+      && event.ctrlKey === hasCtrl
+      && event.metaKey === hasMeta
+      && event.key.toLowerCase() === key.toLowerCase();
+  };
+}
+
+export function onKeyboardShortcut(descriptor: string, action: () => void): WindowEventListener<"keydown"> {
+  const isMatchingEvent = parseKeyDownDescriptor(descriptor);
+
+  return (event) => {
+    if (isMatchingEvent(event)) {
+      action();
+    }
+  };
+}

src/renderer/window/event-listener.injectable.ts

@@ -7,8 +7,9 @@ import { getInjectable } from "@ogre-tools/injectable";
 import type { Disposer } from "../utils";
 
 export type AddWindowEventListener = typeof addWindowEventListener;
+export type WindowEventListener<K extends keyof WindowEventMap> = (this: Window, ev: WindowEventMap[K]) => any;
 
-function addWindowEventListener<K extends keyof WindowEventMap>(type: K, listener: (this: Window, ev: WindowEventMap[K]) => any, options?: boolean | AddEventListenerOptions): Disposer {
+function addWindowEventListener<K extends keyof WindowEventMap>(type: K, listener: WindowEventListener<K>, options?: boolean | AddEventListenerOptions): Disposer {
   window.addEventListener(type, listener, options);
 
   return () => void window.removeEventListener(type, listener);