镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 69 additions & 0 deletions apps/web/src/components/RightPanelTabs.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,62 @@ describe("surface shortcuts", () => {
const actions = [
{ shortcut: "B", available: true, label: "Browser" },
{ shortcut: "D", available: false, label: "Diff" },
{ shortcut: "T", available: true, label: "Terminal" },
{ shortcut: "M", available: true, label: "Device" },
] as const;

it("falls back to the physical key when the layout types a non-Latin letter or mark", () => {
// Ukrainian layout: the B key types "и".
expect(surfaceShortcutActionForKey(actions, shortcutEvent("\u0438", { code: "KeyB" }))).toBe(
actions[0],
);
// Greek layout: the B key types "β".
expect(surfaceShortcutActionForKey(actions, shortcutEvent("\u03b2", { code: "KeyB" }))).toBe(
actions[0],
);
// Thai layout: the B key types the vowel sign "\u0e34", a combining mark.
expect(surfaceShortcutActionForKey(actions, shortcutEvent("\u0e34", { code: "KeyB" }))).toBe(
actions[0],
);
// Arabic (101) layout: the B key types lam alef "\u0644\u0627", two code points.
expect(
surfaceShortcutActionForKey(actions, shortcutEvent("\u0644\u0627", { code: "KeyB" })),
).toBe(actions[0]);
// Ukrainian layout: the D key types "\u0432", but the Diff surface is unavailable.
expect(
surfaceShortcutActionForKey(actions, shortcutEvent("\u0432", { code: "KeyD" })),
).toBeNull();
});

it("follows the typed letter on Latin layouts that move it", () => {
// Dvorak: the key at QWERTY B types "x", and "b" sits on QWERTY N.
expect(surfaceShortcutActionForKey(actions, shortcutEvent("x", { code: "KeyB" }))).toBeNull();
// The same key with Shift or Caps Lock types "X".
expect(surfaceShortcutActionForKey(actions, shortcutEvent("X", { code: "KeyB" }))).toBeNull();
expect(surfaceShortcutActionForKey(actions, shortcutEvent("b", { code: "KeyN" }))).toBe(
actions[0],
);
});

it("keeps accented Latin letters typed on a letter key", () => {
// Bépo: the T key types "è".
expect(
surfaceShortcutActionForKey(actions, shortcutEvent("\u00e8", { code: "KeyT" })),
).toBeNull();
});

it("keeps punctuation typed on a letter key", () => {
// French AZERTY: the M key types ",".
expect(surfaceShortcutActionForKey(actions, shortcutEvent(",", { code: "KeyM" }))).toBeNull();
});

it("keeps a dead key on a letter key", () => {
// A dead key starts an accented letter on a Latin layout.
expect(
surfaceShortcutActionForKey(actions, shortcutEvent("Dead", { code: "KeyT" })),
).toBeNull();
});

it("leaves modified, composing, and already-handled key events alone", () => {
expect(surfaceShortcutActionForKey(actions, shortcutEvent("b", { metaKey: true }))).toBeNull();
expect(
Expand All @@ -37,6 +91,21 @@ describe("surface shortcuts", () => {
surfaceShortcutActionForKey(actions, shortcutEvent("b", { defaultPrevented: true })),
).toBeNull();
});

it("leaves AltGr text alone unless it is a plain letter", () => {
const getModifierState = (key: string) => key === "AltGraph";
// German layout: AltGr+M types "\u00b5" and sets only AltGraph on Linux and Windows.
expect(
surfaceShortcutActionForKey(
actions,
shortcutEvent("\u00b5", { code: "KeyM", getModifierState }),
),
).toBeNull();
// Finnish layout: AltGr+B types "b", which still matches, as it does in keybindings.ts.
expect(
surfaceShortcutActionForKey(actions, shortcutEvent("b", { code: "KeyB", getModifierState })),
).toBe(actions[0]);
});
});

describe("surface shortcut typing contexts", () => {
Expand Down
9 changes: 5 additions & 4 deletions apps/web/src/components/RightPanelTabs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ import { PierreEntryIcon } from "./chat/PierreEntryIcon";
import { resolvePullRequestState } from "./pullRequest/pullRequestPresentation";
import { PullRequestGlyph } from "~/components/pullRequest/pullRequestIcons";
import { observeResize } from "~/lib/observeResize";
import { surfaceShortcutKey } from "~/lib/surfaceShortcutKey";

export interface RightPanelTabsProps {
mode: PreviewPanelMode;
Expand Down Expand Up @@ -238,17 +239,17 @@ function tabAudioState(overlay: DesktopPreviewOverlay | null): TabAudioState {
type SurfaceShortcutEvent = Pick<
KeyboardEvent,
"altKey" | "ctrlKey" | "defaultPrevented" | "isComposing" | "key" | "metaKey"
>;
> &
Partial<Pick<KeyboardEvent, "code" | "getModifierState">>;

export function surfaceShortcutActionForKey<
const Action extends { available: boolean; shortcut: string },
>(actions: readonly Action[], event: SurfaceShortcutEvent): Action | null {
if (event.defaultPrevented || event.isComposing) return null;
if (event.metaKey || event.ctrlKey || event.altKey) return null;
const key = surfaceShortcutKey(event);
return (
actions.find(
(action) => action.available && action.shortcut.toLowerCase() === event.key.toLowerCase(),
) ?? null
actions.find((action) => action.available && action.shortcut.toLowerCase() === key) ?? null
);
}

Expand Down
12 changes: 8 additions & 4 deletions apps/web/src/components/chat/composerFocusRedirect.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { surfaceShortcutKey } from "~/lib/surfaceShortcutKey";

export const TYPE_TO_FOCUS_EDITABLE_SELECTOR = [
"input",
"textarea",
Expand Down Expand Up @@ -48,11 +50,13 @@ const SURFACE_LAUNCHER_KEYS_ATTRIBUTE = "data-surface-launcher-keys";
// The right-panel launcher registers its own window capture listener when it
// mounts. When the panel opens after page load that listener runs after ours,
// so we must leave its letters alone or the composer takes them first.
function isSurfaceLauncherKey(key: string): boolean {
function isSurfaceLauncherKey(event: KeyboardEvent): boolean {
const launcherKeys = document
.querySelector(`[${SURFACE_LAUNCHER_KEYS_ATTRIBUTE}]`)
?.getAttribute(SURFACE_LAUNCHER_KEYS_ATTRIBUTE);
return launcherKeys?.toLowerCase().includes(key.toLowerCase()) ?? false;
?.getAttribute(SURFACE_LAUNCHER_KEYS_ATTRIBUTE)
?.toLowerCase();
const key = surfaceShortcutKey(event);
return key !== null && (launcherKeys?.includes(key) ?? false);
}

export function shouldRedirectInputToComposer(event: Event): boolean {
Expand All @@ -70,7 +74,7 @@ export function shouldTypeToFocusComposer(event: KeyboardEvent): boolean {
if (event.metaKey || event.ctrlKey || event.altKey) return false;
if (event.key.length !== 1) return false;
if (!shouldRedirectInputToComposer(event)) return false;
if (isSurfaceLauncherKey(event.key)) return false;
if (isSurfaceLauncherKey(event)) return false;

return true;
}
Expand Down
40 changes: 39 additions & 1 deletion apps/web/src/components/chat/useChatShortcuts.browser.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { useEffect } from "react";
import { expect, it } from "vite-plus/test";
import { userEvent } from "vite-plus/test/browser";

import { surfaceShortcutKey } from "~/lib/surfaceShortcutKey";
import { render } from "../../../test/render";
import type { ChatComposerHandle } from "./ChatComposer";
import { useChatShortcuts } from "./useChatShortcuts";
Expand All @@ -25,7 +26,8 @@ function Launcher(props: { keys: string; received: string[] }) {
useEffect(() => {
const claim = (event: KeyboardEvent) => {
if (event.defaultPrevented) return;
if (!props.keys.toLowerCase().split("").includes(event.key.toLowerCase())) return;
const key = surfaceShortcutKey(event);
if (key === null || !props.keys.toLowerCase().includes(key)) return;
event.preventDefault();
props.received.push(event.key);
};
Expand Down Expand Up @@ -94,3 +96,39 @@ it("still types other letters into the composer while the launcher is open", asy
expect(composerKeys).toEqual(["a"]);
expect(received).toEqual([]);
});

it("leaves a non-Latin launcher letter to the launcher by its physical key", async () => {
const composerKeys: string[] = [];
const received: string[] = [];
await using screen = await render(<Chat composerKeys={composerKeys} />, options);
await screen.rerender(<Chat composerKeys={composerKeys} launcher={{ keys: "BT", received }} />);

// Playwright cannot press a key its US layout lacks, so send what a Ukrainian T key sends.
const event = new KeyboardEvent("keydown", {
key: "\u0435",
code: "KeyT",
bubbles: true,
cancelable: true,
});
(document.activeElement ?? document.body).dispatchEvent(event);

expect(composerKeys).toEqual([]);
expect(received).toEqual(["\u0435"]);
});

it.each([
{ key: "b", code: "KeyN", composer: [], launcher: ["b"] },
{ key: "x", code: "KeyB", composer: ["x"], launcher: [] },
{ key: "è", code: "KeyT", composer: ["è"], launcher: [] },
])("routes $key on $code by the typed letter", async ({ key, code, composer, launcher }) => {
const composerKeys: string[] = [];
const received: string[] = [];
await using screen = await render(<Chat composerKeys={composerKeys} />, options);
await screen.rerender(<Chat composerKeys={composerKeys} launcher={{ keys: "BT", received }} />);

const event = new KeyboardEvent("keydown", { key, code, bubbles: true, cancelable: true });
(document.activeElement ?? document.body).dispatchEvent(event);

expect(composerKeys).toEqual(composer);
expect(received).toEqual(launcher);
});
14 changes: 14 additions & 0 deletions apps/web/src/lib/surfaceShortcutKey.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
type SurfaceKeyEvent = Pick<KeyboardEvent, "key"> &
Partial<Pick<KeyboardEvent, "code" | "getModifierState">>;

export function surfaceShortcutKey(event: SurfaceKeyEvent): string | null {
// AltGr text must stay text even when the browser omits Ctrl and Alt.
if (event.getModifierState?.("AltGraph") && !/^[a-z0-9]$/i.test(event.key)) return null;
const typed = event.key.toLowerCase();
// Preserve Latin layouts, punctuation and dead keys; non-Latin letters/marks use the physical key.
const physical =
/^[\p{L}\p{M}]+$/u.test(typed) && !/\p{Script=Latin}/u.test(typed)
? event.code?.match(/^Key([A-Z])$/)?.[1]?.toLowerCase()
: undefined;
return physical ?? typed;
}
Loading