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..4fbd7b273 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'; @@ -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( - `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; } diff --git a/resources/js/lib/commit-shortcuts.ts b/resources/js/lib/commit-shortcuts.ts new file mode 100644 index 000000000..06b75fd8f --- /dev/null +++ b/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( + `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( + '[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 new file mode 100644 index 000000000..0aeb66e1b --- /dev/null +++ b/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, []); +});