From de51ff52123e90a07617d0d73d9f1553a1057484 Mon Sep 17 00:00:00 2001 From: Jeremy lewi Date: Wed, 13 May 2026 06:58:20 -0700 Subject: [PATCH 1/3] Fix App Console history shortcuts Signed-off-by: Jeremy lewi --- .../components/AppConsole/AppConsole.test.tsx | 72 ++++++++++++++++--- app/src/components/AppConsole/AppConsole.tsx | 63 ++++++++++++---- 2 files changed, 111 insertions(+), 24 deletions(-) diff --git a/app/src/components/AppConsole/AppConsole.test.tsx b/app/src/components/AppConsole/AppConsole.test.tsx index c6138d2..7dfd352 100644 --- a/app/src/components/AppConsole/AppConsole.test.tsx +++ b/app/src/components/AppConsole/AppConsole.test.tsx @@ -3,12 +3,15 @@ import { act, fireEvent, render, screen, waitFor } from "@testing-library/react" import { beforeEach, describe, expect, it, vi } from "vitest"; import { useEffect, useMemo, useRef } from "react"; -import type { ConsoleCell, PersistedConsoleCellRow } from "./model"; +import type { PersistedConsoleCellRow } from "./model"; +const ALT = 1 << 9; const SHIFT = 1 << 10; const ENTER = 1; const UP = 2; const DOWN = 3; +const KEY_N = 4; +const KEY_P = 5; let storedSessionId = "session-1"; let storedCells: PersistedConsoleCellRow[] = []; @@ -166,6 +169,7 @@ vi.mock("../Actions/Editor", () => ({ onMount?: (editor: any, monaco: any) => void; }) => { const commandsRef = useRef(new Map void>()); + const editorRef = useMemo( () => ({ addCommand: (key: number, handler: () => void) => { @@ -178,11 +182,13 @@ vi.mock("../Actions/Editor", () => ({ useEffect(() => { onMount?.(editorRef, { - KeyMod: { Shift: SHIFT }, + KeyMod: { Alt: ALT, Shift: SHIFT }, KeyCode: { Enter: ENTER, UpArrow: UP, DownArrow: DOWN, + KeyN: KEY_N, + KeyP: KEY_P, }, }); }, [editorRef, onMount]); @@ -195,9 +201,6 @@ vi.mock("../Actions/Editor", () => ({ value={value} onChange={(event) => onChange(event.currentTarget.value)} onKeyDown={(event) => { - if (!event.shiftKey) { - return; - } const code = event.key === "Enter" ? ENTER @@ -205,12 +208,19 @@ vi.mock("../Actions/Editor", () => ({ ? UP : event.key === "ArrowDown" ? DOWN + : event.key.toLowerCase() === "n" + ? KEY_N + : event.key.toLowerCase() === "p" + ? KEY_P : null; - if (!code) { + const modifier = event.shiftKey ? SHIFT : event.altKey ? ALT : null; + const handler = + code && modifier !== null ? commandsRef.current.get(modifier | code) : undefined; + if (!handler) { return; } event.preventDefault(); - commandsRef.current.get(SHIFT | code)?.(); + handler(); }} /> ); @@ -293,16 +303,56 @@ describe("AppConsole", () => { fireEvent.change(currentInput(), { target: { value: "partial draft" } }); - fireEvent.keyDown(currentInput(), { key: "ArrowUp", shiftKey: true }); + fireEvent.click(screen.getByTestId("app-console-history-previous")); + expect(currentInput().value).toBe("second()"); + + fireEvent.click(screen.getByTestId("app-console-history-previous")); + expect(currentInput().value).toBe("first()"); + + fireEvent.click(screen.getByTestId("app-console-history-next")); + expect(currentInput().value).toBe("second()"); + + fireEvent.click(screen.getByTestId("app-console-history-next")); + expect(currentInput().value).toBe("partial draft"); + }); + + it("renders history controls with tooltips", async () => { + render(); + + await screen.findByLabelText("App Console input"); + expect(screen.getByTestId("app-console-history-previous").getAttribute("title")).toBe( + "Previous history entry (Alt+P)", + ); + expect(screen.getByTestId("app-console-history-next").getAttribute("title")).toBe( + "Next history entry (Alt+N)", + ); + }); + + it("supports Alt+P and Alt+N history shortcuts", async () => { + render(); + + await screen.findByLabelText("App Console input"); + + fireEvent.change(currentInput(), { target: { value: "first()" } }); + fireEvent.keyDown(currentInput(), { key: "Enter", shiftKey: true }); + await waitFor(() => expect(screen.getAllByTestId("app-console-cell")).toHaveLength(2)); + + fireEvent.change(currentInput(), { target: { value: "second()" } }); + fireEvent.keyDown(currentInput(), { key: "Enter", shiftKey: true }); + await waitFor(() => expect(screen.getAllByTestId("app-console-cell")).toHaveLength(3)); + + fireEvent.change(currentInput(), { target: { value: "partial draft" } }); + + fireEvent.keyDown(currentInput(), { key: "p", altKey: true }); expect(currentInput().value).toBe("second()"); - fireEvent.keyDown(currentInput(), { key: "ArrowUp", shiftKey: true }); + fireEvent.keyDown(currentInput(), { key: "p", altKey: true }); expect(currentInput().value).toBe("first()"); - fireEvent.keyDown(currentInput(), { key: "ArrowDown", shiftKey: true }); + fireEvent.keyDown(currentInput(), { key: "n", altKey: true }); expect(currentInput().value).toBe("second()"); - fireEvent.keyDown(currentInput(), { key: "ArrowDown", shiftKey: true }); + fireEvent.keyDown(currentInput(), { key: "n", altKey: true }); expect(currentInput().value).toBe("partial draft"); }); diff --git a/app/src/components/AppConsole/AppConsole.tsx b/app/src/components/AppConsole/AppConsole.tsx index 166fb52..7440944 100644 --- a/app/src/components/AppConsole/AppConsole.tsx +++ b/app/src/components/AppConsole/AppConsole.tsx @@ -247,6 +247,13 @@ export default function AppConsole({ showHeader = true }: { showHeader?: boolean ); const currentCell = cells[cells.length - 1] ?? null; + const historySources = useMemo(() => getHistorySources(cells), [cells]); + const historyIndex = historyBrowseRef.current.index; + const canBrowsePrevious = + currentCell?.status === "draft" && + historySources.length > 0 && + (historyIndex === null || historyIndex < historySources.length - 1); + const canBrowseNext = currentCell?.status === "draft" && historyIndex !== null; useEffect(() => { if (!currentCell || currentCell.status !== "draft") { @@ -459,13 +466,13 @@ export default function AppConsole({ showHeader = true }: { showHeader?: boolean }, ); editor.addCommand( - monaco.KeyMod.Shift | monaco.KeyCode.UpArrow, + monaco.KeyMod.Alt | monaco.KeyCode.KeyP, () => { browseHistory("previous"); }, ); editor.addCommand( - monaco.KeyMod.Shift | monaco.KeyCode.DownArrow, + monaco.KeyMod.Alt | monaco.KeyCode.KeyN, () => { browseHistory("next"); }, @@ -564,16 +571,46 @@ export default function AppConsole({ showHeader = true }: { showHeader?: boolean ) : null} {isEditable ? ( - + <> + + + + ) : null} @@ -620,7 +657,7 @@ export default function AppConsole({ showHeader = true }: { showHeader?: boolean {isCurrent && cell.status === "draft" ? (
Shortcuts:{" "} - Shift+Enter to run, Shift+Up/Shift+Down to browse history. + Shift+Enter to run. Alt+P/Alt+N browse history.
) : null} From 99b31f9ebf69183d0c8d13a2fa548a2ea7b4a52b Mon Sep 17 00:00:00 2001 From: Jeremy lewi Date: Wed, 13 May 2026 08:14:51 -0700 Subject: [PATCH 2/3] Fix App Console history button state Signed-off-by: Jeremy lewi --- .../components/AppConsole/AppConsole.test.tsx | 24 +++++++++++++++ app/src/components/AppConsole/AppConsole.tsx | 29 +++++++++++-------- 2 files changed, 41 insertions(+), 12 deletions(-) diff --git a/app/src/components/AppConsole/AppConsole.test.tsx b/app/src/components/AppConsole/AppConsole.test.tsx index 7dfd352..0513ad1 100644 --- a/app/src/components/AppConsole/AppConsole.test.tsx +++ b/app/src/components/AppConsole/AppConsole.test.tsx @@ -356,6 +356,30 @@ describe("AppConsole", () => { expect(currentInput().value).toBe("partial draft"); }); + it("updates history button state even when browsing does not change the draft text", async () => { + render(); + + await screen.findByLabelText("App Console input"); + + fireEvent.change(currentInput(), { target: { value: "first()" } }); + fireEvent.keyDown(currentInput(), { key: "Enter", shiftKey: true }); + await waitFor(() => expect(screen.getAllByTestId("app-console-cell")).toHaveLength(2)); + + fireEvent.change(currentInput(), { target: { value: "first()" } }); + + const previousButton = screen.getByTestId("app-console-history-previous"); + const nextButton = screen.getByTestId("app-console-history-next"); + + expect(previousButton.getAttribute("disabled")).toBeNull(); + expect(nextButton.getAttribute("disabled")).not.toBeNull(); + + fireEvent.click(previousButton); + + expect(currentInput().value).toBe("first()"); + expect(previousButton.getAttribute("disabled")).not.toBeNull(); + expect(nextButton.getAttribute("disabled")).toBeNull(); + }); + it("copies a frozen cell back into the current draft", async () => { render(); diff --git a/app/src/components/AppConsole/AppConsole.tsx b/app/src/components/AppConsole/AppConsole.tsx index 7440944..04d8672 100644 --- a/app/src/components/AppConsole/AppConsole.tsx +++ b/app/src/components/AppConsole/AppConsole.tsx @@ -150,7 +150,10 @@ export default function AppConsole({ showHeader = true }: { showHeader?: boolean const draftEditorRef = useRef(null); const bodyRef = useRef(null); - const historyBrowseRef = useRef<{ index: number | null; draftBuffer: string }>({ + const [historyBrowseState, setHistoryBrowseState] = useState<{ + index: number | null; + draftBuffer: string; + }>({ index: null, draftBuffer: "", }); @@ -248,7 +251,7 @@ export default function AppConsole({ showHeader = true }: { showHeader?: boolean const currentCell = cells[cells.length - 1] ?? null; const historySources = useMemo(() => getHistorySources(cells), [cells]); - const historyIndex = historyBrowseRef.current.index; + const historyIndex = historyBrowseState.index; const canBrowsePrevious = currentCell?.status === "draft" && historySources.length > 0 && @@ -289,10 +292,10 @@ export default function AppConsole({ showHeader = true }: { showHeader?: boolean appConsoleData.setDraftSource(source); if (clearHistoryBrowse) { - historyBrowseRef.current = { + setHistoryBrowseState({ index: null, draftBuffer: "", - }; + }); } }, [appConsoleData], @@ -311,16 +314,16 @@ export default function AppConsole({ showHeader = true }: { showHeader?: boolean return; } - const state = historyBrowseRef.current; + const state = historyBrowseState; if (direction === "previous") { const nextIndex = state.index === null ? 0 : Math.min(state.index + 1, history.length - 1); const draftBuffer = state.index === null ? draft.source : state.draftBuffer; const nextSource = history[history.length - 1 - nextIndex] ?? draft.source; - historyBrowseRef.current = { + setHistoryBrowseState({ index: nextIndex, draftBuffer, - }; + }); if (nextSource === draft.source) { return; } @@ -338,7 +341,7 @@ export default function AppConsole({ showHeader = true }: { showHeader?: boolean ? history[history.length - 1 - nextIndex] ?? draft.source : state.draftBuffer; - historyBrowseRef.current = + setHistoryBrowseState( nextIndex >= 0 ? { index: nextIndex, @@ -347,7 +350,8 @@ export default function AppConsole({ showHeader = true }: { showHeader?: boolean : { index: null, draftBuffer: "", - }; + }, + ); if (nextSource === draft.source) { return; @@ -355,7 +359,7 @@ export default function AppConsole({ showHeader = true }: { showHeader?: boolean appConsoleData.setDraftSource(nextSource); }, - [appConsoleData], + [appConsoleData, historyBrowseState], ); const executeCurrentCell = useCallback(async () => { @@ -366,10 +370,10 @@ export default function AppConsole({ showHeader = true }: { showHeader?: boolean return; } - historyBrowseRef.current = { + setHistoryBrowseState({ index: null, draftBuffer: "", - }; + }); const globals = createAppJsGlobals({ runme, @@ -449,6 +453,7 @@ export default function AppConsole({ showHeader = true }: { showHeader?: boolean runme, setCurrentDoc, setDefaultRunner, + setHistoryBrowseState, updateRunner, ]); From 56c357de2943003a56639e8acacd1f2595dec3a1 Mon Sep 17 00:00:00 2001 From: Jeremy lewi Date: Wed, 13 May 2026 09:46:14 -0700 Subject: [PATCH 3/3] Fix App Console keyboard history state Signed-off-by: Jeremy lewi --- .../components/AppConsole/AppConsole.test.tsx | 5 +++- app/src/components/AppConsole/AppConsole.tsx | 25 +++++++++++++------ 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/app/src/components/AppConsole/AppConsole.test.tsx b/app/src/components/AppConsole/AppConsole.test.tsx index 0513ad1..33f8d0e 100644 --- a/app/src/components/AppConsole/AppConsole.test.tsx +++ b/app/src/components/AppConsole/AppConsole.test.tsx @@ -191,7 +191,10 @@ vi.mock("../Actions/Editor", () => ({ KeyP: KEY_P, }, }); - }, [editorRef, onMount]); + // Monaco only invokes the consumer mount callback once per editor instance. + // Keep the mock aligned so closure-related regressions are exercised. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); return (