mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 04:38:33 +00:00
fix(studio): use e.key for playback shortcuts so non-QWERTY layouts work
The 7 letter shortcuts (J/K/L/I/O/A/E) in usePlaybackKeyboard were gated on `e.code === "Key*"`, which is the physical key position on a US-QWERTY layout. On AZERTY (and other layouts) the physical "KeyA" slot produces e.key="q", so "Jump to in-point" and the rest of the letter shortcuts either fired on the wrong character or not at all. Switch the 7 letter shortcuts to compare `e.key.toLowerCase()` and rename `pressedCodesRef` → `pressedKeysRef` so the K-hold combo (K+J / K+L for frame stepping) is also keyed off the typed character. `Space` and `Arrow*` keep using `e.code` since those codes are layout-independent. Adds a happy-dom test covering QWERTY happy path, AZERTY (physical KeyQ produces e.key="a" → in-point seek fires), AZERTY contrapositive (physical KeyA producing e.key="q" no longer triggers in-point), Shift+I clears in-point, K-hold combo for frame stepping, K release returning the set to clean state, and Space passthrough. Addresses bug #3 in #834. Bugs #1 (loop at out-point) and #2 (Jump to in-point forcing pause) live outside this hook (player loop and adapter `seek` respectively) and are left for follow-up PRs.
This commit is contained in:
@@ -0,0 +1,174 @@
|
||||
// @vitest-environment happy-dom
|
||||
|
||||
import React, { act, useEffect } from "react";
|
||||
import { createRoot } from "react-dom/client";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { usePlaybackKeyboard } from "./usePlaybackKeyboard";
|
||||
import { usePlayerStore } from "../store/playerStore";
|
||||
|
||||
globalThis.IS_REACT_ACT_ENVIRONMENT = true;
|
||||
|
||||
afterEach(() => {
|
||||
document.body.innerHTML = "";
|
||||
usePlayerStore.getState().reset();
|
||||
});
|
||||
|
||||
interface Spies {
|
||||
seek: ReturnType<typeof vi.fn>;
|
||||
play: ReturnType<typeof vi.fn>;
|
||||
playBackward: ReturnType<typeof vi.fn>;
|
||||
pause: ReturnType<typeof vi.fn>;
|
||||
}
|
||||
|
||||
interface HookHandle {
|
||||
dispatch: (event: KeyboardEvent) => void;
|
||||
release: (event: KeyboardEvent) => void;
|
||||
spies: Spies;
|
||||
}
|
||||
|
||||
function setupHook(): HookHandle {
|
||||
const spies: Spies = {
|
||||
seek: vi.fn(),
|
||||
play: vi.fn(),
|
||||
playBackward: vi.fn(),
|
||||
pause: vi.fn(),
|
||||
};
|
||||
|
||||
let captured: ReturnType<typeof usePlaybackKeyboard> | null = null;
|
||||
|
||||
function Harness() {
|
||||
const iframeRef = React.useRef<HTMLIFrameElement | null>(null);
|
||||
const shuttleDirectionRef = React.useRef<"forward" | "backward" | null>(null);
|
||||
const shuttleSpeedIndexRef = React.useRef(0);
|
||||
const iframeShortcutCleanupRef = React.useRef<(() => void) | null>(null);
|
||||
const result = usePlaybackKeyboard({
|
||||
iframeRef,
|
||||
shuttleDirectionRef,
|
||||
shuttleSpeedIndexRef,
|
||||
iframeShortcutCleanupRef,
|
||||
getAdapter: () => null,
|
||||
...spies,
|
||||
});
|
||||
useEffect(() => {
|
||||
captured = result;
|
||||
});
|
||||
return null;
|
||||
}
|
||||
|
||||
const host = document.createElement("div");
|
||||
document.body.append(host);
|
||||
const root = createRoot(host);
|
||||
act(() => {
|
||||
root.render(React.createElement(Harness));
|
||||
});
|
||||
|
||||
if (!captured) throw new Error("usePlaybackKeyboard harness did not capture handlers");
|
||||
|
||||
return {
|
||||
dispatch: (event) => captured!.playbackKeyDownRef.current(event),
|
||||
release: (event) => captured!.playbackKeyUpRef.current(event),
|
||||
spies,
|
||||
};
|
||||
}
|
||||
|
||||
function keydown(init: { code: string; key: string; shiftKey?: boolean }): KeyboardEvent {
|
||||
return new KeyboardEvent("keydown", {
|
||||
code: init.code,
|
||||
key: init.key,
|
||||
shiftKey: init.shiftKey ?? false,
|
||||
cancelable: true,
|
||||
});
|
||||
}
|
||||
|
||||
function keyup(init: { code: string; key: string }): KeyboardEvent {
|
||||
return new KeyboardEvent("keyup", { code: init.code, key: init.key });
|
||||
}
|
||||
|
||||
describe("usePlaybackKeyboard — keyboard layout independence (#834)", () => {
|
||||
it("'Jump to in-point' fires on physical KeyA in a QWERTY layout", () => {
|
||||
const { dispatch, spies } = setupHook();
|
||||
usePlayerStore.setState({ inPoint: 1.5 });
|
||||
|
||||
act(() => {
|
||||
dispatch(keydown({ code: "KeyA", key: "a" }));
|
||||
});
|
||||
|
||||
expect(spies.seek).toHaveBeenCalledWith(1.5);
|
||||
});
|
||||
|
||||
it("'Jump to in-point' fires on AZERTY (physical KeyQ produces e.key='a')", () => {
|
||||
const { dispatch, spies } = setupHook();
|
||||
usePlayerStore.setState({ inPoint: 2.5 });
|
||||
|
||||
act(() => {
|
||||
dispatch(keydown({ code: "KeyQ", key: "a" }));
|
||||
});
|
||||
|
||||
expect(spies.seek).toHaveBeenCalledWith(2.5);
|
||||
});
|
||||
|
||||
it("AZERTY 'A' physical key (e.key='q') no longer triggers in-point seek", () => {
|
||||
const { dispatch, spies } = setupHook();
|
||||
usePlayerStore.setState({ inPoint: 4.0 });
|
||||
|
||||
act(() => {
|
||||
dispatch(keydown({ code: "KeyA", key: "q" }));
|
||||
});
|
||||
|
||||
expect(spies.seek).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("Shift+I clears the in-point (e.key='I' is matched after lowercasing)", () => {
|
||||
const { dispatch } = setupHook();
|
||||
usePlayerStore.setState({ inPoint: 3.0 });
|
||||
|
||||
act(() => {
|
||||
dispatch(keydown({ code: "KeyI", key: "I", shiftKey: true }));
|
||||
});
|
||||
|
||||
expect(usePlayerStore.getState().inPoint).toBeNull();
|
||||
});
|
||||
|
||||
it("K-held + L steps forward one frame (combo uses character, not physical position)", () => {
|
||||
const { dispatch, spies } = setupHook();
|
||||
usePlayerStore.setState({ currentTime: 0 });
|
||||
|
||||
act(() => {
|
||||
dispatch(keydown({ code: "KeyK", key: "k" }));
|
||||
});
|
||||
act(() => {
|
||||
dispatch(keydown({ code: "KeyL", key: "l" }));
|
||||
});
|
||||
|
||||
expect(spies.seek).toHaveBeenCalledTimes(1);
|
||||
expect(spies.play).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("releasing K removes it from the pressed set so subsequent L resumes forward shuttle", () => {
|
||||
const { dispatch, release, spies } = setupHook();
|
||||
|
||||
act(() => {
|
||||
dispatch(keydown({ code: "KeyK", key: "k" }));
|
||||
});
|
||||
act(() => {
|
||||
release(keyup({ code: "KeyK", key: "k" }));
|
||||
});
|
||||
act(() => {
|
||||
dispatch(keydown({ code: "KeyL", key: "l" }));
|
||||
});
|
||||
|
||||
expect(spies.play).toHaveBeenCalledTimes(1);
|
||||
expect(spies.seek).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("Space (universal e.code) still toggles play", () => {
|
||||
const { dispatch, spies } = setupHook();
|
||||
usePlayerStore.setState({ isPlaying: false });
|
||||
|
||||
act(() => {
|
||||
dispatch(keydown({ code: "Space", key: " " }));
|
||||
});
|
||||
|
||||
expect(spies.play).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
@@ -36,7 +36,7 @@ export function usePlaybackKeyboard({
|
||||
pause,
|
||||
seek,
|
||||
}: UsePlaybackKeyboardParams) {
|
||||
const pressedCodesRef = useRef(new Set<string>());
|
||||
const pressedKeysRef = useRef(new Set<string>());
|
||||
const playbackKeyDownRef = useRef<(e: KeyboardEvent) => void>(() => {});
|
||||
const playbackKeyUpRef = useRef<(e: KeyboardEvent) => void>(() => {});
|
||||
|
||||
@@ -90,7 +90,8 @@ export function usePlaybackKeyboard({
|
||||
) {
|
||||
return;
|
||||
}
|
||||
pressedCodesRef.current.add(e.code);
|
||||
const key = e.key.toLowerCase();
|
||||
pressedKeysRef.current.add(key);
|
||||
if (e.code === "Space") {
|
||||
e.preventDefault();
|
||||
togglePlay();
|
||||
@@ -107,47 +108,47 @@ export function usePlaybackKeyboard({
|
||||
return;
|
||||
}
|
||||
if (e.repeat) return;
|
||||
if (e.code === "KeyK") {
|
||||
if (key === "k") {
|
||||
e.preventDefault();
|
||||
pause();
|
||||
return;
|
||||
}
|
||||
if (e.code === "KeyJ") {
|
||||
if (key === "j") {
|
||||
e.preventDefault();
|
||||
if (pressedCodesRef.current.has("KeyK")) {
|
||||
if (pressedKeysRef.current.has("k")) {
|
||||
stepFrames(-1);
|
||||
return;
|
||||
}
|
||||
shuttle("backward");
|
||||
return;
|
||||
}
|
||||
if (e.code === "KeyL") {
|
||||
if (key === "l") {
|
||||
e.preventDefault();
|
||||
if (pressedCodesRef.current.has("KeyK")) {
|
||||
if (pressedKeysRef.current.has("k")) {
|
||||
stepFrames(1);
|
||||
return;
|
||||
}
|
||||
shuttle("forward");
|
||||
return;
|
||||
}
|
||||
if (e.code === "KeyI") {
|
||||
if (key === "i") {
|
||||
e.preventDefault();
|
||||
const t = getAdapter()?.getTime() ?? usePlayerStore.getState().currentTime;
|
||||
usePlayerStore.getState().setInPoint(e.shiftKey ? null : t);
|
||||
return;
|
||||
}
|
||||
if (e.code === "KeyO") {
|
||||
if (key === "o") {
|
||||
e.preventDefault();
|
||||
const t = getAdapter()?.getTime() ?? usePlayerStore.getState().currentTime;
|
||||
usePlayerStore.getState().setOutPoint(e.shiftKey ? null : t);
|
||||
return;
|
||||
}
|
||||
if (e.code === "KeyA") {
|
||||
if (key === "a") {
|
||||
e.preventDefault();
|
||||
seek(usePlayerStore.getState().inPoint ?? 0);
|
||||
return;
|
||||
}
|
||||
if (e.code === "KeyE") {
|
||||
if (key === "e") {
|
||||
e.preventDefault();
|
||||
const { outPoint } = usePlayerStore.getState();
|
||||
seek(outPoint ?? getAdapter()?.getDuration() ?? usePlayerStore.getState().duration);
|
||||
@@ -158,7 +159,7 @@ export function usePlaybackKeyboard({
|
||||
);
|
||||
|
||||
const handlePlaybackKeyUp = useCallback((e: KeyboardEvent) => {
|
||||
pressedCodesRef.current.delete(e.code);
|
||||
pressedKeysRef.current.delete(e.key.toLowerCase());
|
||||
}, []);
|
||||
|
||||
playbackKeyDownRef.current = handlePlaybackKeyDown;
|
||||
|
||||
Reference in New Issue
Block a user