fix(KeyComboTag): Better rendering on non-Mac operating systems (#7025)

Co-authored-by: svc-changelog <svc-changelog@palantir.com>

packages/core/changelog/@unreleased/pr-7025.v2.yml

@@ -0,0 +1,6 @@
+type: fix
+fix:
+  description: The KeyComboTag component no longer renders modifier key icons on non-Mac
+    operating systems
+  links:
+  - https://github.com/palantir/blueprint/pull/7025

packages/core/src/components/hotkeys/_hotkeys.scss

@@ -4,7 +4,10 @@
 @import "../../common/mixins";
 
 .#{$ns}-key-combo {
-  @include pt-flex-container(row, $pt-grid-size * 0.5);
+  &:not(.#{$ns}-minimal) {
+    @include pt-flex-container(row, $pt-grid-size * 0.5);
+  }
+
   align-items: center;
 }
 

packages/core/src/components/hotkeys/hotkeyParser.ts

@@ -50,7 +50,7 @@ export const CONFIG_ALIASES: KeyMap = {
     esc: "escape",
     escape: "escape",
     minus: "-",
-    mod: isMac() ? "meta" : "ctrl",
+    mod: isMac(undefined) ? "meta" : "ctrl",
     option: "alt",
     plus: "+",
     return: "enter",
@@ -232,15 +232,15 @@ export const getKeyCombo = (e: KeyboardEvent): KeyCombo => {
  * Unlike the parseKeyCombo method, this method does NOT convert shifted
  * action keys. So `"@"` will NOT be converted to `["shift", "2"]`).
  */
-export const normalizeKeyCombo = (combo: string, platformOverride?: string): string[] => {
+export const normalizeKeyCombo = (combo: string, platformOverride: string | undefined): string[] => {
     const keys = combo.replace(/\s/g, "").split("+");
     return keys.map(key => {
         const keyName = CONFIG_ALIASES[key] != null ? CONFIG_ALIASES[key] : key;
         return keyName === "meta" ? (isMac(platformOverride) ? "cmd" : "ctrl") : keyName;
     });
 };
 
-function isMac(platformOverride?: string) {
+export function isMac(platformOverride: string | undefined) {
     // HACKHACK: see https://github.com/palantir/blueprint/issues/5174
     // eslint-disable-next-line deprecation/deprecation
     const platform = platformOverride ?? (typeof navigator !== "undefined" ? navigator.platform : undefined);

packages/core/src/components/hotkeys/keyComboTag.tsx

@@ -33,20 +33,20 @@ import {
 import { AbstractPureComponent, Classes, DISPLAYNAME_PREFIX, type Props } from "../../common";
 import { Icon } from "../icon/icon";
 
-import { normalizeKeyCombo } from "./hotkeyParser";
+import { isMac, normalizeKeyCombo } from "./hotkeyParser";
 
-const KEY_ICONS: Record<string, { icon: React.JSX.Element; iconTitle: string }> = {
-    ArrowDown: { icon: <ArrowDown />, iconTitle: "Down key" },
-    ArrowLeft: { icon: <ArrowLeft />, iconTitle: "Left key" },
-    ArrowRight: { icon: <ArrowRight />, iconTitle: "Right key" },
-    ArrowUp: { icon: <ArrowUp />, iconTitle: "Up key" },
-    alt: { icon: <KeyOption />, iconTitle: "Alt/Option key" },
-    cmd: { icon: <KeyCommand />, iconTitle: "Command key" },
-    ctrl: { icon: <KeyControl />, iconTitle: "Control key" },
+const KEY_ICONS: Record<string, { icon: React.JSX.Element; iconTitle: string; isMacOnly?: boolean }> = {
+    alt: { icon: <KeyOption />, iconTitle: "Alt/Option key", isMacOnly: true },
+    arrowdown: { icon: <ArrowDown />, iconTitle: "Down key" },
+    arrowleft: { icon: <ArrowLeft />, iconTitle: "Left key" },
+    arrowright: { icon: <ArrowRight />, iconTitle: "Right key" },
+    arrowup: { icon: <ArrowUp />, iconTitle: "Up key" },
+    cmd: { icon: <KeyCommand />, iconTitle: "Command key", isMacOnly: true },
+    ctrl: { icon: <KeyControl />, iconTitle: "Control key", isMacOnly: true },
     delete: { icon: <KeyDelete />, iconTitle: "Delete key" },
     enter: { icon: <KeyEnter />, iconTitle: "Enter key" },
-    meta: { icon: <KeyCommand />, iconTitle: "Command key" },
-    shift: { icon: <KeyShift />, iconTitle: "Shift key" },
+    meta: { icon: <KeyCommand />, iconTitle: "Command key", isMacOnly: true },
+    shift: { icon: <KeyShift />, iconTitle: "Shift key", isMacOnly: true },
 };
 
 /** Reverse table of some CONFIG_ALIASES fields, for display by KeyComboTag */
@@ -71,20 +71,30 @@ export interface KeyComboTagProps extends Props {
     minimal?: boolean;
 }
 
-export class KeyComboTag extends AbstractPureComponent<KeyComboTagProps> {
+interface KeyComboTagInternalProps extends KeyComboTagProps {
+    /** Override the oeprating system rendering for internal testing purposes */
+    platformOverride?: string;
+}
+
+export class KeyComboTagInternal extends AbstractPureComponent<KeyComboTagInternalProps> {
     public static displayName = `${DISPLAYNAME_PREFIX}.KeyComboTag`;
 
     public render() {
-        const { className, combo, minimal } = this.props;
-        const keys = normalizeKeyCombo(combo)
+        const { className, combo, minimal, platformOverride } = this.props;
+        const normalizedKeys = normalizeKeyCombo(combo, platformOverride);
+        const keys = normalizedKeys
             .map(key => (key.length === 1 ? key.toUpperCase() : key))
-            .map(minimal ? this.renderMinimalKey : this.renderKey);
-        return <span className={classNames(Classes.KEY_COMBO, className)}>{keys}</span>;
+            .map((key, index) =>
+                minimal
+                    ? this.renderMinimalKey(key, index, index === normalizedKeys.length - 1)
+                    : this.renderKey(key, index),
+            );
+        return <span className={classNames(Classes.KEY_COMBO, className, { [Classes.MINIMAL]: minimal })}>{keys}</span>;
     }
 
     private renderKey = (key: string, index: number) => {
         const keyString = DISPLAY_ALIASES[key] ?? key;
-        const icon = KEY_ICONS[key];
+        const icon = this.getKeyIcon(key);
         const reactKey = `key-${index}`;
         return (
             <kbd className={classNames(Classes.KEY, { [Classes.MODIFIER_KEY]: icon != null })} key={reactKey}>
@@ -94,8 +104,22 @@ export class KeyComboTag extends AbstractPureComponent<KeyComboTagProps> {
         );
     };
 
-    private renderMinimalKey = (key: string, index: number) => {
-        const icon = KEY_ICONS[key];
-        return icon == null ? key : <Icon icon={icon.icon} title={icon.iconTitle} key={`key-${index}`} />;
+    private renderMinimalKey = (key: string, index: number, isLastKey: boolean) => {
+        const icon = this.getKeyIcon(key);
+        if (icon == null) {
+            return isLastKey ? key : <React.Fragment key={`key-${index}`}>{key}&nbsp;+&nbsp;</React.Fragment>;
+        }
+        return <Icon icon={icon.icon} title={icon.iconTitle} key={`key-${index}`} />;
     };
+
+    private getKeyIcon(key: string) {
+        const { platformOverride } = this.props;
+        const icon = KEY_ICONS[key];
+        if (icon?.isMacOnly && !isMac(platformOverride)) {
+            return undefined;
+        }
+        return icon;
+    }
 }
+
+export const KeyComboTag: React.ComponentType<KeyComboTagProps> = KeyComboTagInternal;

packages/core/test/hotkeys/keyComboTagTests.tsx

@@ -18,11 +18,24 @@ import { render, screen } from "@testing-library/react";
 import { expect } from "chai";
 import * as React from "react";
 
-import { KeyComboTag } from "../../src/components/hotkeys";
+import { KeyComboTagInternal } from "../../src/components/hotkeys/keyComboTag";
 
 describe("KeyCombo", () => {
     it("renders key combo", () => {
-        render(<KeyComboTag combo="cmd+C" />);
+        render(<KeyComboTagInternal combo="cmd+C" platformOverride="Mac" />);
         expect(screen.getByText("C")).not.to.be.undefined;
     });
+
+    it("should render minimal key combos on Mac using icons", () => {
+        render(<KeyComboTagInternal combo="mod+C" minimal={true} platformOverride="Mac" />);
+        expect(() => screen.getByText("cmd + C", { exact: false })).to.throw;
+    });
+
+    it("should render minimal key combos on non-Macs using text", () => {
+        render(<KeyComboTagInternal combo="mod+C" minimal={true} platformOverride="Win32" />);
+        const text = screen.getByText("ctrl + C", { exact: false }).innerText;
+        expect(text).to.contain("ctrl");
+        expect(text).to.contain("+");
+        expect(text).to.contain("C");
+    });
 });

packages/docs-app/changelog/@unreleased/pr-7025.v2.yml

@@ -0,0 +1,6 @@
+type: fix
+fix:
+  description: The useHotkeys documentation now shows minimal and non-minimal states
+    for the KeyComboTag component.
+  links:
+  - https://github.com/palantir/blueprint/pull/7025

packages/docs-app/src/examples/core-examples/hotkeyTesterExample.tsx

@@ -51,6 +51,7 @@ export class HotkeyTesterExample extends React.PureComponent<ExampleProps, Hotke
             return (
                 <>
                     <KeyComboTag combo={combo} />
+                    <KeyComboTag combo={combo} minimal={true} />
                     <Code>{combo}</Code>
                 </>
             );