From fa7b6372774efe92f02bacec26f8fbe87c7aa54c Mon Sep 17 00:00:00 2001 From: Punyapal Shah <53343069+MrPunyapal@users.noreply.github.com> Date: Thu, 29 Oct 2026 23:32:23 +0530 Subject: [PATCH 1/3] fix: use focused commit for shortcuts on commit pages --- resources/js/components/commit-card.tsx | 1 + resources/js/hooks/use-keyboard-shortcuts.ts | 13 ++-- resources/js/lib/commit-shortcuts.ts | 17 +++++ tests/Unit/CommitPageShortcuts.test.ts | 77 ++++++++++++++++++++ 4 files changed, 101 insertions(+), 7 deletions(-) create mode 100644 resources/js/lib/commit-shortcuts.ts create mode 100644 tests/Unit/CommitPageShortcuts.test.ts diff --git a/resources/js/components/commit-card.tsx b/resources/js/components/commit-card.tsx index 9634b1df0..dc705ca13 100644 --- a/resources/js/components/commit-card.tsx +++ b/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} diff --git a/resources/js/hooks/use-keyboard-shortcuts.ts b/resources/js/hooks/use-keyboard-shortcuts.ts index de9be7a96..c7a6aa09b 100644 --- a/resources/js/hooks/use-keyboard-shortcuts.ts +++ b/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'; @@ -51,13 +52,11 @@ 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( - `button[aria-keyshortcuts="${CSS.escape(key)}"]`, - ); - - return action && !action.closest('[inert]') ? action : null; + return commitActionForShortcut( + key, + document.activeElement?.closest('[data-reply], [data-commit]') ?? null, + key === 'r' ? document.querySelector('[data-focused-commit]') : null, + ); } function openFocusedCommit(): boolean { diff --git a/resources/js/lib/commit-shortcuts.ts b/resources/js/lib/commit-shortcuts.ts new file mode 100644 index 000000000..47e948be9 --- /dev/null +++ b/resources/js/lib/commit-shortcuts.ts @@ -0,0 +1,17 @@ +/** + * 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 | null { + const action = ( + activeTarget ?? (key === 'r' ? pageCommit : null) + )?.querySelector( + `button[aria-keyshortcuts="${CSS.escape(key)}"]`, + ); + + return action && !action.closest('[inert]') ? action : null; +} diff --git a/tests/Unit/CommitPageShortcuts.test.ts b/tests/Unit/CommitPageShortcuts.test.ts new file mode 100644 index 000000000..dc965f6d8 --- /dev/null +++ b/tests/Unit/CommitPageShortcuts.test.ts @@ -0,0 +1,77 @@ +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, +): Element { + return { + 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; + }, + } as unknown as Element; +} + +function press(key: string, active: Element | null, page: Element | null): boolean { + const action = commitActionForShortcut(key, active, page); + 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, []); +}); From eeee66d10dfa85099c5a5a41159cebd0522d68d3 Mon Sep 17 00:00:00 2001 From: Punyapal Shah <53343069+MrPunyapal@users.noreply.github.com> Date: Thu, 29 Oct 2026 23:54:41 +0530 Subject: [PATCH 2/3] fix: focus an already open reply composer with r --- resources/js/hooks/use-keyboard-shortcuts.ts | 7 ++- resources/js/lib/commit-shortcuts.ts | 29 ++++++++-- tests/Unit/CommitPageShortcuts.test.ts | 56 ++++++++++++++++++-- 3 files changed, 83 insertions(+), 9 deletions(-) diff --git a/resources/js/hooks/use-keyboard-shortcuts.ts b/resources/js/hooks/use-keyboard-shortcuts.ts index c7a6aa09b..99165c21d 100644 --- a/resources/js/hooks/use-keyboard-shortcuts.ts +++ b/resources/js/hooks/use-keyboard-shortcuts.ts @@ -157,7 +157,12 @@ export function useKeyboardShortcuts( if (commitAction) { event.preventDefault(); - commitAction.click(); + + if (commitAction.tagName === 'TEXTAREA') { + commitAction.focus(); + } else { + commitAction.click(); + } return; } diff --git a/resources/js/lib/commit-shortcuts.ts b/resources/js/lib/commit-shortcuts.ts index 47e948be9..06b75fd8f 100644 --- a/resources/js/lib/commit-shortcuts.ts +++ b/resources/js/lib/commit-shortcuts.ts @@ -6,12 +6,31 @@ export function commitActionForShortcut( key: string, activeTarget: Element | null, pageCommit: Element | null, -): HTMLButtonElement | null { - const action = ( - activeTarget ?? (key === 'r' ? pageCommit : null) - )?.querySelector( +): HTMLButtonElement | HTMLTextAreaElement | null { + const target = activeTarget ?? (key === 'r' ? pageCommit : null); + const action = target?.querySelector( `button[aria-keyshortcuts="${CSS.escape(key)}"]`, ); - return action && !action.closest('[inert]') ? action : null; + 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( + '[data-test="reply-composer"] textarea', + ), + ).find( + (textarea) => + textarea.closest('[data-reply], [data-commit]') === target && + !textarea.closest('[inert]'), + ) ?? null + ); } diff --git a/tests/Unit/CommitPageShortcuts.test.ts b/tests/Unit/CommitPageShortcuts.test.ts index dc965f6d8..0aeb66e1b 100644 --- a/tests/Unit/CommitPageShortcuts.test.ts +++ b/tests/Unit/CommitPageShortcuts.test.ts @@ -12,8 +12,9 @@ function card( keys: string[], clicks: string[], inert = false, + focuses?: string[], ): Element { - return { + const element = { querySelector: (selector: string) => { const key = selector.match(/^button\[aria-keyshortcuts="(.+)"\]$/)?.[1]; @@ -24,12 +25,33 @@ function card( } : null; }, - } as unknown as Element; + 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); - action?.click(); + if (action?.tagName === 'TEXTAREA') { + action.focus(); + } else { + action?.click(); + } return action !== null; } @@ -75,3 +97,31 @@ test('actions inside inert commits are not triggered', () => { 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, []); +}); From 388035aa9216f4b4d79362b7b77bbbb3efd6337f Mon Sep 17 00:00:00 2001 From: Punyapal Shah <53343069+MrPunyapal@users.noreply.github.com> Date: Tue, 3 Nov 2026 14:04:25 +0530 Subject: [PATCH 3/3] fix: include reply textarea in shortcut action return type --- resources/js/hooks/use-keyboard-shortcuts.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/resources/js/hooks/use-keyboard-shortcuts.ts b/resources/js/hooks/use-keyboard-shortcuts.ts index 99165c21d..4fbd7b273 100644 --- a/resources/js/hooks/use-keyboard-shortcuts.ts +++ b/resources/js/hooks/use-keyboard-shortcuts.ts @@ -51,7 +51,9 @@ 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 { +function focusedCommitAction( + key: string, +): HTMLButtonElement | HTMLTextAreaElement | null { return commitActionForShortcut( key, document.activeElement?.closest('[data-reply], [data-commit]') ?? null,