From 0386e290c578dad2bef2078fd3790bfe62e0aed1 Mon Sep 17 00:00:00 2001 From: Jonah Scheinerman Date: Thu, 24 Oct 2024 11:01:28 -0400 Subject: [PATCH 01/10] fix(KeyComboTag): Better rendering on non-Mac operating systems --- .../src/components/hotkeys/hotkeyParser.ts | 2 +- .../src/components/hotkeys/keyComboTag.tsx | 50 ++++++++++++------- .../core-examples/hotkeyTesterExample.tsx | 1 + 3 files changed, 35 insertions(+), 18 deletions(-) diff --git a/packages/core/src/components/hotkeys/hotkeyParser.ts b/packages/core/src/components/hotkeys/hotkeyParser.ts index 14f0c5dbfe..e0fdda17d9 100644 --- a/packages/core/src/components/hotkeys/hotkeyParser.ts +++ b/packages/core/src/components/hotkeys/hotkeyParser.ts @@ -240,7 +240,7 @@ export const normalizeKeyCombo = (combo: string, platformOverride?: string): str }); }; -function isMac(platformOverride?: string) { +export function isMac(platformOverride?: string) { // HACKHACK: see https://github.com/palantir/blueprint/issues/5174 // eslint-disable-next-line deprecation/deprecation const platform = platformOverride ?? (typeof navigator !== "undefined" ? navigator.platform : undefined); diff --git a/packages/core/src/components/hotkeys/keyComboTag.tsx b/packages/core/src/components/hotkeys/keyComboTag.tsx index b97e548314..7a0be3cd4e 100644 --- a/packages/core/src/components/hotkeys/keyComboTag.tsx +++ b/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 = { - ArrowDown: { icon: , iconTitle: "Down key" }, - ArrowLeft: { icon: , iconTitle: "Left key" }, - ArrowRight: { icon: , iconTitle: "Right key" }, - ArrowUp: { icon: , iconTitle: "Up key" }, - alt: { icon: , iconTitle: "Alt/Option key" }, - cmd: { icon: , iconTitle: "Command key" }, - ctrl: { icon: , iconTitle: "Control key" }, +const KEY_ICONS: Record = { + alt: { icon: , iconTitle: "Alt/Option key", isMacOnly: true }, + arrowdown: { icon: , iconTitle: "Down key" }, + arrowleft: { icon: , iconTitle: "Left key" }, + arrowright: { icon: , iconTitle: "Right key" }, + arrowup: { icon: , iconTitle: "Up key" }, + cmd: { icon: , iconTitle: "Command key", isMacOnly: true }, + ctrl: { icon: , iconTitle: "Control key", isMacOnly: true }, delete: { icon: , iconTitle: "Delete key" }, enter: { icon: , iconTitle: "Enter key" }, - meta: { icon: , iconTitle: "Command key" }, - shift: { icon: , iconTitle: "Shift key" }, + meta: { icon: , iconTitle: "Command key", isMacOnly: true }, + shift: { icon: , iconTitle: "Shift key", isMacOnly: true }, }; /** Reverse table of some CONFIG_ALIASES fields, for display by KeyComboTag */ @@ -76,15 +76,20 @@ export class KeyComboTag extends AbstractPureComponent { public render() { const { className, combo, minimal } = this.props; - const keys = normalizeKeyCombo(combo) + const normalizedKeys = normalizeKeyCombo(combo); + const keys = normalizedKeys .map(key => (key.length === 1 ? key.toUpperCase() : key)) - .map(minimal ? this.renderMinimalKey : this.renderKey); + .map((key, index) => + minimal + ? this.renderMinimalKey(key, index, index === normalizedKeys.length - 1) + : this.renderKey(key, index), + ); return {keys}; } private renderKey = (key: string, index: number) => { const keyString = DISPLAY_ALIASES[key] ?? key; - const icon = KEY_ICONS[key]; + const icon = getKeyIcon(key); const reactKey = `key-${index}`; return ( @@ -94,8 +99,19 @@ export class KeyComboTag extends AbstractPureComponent { ); }; - private renderMinimalKey = (key: string, index: number) => { - const icon = KEY_ICONS[key]; - return icon == null ? key : ; + private renderMinimalKey = (key: string, index: number, isLastKey: boolean) => { + const icon = getKeyIcon(key); + if (icon == null) { + return isLastKey ? key : <>{key} + ; + } + return ; }; } + +function getKeyIcon(key: string) { + const icon = KEY_ICONS[key]; + if (icon?.isMacOnly && !isMac()) { + return undefined; + } + return icon; +} diff --git a/packages/docs-app/src/examples/core-examples/hotkeyTesterExample.tsx b/packages/docs-app/src/examples/core-examples/hotkeyTesterExample.tsx index 9871a55e0f..1d6d6542bc 100644 --- a/packages/docs-app/src/examples/core-examples/hotkeyTesterExample.tsx +++ b/packages/docs-app/src/examples/core-examples/hotkeyTesterExample.tsx @@ -51,6 +51,7 @@ export class HotkeyTesterExample extends React.PureComponent + {combo} ); From f3d75a0b5d475c109e7dd5b68a220ce75fdbc9f6 Mon Sep 17 00:00:00 2001 From: Jonah Scheinerman Date: Thu, 24 Oct 2024 12:26:59 -0400 Subject: [PATCH 02/10] Tests --- .../src/components/hotkeys/hotkeyParser.ts | 6 ++-- .../src/components/hotkeys/keyComboTag.tsx | 30 ++++++++++++------- .../core/test/hotkeys/keyComboTagTests.tsx | 15 ++++++++-- 3 files changed, 35 insertions(+), 16 deletions(-) diff --git a/packages/core/src/components/hotkeys/hotkeyParser.ts b/packages/core/src/components/hotkeys/hotkeyParser.ts index e0fdda17d9..9a1bfd401b 100644 --- a/packages/core/src/components/hotkeys/hotkeyParser.ts +++ b/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,7 +232,7 @@ 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; @@ -240,7 +240,7 @@ export const normalizeKeyCombo = (combo: string, platformOverride?: string): str }); }; -export 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); diff --git a/packages/core/src/components/hotkeys/keyComboTag.tsx b/packages/core/src/components/hotkeys/keyComboTag.tsx index 7a0be3cd4e..30016951c5 100644 --- a/packages/core/src/components/hotkeys/keyComboTag.tsx +++ b/packages/core/src/components/hotkeys/keyComboTag.tsx @@ -71,12 +71,17 @@ export interface KeyComboTagProps extends Props { minimal?: boolean; } -export class KeyComboTag extends AbstractPureComponent { +interface KeyComboTagInternalProps extends KeyComboTagProps { + /** Override the oeprating system rendering for internal testing purposes */ + platformOverride?: string; +} + +export class KeyComboTagInternal extends AbstractPureComponent { public static displayName = `${DISPLAYNAME_PREFIX}.KeyComboTag`; public render() { - const { className, combo, minimal } = this.props; - const normalizedKeys = 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((key, index) => @@ -89,7 +94,7 @@ export class KeyComboTag extends AbstractPureComponent { private renderKey = (key: string, index: number) => { const keyString = DISPLAY_ALIASES[key] ?? key; - const icon = getKeyIcon(key); + const icon = this.getKeyIcon(key); const reactKey = `key-${index}`; return ( @@ -100,18 +105,21 @@ export class KeyComboTag extends AbstractPureComponent { }; private renderMinimalKey = (key: string, index: number, isLastKey: boolean) => { - const icon = getKeyIcon(key); + const icon = this.getKeyIcon(key); if (icon == null) { return isLastKey ? key : <>{key} + ; } return ; }; -} -function getKeyIcon(key: string) { - const icon = KEY_ICONS[key]; - if (icon?.isMacOnly && !isMac()) { - return undefined; + private getKeyIcon(key: string) { + const { platformOverride } = this.props; + const icon = KEY_ICONS[key]; + if (icon?.isMacOnly && !isMac(platformOverride)) { + return undefined; + } + return icon; } - return icon; } + +export const KeyComboTag: React.ComponentType = KeyComboTagInternal; diff --git a/packages/core/test/hotkeys/keyComboTagTests.tsx b/packages/core/test/hotkeys/keyComboTagTests.tsx index 28cda3df17..e687e04860 100644 --- a/packages/core/test/hotkeys/keyComboTagTests.tsx +++ b/packages/core/test/hotkeys/keyComboTagTests.tsx @@ -18,11 +18,22 @@ 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(); + render(); expect(screen.getByText("C")).not.to.be.undefined; }); + + describe("should render minimal key combos on Mac using icons", () => { + render(); + expect(() => screen.getByText("cmd + C", { exact: false })).to.throw; + expect(screen.findAllByAltText("Command key")).not.to.be.undefined; + }); + + it("should render minimal key combos on non-Macs using text", () => { + render(); + expect(screen.getByText("ctrl + C", { exact: false }).innerText).to.equal("ctrl + C"); + }); }); From 45fa21e30e0fd3fdb810bcb9db2bb1d45a8b1142 Mon Sep 17 00:00:00 2001 From: svc-changelog Date: Thu, 24 Oct 2024 16:34:28 +0000 Subject: [PATCH 03/10] Add generated changelog entries --- packages/core/changelog/@unreleased/pr-7025.v2.yml | 6 ++++++ packages/docs-app/changelog/@unreleased/pr-7025.v2.yml | 6 ++++++ 2 files changed, 12 insertions(+) create mode 100644 packages/core/changelog/@unreleased/pr-7025.v2.yml create mode 100644 packages/docs-app/changelog/@unreleased/pr-7025.v2.yml diff --git a/packages/core/changelog/@unreleased/pr-7025.v2.yml b/packages/core/changelog/@unreleased/pr-7025.v2.yml new file mode 100644 index 0000000000..e69900530e --- /dev/null +++ b/packages/core/changelog/@unreleased/pr-7025.v2.yml @@ -0,0 +1,6 @@ +type: fix +fix: + description: 'fix(KeyComboTag): No longer render modifier key icons on non-Mac operating + systems' + links: + - https://github.com/palantir/blueprint/pull/7025 diff --git a/packages/docs-app/changelog/@unreleased/pr-7025.v2.yml b/packages/docs-app/changelog/@unreleased/pr-7025.v2.yml new file mode 100644 index 0000000000..e69900530e --- /dev/null +++ b/packages/docs-app/changelog/@unreleased/pr-7025.v2.yml @@ -0,0 +1,6 @@ +type: fix +fix: + description: 'fix(KeyComboTag): No longer render modifier key icons on non-Mac operating + systems' + links: + - https://github.com/palantir/blueprint/pull/7025 From 44eb2e748b803604494a6fbad593f7ed24ab3e8a Mon Sep 17 00:00:00 2001 From: svc-changelog Date: Thu, 24 Oct 2024 16:36:14 +0000 Subject: [PATCH 04/10] Add generated changelog entries --- packages/core/changelog/@unreleased/pr-7025.v2.yml | 4 ++-- packages/docs-app/changelog/@unreleased/pr-7025.v2.yml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/core/changelog/@unreleased/pr-7025.v2.yml b/packages/core/changelog/@unreleased/pr-7025.v2.yml index e69900530e..6d12b686a8 100644 --- a/packages/core/changelog/@unreleased/pr-7025.v2.yml +++ b/packages/core/changelog/@unreleased/pr-7025.v2.yml @@ -1,6 +1,6 @@ type: fix fix: - description: 'fix(KeyComboTag): No longer render modifier key icons on non-Mac operating - systems' + description: The KeyComboTag component no longer renders modifier key icons on non-Mac + operating systems links: - https://github.com/palantir/blueprint/pull/7025 diff --git a/packages/docs-app/changelog/@unreleased/pr-7025.v2.yml b/packages/docs-app/changelog/@unreleased/pr-7025.v2.yml index e69900530e..6d12b686a8 100644 --- a/packages/docs-app/changelog/@unreleased/pr-7025.v2.yml +++ b/packages/docs-app/changelog/@unreleased/pr-7025.v2.yml @@ -1,6 +1,6 @@ type: fix fix: - description: 'fix(KeyComboTag): No longer render modifier key icons on non-Mac operating - systems' + description: The KeyComboTag component no longer renders modifier key icons on non-Mac + operating systems links: - https://github.com/palantir/blueprint/pull/7025 From ebd77282449d7596ac21754408d69b2f9d0551aa Mon Sep 17 00:00:00 2001 From: svc-changelog Date: Thu, 24 Oct 2024 16:37:00 +0000 Subject: [PATCH 05/10] Add generated changelog entries --- packages/docs-app/changelog/@unreleased/pr-7025.v2.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/docs-app/changelog/@unreleased/pr-7025.v2.yml b/packages/docs-app/changelog/@unreleased/pr-7025.v2.yml index 6d12b686a8..9659c11177 100644 --- a/packages/docs-app/changelog/@unreleased/pr-7025.v2.yml +++ b/packages/docs-app/changelog/@unreleased/pr-7025.v2.yml @@ -1,6 +1,6 @@ type: fix fix: - description: The KeyComboTag component no longer renders modifier key icons on non-Mac - operating systems + description: The useHotkeys docuementation now shows minimal and non-minimal states + for the KeyComboTag component. links: - https://github.com/palantir/blueprint/pull/7025 From d5adb4033b8675d519bc34ad9cd1429cc510ddbc Mon Sep 17 00:00:00 2001 From: svc-changelog Date: Thu, 24 Oct 2024 16:37:11 +0000 Subject: [PATCH 06/10] Add generated changelog entries --- packages/docs-app/changelog/@unreleased/pr-7025.v2.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/docs-app/changelog/@unreleased/pr-7025.v2.yml b/packages/docs-app/changelog/@unreleased/pr-7025.v2.yml index 9659c11177..0235cfe800 100644 --- a/packages/docs-app/changelog/@unreleased/pr-7025.v2.yml +++ b/packages/docs-app/changelog/@unreleased/pr-7025.v2.yml @@ -1,6 +1,6 @@ type: fix fix: - description: The useHotkeys docuementation now shows minimal and non-minimal states + description: The useHotkeys documentation now shows minimal and non-minimal states for the KeyComboTag component. links: - https://github.com/palantir/blueprint/pull/7025 From 701f224a0b75de8f2cf16450ac5e8ddd4ada22b8 Mon Sep 17 00:00:00 2001 From: Jonah Scheinerman Date: Thu, 24 Oct 2024 13:07:12 -0400 Subject: [PATCH 07/10] Fixes --- packages/core/src/components/hotkeys/keyComboTag.tsx | 2 +- packages/core/test/hotkeys/keyComboTagTests.tsx | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/core/src/components/hotkeys/keyComboTag.tsx b/packages/core/src/components/hotkeys/keyComboTag.tsx index 30016951c5..67874e6711 100644 --- a/packages/core/src/components/hotkeys/keyComboTag.tsx +++ b/packages/core/src/components/hotkeys/keyComboTag.tsx @@ -107,7 +107,7 @@ export class KeyComboTagInternal extends AbstractPureComponent { const icon = this.getKeyIcon(key); if (icon == null) { - return isLastKey ? key : <>{key} + ; + return isLastKey ? key : {key} + ; } return ; }; diff --git a/packages/core/test/hotkeys/keyComboTagTests.tsx b/packages/core/test/hotkeys/keyComboTagTests.tsx index e687e04860..95fd3ee10a 100644 --- a/packages/core/test/hotkeys/keyComboTagTests.tsx +++ b/packages/core/test/hotkeys/keyComboTagTests.tsx @@ -34,6 +34,9 @@ describe("KeyCombo", () => { it("should render minimal key combos on non-Macs using text", () => { render(); - expect(screen.getByText("ctrl + C", { exact: false }).innerText).to.equal("ctrl + C"); + const text = screen.getByText("ctrl + C", { exact: false }).innerText; + expect(text).to.contain("ctrl"); + expect(text).to.contain("+"); + expect(text).to.contain("C"); }); }); From 672aa418327231825f6a1c1861d8ca59ff49ac7e Mon Sep 17 00:00:00 2001 From: Jonah Scheinerman Date: Thu, 24 Oct 2024 14:51:43 -0400 Subject: [PATCH 08/10] describe -> it --- packages/core/test/hotkeys/keyComboTagTests.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/test/hotkeys/keyComboTagTests.tsx b/packages/core/test/hotkeys/keyComboTagTests.tsx index 95fd3ee10a..8023f57299 100644 --- a/packages/core/test/hotkeys/keyComboTagTests.tsx +++ b/packages/core/test/hotkeys/keyComboTagTests.tsx @@ -26,7 +26,7 @@ describe("KeyCombo", () => { expect(screen.getByText("C")).not.to.be.undefined; }); - describe("should render minimal key combos on Mac using icons", () => { + it("should render minimal key combos on Mac using icons", () => { render(); expect(() => screen.getByText("cmd + C", { exact: false })).to.throw; expect(screen.findAllByAltText("Command key")).not.to.be.undefined; From 70ebd7f19b6c7c18a328a84fa61739a8da07ad8c Mon Sep 17 00:00:00 2001 From: Jonah Scheinerman Date: Thu, 24 Oct 2024 15:31:42 -0400 Subject: [PATCH 09/10] Fix tests for real --- packages/core/test/hotkeys/keyComboTagTests.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/core/test/hotkeys/keyComboTagTests.tsx b/packages/core/test/hotkeys/keyComboTagTests.tsx index 8023f57299..fcef12c134 100644 --- a/packages/core/test/hotkeys/keyComboTagTests.tsx +++ b/packages/core/test/hotkeys/keyComboTagTests.tsx @@ -29,7 +29,6 @@ describe("KeyCombo", () => { it("should render minimal key combos on Mac using icons", () => { render(); expect(() => screen.getByText("cmd + C", { exact: false })).to.throw; - expect(screen.findAllByAltText("Command key")).not.to.be.undefined; }); it("should render minimal key combos on non-Macs using text", () => { From d8d658d973707ea62041267e6bdd737bcd25d4a2 Mon Sep 17 00:00:00 2001 From: Jonah Scheinerman Date: Mon, 28 Oct 2024 10:40:41 -0400 Subject: [PATCH 10/10] Fix spacing --- packages/core/src/components/hotkeys/_hotkeys.scss | 5 ++++- packages/core/src/components/hotkeys/keyComboTag.tsx | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/core/src/components/hotkeys/_hotkeys.scss b/packages/core/src/components/hotkeys/_hotkeys.scss index c031154216..f03e206796 100644 --- a/packages/core/src/components/hotkeys/_hotkeys.scss +++ b/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; } diff --git a/packages/core/src/components/hotkeys/keyComboTag.tsx b/packages/core/src/components/hotkeys/keyComboTag.tsx index 67874e6711..e1a4ee8cd8 100644 --- a/packages/core/src/components/hotkeys/keyComboTag.tsx +++ b/packages/core/src/components/hotkeys/keyComboTag.tsx @@ -89,7 +89,7 @@ export class KeyComboTagInternal extends AbstractPureComponent{keys}; + return {keys}; } private renderKey = (key: string, index: number) => {