From 5ad6197d0ee10e633f4e1f168c282eba5b0b5d21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Sun, 1 Nov 2026 12:00:19 +0000 Subject: [PATCH 1/2] fix(mobile): give the PR Review paste control a visible result on every tap The paste control runs as a fire-and-forget press handler, and it read the clipboard with no error handling. A read the platform refuses therefore disappeared: no text in the field, no inline validation, no toast, nothing. Read the clipboard through one helper that never rejects. A refused text read, or a refused text probe on the empty path, now reports the paste as denied, so the tap always ends in the field being replaced or a toast that names why nothing was pasted. --- .../pr-review-entry-screen-paste-read.test.ts | 50 +++++++++++++++ .../pr-review-entry-screen-test-utils.ts | 10 +++ .../pr-review/pr-review-entry-screen.tsx | 23 +++---- .../src/lib/pr-review/pr-link-paste.test.ts | 58 +++++++++++++++++ .../mobile/src/lib/pr-review/pr-link-paste.ts | 63 +++++++++++++++++++ 5 files changed, 193 insertions(+), 11 deletions(-) create mode 100644 apps/mobile/src/components/pr-review/pr-review-entry-screen-paste-read.test.ts diff --git a/apps/mobile/src/components/pr-review/pr-review-entry-screen-paste-read.test.ts b/apps/mobile/src/components/pr-review/pr-review-entry-screen-paste-read.test.ts new file mode 100644 index 00000000000..879c13ed56c --- /dev/null +++ b/apps/mobile/src/components/pr-review/pr-review-entry-screen-paste-read.test.ts @@ -0,0 +1,50 @@ +// The paste control runs a fire-and-forget press handler, so a clipboard read +// the platform refuses must still end in a visible result. These cases live +// apart from the URL-field suite to keep both files within the lint line cap. + +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { find, flush, mocks, propsOf, renderLoaded, resetHookSlots } from './pr-review-entry-screen-test-utils'; + +beforeEach(() => { + vi.clearAllMocks(); + resetHookSlots(); + mocks.clipboard.current = ''; + mocks.clipboardHasString.current = false; + mocks.clipboardFailure.current = null; + mocks.clipboardHasStringFailure.current = null; +}); + +function pasteControl(tree: unknown) { + return find( + tree, + 'Pressable', + p => p.accessibilityLabel === 'Paste pull request or merge request link' + ); +} + +describe('paste clipboard read failures', () => { + it('reports a refused read instead of leaving the tap without a result', async () => { + mocks.clipboardFailure.current = new Error('clipboard unavailable'); + const tree = await renderLoaded(); + await (propsOf(pasteControl(tree)).onPress as () => Promise)(); + await flush(); + expect(mocks.toastError).toHaveBeenCalledWith( + 'Paste access was denied. Allow paste and try again.' + ); + expect(mocks.toastError).toHaveBeenCalledTimes(1); + expect(mocks.push).not.toHaveBeenCalled(); + }); + + it('reports the same denial when only the text probe fails', async () => { + mocks.clipboardHasStringFailure.current = new Error('clipboard unavailable'); + const tree = await renderLoaded(); + await (propsOf(pasteControl(tree)).onPress as () => Promise)(); + await flush(); + expect(mocks.toastError).toHaveBeenCalledWith( + 'Paste access was denied. Allow paste and try again.' + ); + expect(mocks.toastError).not.toHaveBeenCalledWith('Clipboard is empty'); + expect(mocks.push).not.toHaveBeenCalled(); + }); +}); \ No newline at end of file diff --git a/apps/mobile/src/components/pr-review/pr-review-entry-screen-test-utils.ts b/apps/mobile/src/components/pr-review/pr-review-entry-screen-test-utils.ts index 120efc2380c..74c0ce66348 100644 --- a/apps/mobile/src/components/pr-review/pr-review-entry-screen-test-utils.ts +++ b/apps/mobile/src/components/pr-review/pr-review-entry-screen-test-utils.ts @@ -23,6 +23,10 @@ const harnessMocks = vi.hoisted(() => ({ toastError: vi.fn(), clipboard: { current: '' as string }, clipboardHasString: { current: false }, + // A clipboard read the platform refuses: the mocks below throw these so a + // test can prove the tap still ends in a visible result. + clipboardFailure: { current: null as Error | null }, + clipboardHasStringFailure: { current: null as Error | null }, })); // A hoisted binding cannot be exported directly; alias it for the test files. @@ -53,10 +57,16 @@ vi.mock('expo-router', () => ({ vi.mock('expo-clipboard', () => ({ getStringAsync: async () => { await Promise.resolve(); + if (harnessMocks.clipboardFailure.current) { + throw harnessMocks.clipboardFailure.current; + } return harnessMocks.clipboard.current; }, hasStringAsync: async () => { await Promise.resolve(); + if (harnessMocks.clipboardHasStringFailure.current) { + throw harnessMocks.clipboardHasStringFailure.current; + } return harnessMocks.clipboardHasString.current; }, })); diff --git a/apps/mobile/src/components/pr-review/pr-review-entry-screen.tsx b/apps/mobile/src/components/pr-review/pr-review-entry-screen.tsx index cc079fcf842..7f94d5e36b6 100644 --- a/apps/mobile/src/components/pr-review/pr-review-entry-screen.tsx +++ b/apps/mobile/src/components/pr-review/pr-review-entry-screen.tsx @@ -24,11 +24,10 @@ import { ICON_TARGET_BOX_CLASS } from '@/lib/a11y/tap-target'; import { useThemeColors } from '@/lib/hooks/use-theme-colors'; import { providerPrRefLabel, providerPrRoutePath } from '@/lib/pr-review/provider-pr-ref'; import { - decidePrLinkPaste, prLinkToastClipboardEmptyCopy, prLinkToastPasteDeniedCopy, + readPrLinkPasteClipboard, selectPrLinkClearButtonVisible, - selectPrLinkPasteEmptyReason, } from '@/lib/pr-review/pr-link-paste'; import { getRecentPrs, @@ -92,22 +91,24 @@ export function PrReviewEntryScreen() { }; const handlePaste = async () => { - const clipboard = await Clipboard.getStringAsync(); - const decision = decidePrLinkPaste(clipboard); - if (decision.kind === 'empty') { - // iOS answers a denied prompt with an empty read, so ask the prompt-free - // probe whether the clipboard still holds text before calling it empty. - const clipboardHasString = await Clipboard.hasStringAsync(); + // Every tap ends in a visible result: the field is replaced, or a toast + // names why nothing could be pasted. The reader never rejects, so a + // refused or broken clipboard read cannot vanish into the press handler. + const outcome = await readPrLinkPasteClipboard(Clipboard); + if (outcome.kind === 'empty') { + // iOS answers a denied prompt with an empty read, so the reader asks the + // prompt-free probe whether the clipboard still holds text before calling + // it empty; a read that failed outright reports the same denial. announcingToast.error( - selectPrLinkPasteEmptyReason(clipboard, clipboardHasString) === 'denied' + outcome.reason === 'denied' ? prLinkToastPasteDeniedCopy() : prLinkToastClipboardEmptyCopy() ); return; } // Replace the native field, ref and derived UI together, never append at the cursor. - writeField(decision.text); - if (decision.kind === 'non-url-text') { + writeField(outcome.text); + if (outcome.kind === 'non-url-text') { return; } handleSubmit(); diff --git a/apps/mobile/src/lib/pr-review/pr-link-paste.test.ts b/apps/mobile/src/lib/pr-review/pr-link-paste.test.ts index 1cc11a34112..4139ac13708 100644 --- a/apps/mobile/src/lib/pr-review/pr-link-paste.test.ts +++ b/apps/mobile/src/lib/pr-review/pr-link-paste.test.ts @@ -5,6 +5,7 @@ import { decidePrLinkPaste, prLinkToastClipboardEmptyCopy, prLinkToastPasteDeniedCopy, + readPrLinkPasteClipboard, selectPrLinkClearButtonVisible, selectPrLinkPasteEmptyReason, } from './pr-link-paste'; @@ -137,6 +138,63 @@ describe('selectPrLinkPasteEmptyReason', () => { }); }); +describe('readPrLinkPasteClipboard', () => { + const reader = (text: string | null, hasString: boolean) => ({ + getStringAsync: async () => { + await Promise.resolve(); + return text; + }, + hasStringAsync: async () => { + await Promise.resolve(); + return hasString; + }, + }); + + it('hands a resolvable read to the paste decision', async () => { + const url = 'https://github.com/octocat/hello-world/pull/42'; + await expect(readPrLinkPasteClipboard(reader(url, true))).resolves.toEqual({ + kind: 'valid-pr-url', + text: url, + }); + await expect(readPrLinkPasteClipboard(reader(' not a link ', true))).resolves.toEqual({ + kind: 'non-url-text', + text: 'not a link', + }); + }); + + it('names an empty clipboard as empty', async () => { + await expect(readPrLinkPasteClipboard(reader('', false))).resolves.toEqual({ + kind: 'empty', + reason: 'empty', + }); + }); + + it('reports a declined paste prompt as a denial', async () => { + await expect(readPrLinkPasteClipboard(reader('', true))).resolves.toEqual({ + kind: 'empty', + reason: 'denied', + }); + }); + + it('turns a rejected text read into the denied result instead of rejecting', async () => { + await expect( + readPrLinkPasteClipboard({ + getStringAsync: () => Promise.reject(new Error('clipboard unavailable')), + hasStringAsync: () => Promise.resolve(false), + }) + ).resolves.toEqual({ kind: 'empty', reason: 'denied' }); + }); + + it('turns a rejected text probe into the denied result instead of rejecting', async () => { + await expect( + readPrLinkPasteClipboard({ + getStringAsync: () => Promise.resolve(''), + hasStringAsync: () => Promise.reject(new Error('clipboard unavailable')), + }) + ).resolves.toEqual({ kind: 'empty', reason: 'denied' }); + }); +}); + describe('selectPrLinkClearButtonVisible', () => { it('is present when the field has content', () => { expect(selectPrLinkClearButtonVisible({ hasInput: true })).toBe(true); diff --git a/apps/mobile/src/lib/pr-review/pr-link-paste.ts b/apps/mobile/src/lib/pr-review/pr-link-paste.ts index 7193e02588c..968ca954435 100644 --- a/apps/mobile/src/lib/pr-review/pr-link-paste.ts +++ b/apps/mobile/src/lib/pr-review/pr-link-paste.ts @@ -67,6 +67,69 @@ export function selectPrLinkPasteEmptyReason( return (clipboard ?? '').length === 0 && clipboardHasString ? 'denied' : 'empty'; } +type PrLinkPasteClipboardReader = { + getStringAsync: () => Promise; + hasStringAsync: () => Promise; +}; + +/** + * A paste tap's clipboard outcome. `empty` carries why the read produced + * nothing, so the call site picks the toast that names it; every other kind + * carries the text to write into the field. + */ +export type PrLinkPasteOutcome = + | { kind: 'valid-pr-url'; text: string } + | { kind: 'non-url-text'; text: string } + | { kind: 'empty'; reason: PrLinkPasteEmptyReason }; + +/** + * Read the clipboard for a paste tap without ever rejecting. + * + * The control runs this as a fire-and-forget press handler, so a rejected + * read would leave the tap with no visible result at all: no text, no + * validation, no toast. A read the platform refuses — the iOS paste prompt + * declined, a service that is not available, a throw out of the native + * module — is reported as `denied`, the state whose copy already tells the + * reader that paste access was refused and asks them to try again. The + * clipboard-empty answer keeps its own copy through + * `selectPrLinkPasteEmptyReason`, so a resolver that throws on the second + * probe cannot swallow the tap either. + * + * The attachment helpers (`readClipboardText`, `hasClipboardText`) also + * swallow a failed read, but they collapse it into `''`/`false`, which this + * control cannot use: it has to tell a refused read from a clipboard that is + * genuinely empty to pick the copy that names the right cause. + */ +export async function readPrLinkPasteClipboard( + reader: PrLinkPasteClipboardReader +): Promise { + const textRead = await settleClipboardRead(() => reader.getStringAsync()); + if (!textRead.ok) { + return { kind: 'empty', reason: 'denied' }; + } + const clipboard = textRead.value ?? ''; + const decision = decidePrLinkPaste(clipboard); + if (decision.kind !== 'empty') { + return decision; + } + const probe = await settleClipboardRead(() => reader.hasStringAsync()); + if (!probe.ok) { + return { kind: 'empty', reason: 'denied' }; + } + return { kind: 'empty', reason: selectPrLinkPasteEmptyReason(clipboard, probe.value) }; +} + +/** One clipboard read, with a refusal reported as data instead of a rejection. */ +async function settleClipboardRead( + read: () => Promise +): Promise<{ ok: true; value: T } | { ok: false }> { + try { + return { ok: true, value: await read() }; + } catch { + return { ok: false }; + } +} + /** Toast copy when paste finds an empty clipboard. */ export function prLinkToastClipboardEmptyCopy(): string { return i18n.t('prReview.linkPasteClipboardEmpty'); From a178640706332c08b0859cc8ff26615ee493b903 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Sun, 1 Nov 2026 12:46:26 +0000 Subject: [PATCH 2/2] fix(mobile): apply the repository formatter to the paste change Reformat the new test's import and the toast selection to the shared print width, and end the file with a newline. --- .../pr-review-entry-screen-paste-read.test.ts | 11 +++++++++-- .../components/pr-review/pr-review-entry-screen.tsx | 4 +--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/apps/mobile/src/components/pr-review/pr-review-entry-screen-paste-read.test.ts b/apps/mobile/src/components/pr-review/pr-review-entry-screen-paste-read.test.ts index 879c13ed56c..d32a0f2d141 100644 --- a/apps/mobile/src/components/pr-review/pr-review-entry-screen-paste-read.test.ts +++ b/apps/mobile/src/components/pr-review/pr-review-entry-screen-paste-read.test.ts @@ -4,7 +4,14 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { find, flush, mocks, propsOf, renderLoaded, resetHookSlots } from './pr-review-entry-screen-test-utils'; +import { + find, + flush, + mocks, + propsOf, + renderLoaded, + resetHookSlots, +} from './pr-review-entry-screen-test-utils'; beforeEach(() => { vi.clearAllMocks(); @@ -47,4 +54,4 @@ describe('paste clipboard read failures', () => { expect(mocks.toastError).not.toHaveBeenCalledWith('Clipboard is empty'); expect(mocks.push).not.toHaveBeenCalled(); }); -}); \ No newline at end of file +}); diff --git a/apps/mobile/src/components/pr-review/pr-review-entry-screen.tsx b/apps/mobile/src/components/pr-review/pr-review-entry-screen.tsx index 7f94d5e36b6..3a832e09b36 100644 --- a/apps/mobile/src/components/pr-review/pr-review-entry-screen.tsx +++ b/apps/mobile/src/components/pr-review/pr-review-entry-screen.tsx @@ -100,9 +100,7 @@ export function PrReviewEntryScreen() { // prompt-free probe whether the clipboard still holds text before calling // it empty; a read that failed outright reports the same denial. announcingToast.error( - outcome.reason === 'denied' - ? prLinkToastPasteDeniedCopy() - : prLinkToastClipboardEmptyCopy() + outcome.reason === 'denied' ? prLinkToastPasteDeniedCopy() : prLinkToastClipboardEmptyCopy() ); return; }