fix: use focused commit for shortcuts on commit pages

resources/js/components/commit-card.tsx

@@ -254,6 +254,7 @@ export function CommitCard({
             )}
             tabIndex={-1}
             data-commit={commit.id}
+            data-focused-commit={isFocused || undefined}
             data-permalink={wasDeleted ? undefined : permalink.url}
             data-test="commit"
             onClick={openClickedCommit}

resources/js/hooks/use-keyboard-shortcuts.ts

@@ -5,6 +5,7 @@ import LogController from '@/actions/App/Http/Controllers/LogController';
 import NotificationController from '@/actions/App/Http/Controllers/NotificationController';
 import StarredCommitController from '@/actions/App/Http/Controllers/StarredCommitController';
 import { signIn } from '@/lib/auth';
+import { commitActionForShortcut } from '@/lib/commit-shortcuts';
 import { hasModKey } from '@/lib/keyboard';
 import { show } from '@/routes/user';
 import { edit } from '@/routes/user-profile';
@@ -50,14 +51,14 @@ export function focusCommit(commit: HTMLElement): void {
  *
  * A deleted commit's actions are inert, which stops pointers and focus but not a scripted click.
  */
-function focusedCommitAction(key: string): HTMLButtonElement | null {
-    const action = document.activeElement
-        ?.closest('[data-reply], [data-commit]')
-        ?.querySelector<HTMLButtonElement>(
-            `button[aria-keyshortcuts="${CSS.escape(key)}"]`,
-        );
-
-    return action && !action.closest('[inert]') ? action : null;
+function focusedCommitAction(
+    key: string,
+): HTMLButtonElement | HTMLTextAreaElement | null {
+    return commitActionForShortcut(
+        key,
+        document.activeElement?.closest('[data-reply], [data-commit]') ?? null,
+        key === 'r' ? document.querySelector('[data-focused-commit]') : null,
+    );
 }
 
 function openFocusedCommit(): boolean {
@@ -158,7 +159,12 @@ export function useKeyboardShortcuts(
 
             if (commitAction) {
                 event.preventDefault();
-                commitAction.click();
+
+                if (commitAction.tagName === 'TEXTAREA') {
+                    commitAction.focus();
+                } else {
+                    commitAction.click();
+                }
 
                 return;
             }

resources/js/lib/commit-shortcuts.ts

@@ -0,0 +1,36 @@
+/**
+ * Finds the active reply or commit's keyboard action. Only reply falls back to the
+ * main commit on a detail page when keyboard focus is elsewhere.
+ */
+export function commitActionForShortcut(
+    key: string,
+    activeTarget: Element | null,
+    pageCommit: Element | null,
+): HTMLButtonElement | HTMLTextAreaElement | null {
+    const target = activeTarget ?? (key === 'r' ? pageCommit : null);
+    const action = target?.querySelector<HTMLButtonElement>(
+        `button[aria-keyshortcuts="${CSS.escape(key)}"]`,
+    );
+
+    if (action && !action.closest('[inert]')) {
+        return action;
+    }
+
+    if (key !== 'r' || !target) {
+        return null;
+    }
+
+    // A reply button loses its shortcut while its composer is open.
+    // Only focus the composer belonging to this commit or reply, not a nested one.
+    return (
+        Array.from(
+            target.querySelectorAll<HTMLTextAreaElement>(
+                '[data-test="reply-composer"] textarea',
+            ),
+        ).find(
+            (textarea) =>
+                textarea.closest('[data-reply], [data-commit]') === target &&
+                !textarea.closest('[inert]'),
+        ) ?? null
+    );
+}

tests/Unit/CommitPageShortcuts.test.ts

@@ -0,0 +1,127 @@
+import assert from 'node:assert/strict';
+import { test } from 'node:test';
+import { commitActionForShortcut } from '../../resources/js/lib/commit-shortcuts.ts';
+
+Object.defineProperty(globalThis, 'CSS', {
+    configurable: true,
+    value: { escape: (value: string) => value },
+});
+
+function card(
+    name: string,
+    keys: string[],
+    clicks: string[],
+    inert = false,
+    focuses?: string[],
+): Element {
+    const element = {
+        querySelector: (selector: string) => {
+            const key = selector.match(/^button\[aria-keyshortcuts="(.+)"\]$/)?.[1];
+
+            return key && keys.includes(key)
+                ? {
+                      click: () => clicks.push(`${name}:${key}`),
+                      closest: () => (inert ? {} : null),
+                  }
+                : null;
+        },
+        querySelectorAll: (selector: string) =>
+            selector === '[data-test="reply-composer"] textarea' && focuses
+                ? [
+                      {
+                          tagName: 'TEXTAREA',
+                          focus: () => focuses.push(name),
+                          closest: (selector: string) =>
+                              selector === '[data-reply], [data-commit]'
+                                  ? element
+                                  : inert
+                                    ? {}
+                                    : null,
+                      },
+                  ]
+                : [],
+    };
+
+    return element as unknown as Element;
+}
+
+function press(key: string, active: Element | null, page: Element | null): boolean {
+    const action = commitActionForShortcut(key, active, page);
+    if (action?.tagName === 'TEXTAREA') {
+        action.focus();
+    } else {
+        action?.click();
+    }
+
+    return action !== null;
+}
+
+test('reply uses the main commit when focus is outside any commit', () => {
+    const clicks: string[] = [];
+
+    assert.equal(press('r', null, card('main', ['r'], clicks)), true);
+    assert.deepEqual(clicks, ['main:r']);
+});
+
+test('reply prefers the commit containing keyboard focus', () => {
+    const clicks: string[] = [];
+
+    assert.equal(
+        press('r', card('reply', ['r'], clicks), card('main', ['r'], clicks)),
+        true,
+    );
+    assert.deepEqual(clicks, ['reply:r']);
+});
+
+test('quote does not use the detail-page fallback', () => {
+    const clicks: string[] = [];
+
+    assert.equal(press('q', null, card('main', ['q'], clicks)), false);
+    assert.deepEqual(clicks, []);
+});
+
+test('quote still works when a commit has keyboard focus', () => {
+    const clicks: string[] = [];
+
+    assert.equal(press('q', card('selected', ['q'], clicks), null), true);
+    assert.deepEqual(clicks, ['selected:q']);
+});
+
+test('without any commit, reply does nothing', () => {
+    assert.equal(press('r', null, null), false);
+});
+
+test('actions inside inert commits are not triggered', () => {
+    const clicks: string[] = [];
+
+    assert.equal(press('r', card('deleted', ['r'], clicks, true), null), false);
+    assert.deepEqual(clicks, []);
+});
+
+test('reply focuses an already open composer without cancelling it', () => {
+    const clicks: string[] = [];
+    const focuses: string[] = [];
+    const main = card('main', [], clicks, false, focuses);
+
+    assert.equal(press('r', null, main), true);
+    assert.deepEqual(focuses, ['main']);
+    assert.deepEqual(clicks, []);
+});
+
+test('an open composer inside an inert card cannot receive focus', () => {
+    const focuses: string[] = [];
+
+    assert.equal(press('r', null, card('deleted', [], [], true, focuses)), false);
+    assert.deepEqual(focuses, []);
+});
+
+test('a nested reply composer does not take the main commit shortcut', () => {
+    const focuses: string[] = [];
+    const reply = card('nested', [], [], false, focuses);
+    const main = card('main', [], []);
+
+    main.querySelectorAll = reply.querySelectorAll.bind(reply);
+
+    assert.equal(press('r', null, main), false);
+    assert.deepEqual(focuses, []);
+});