Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
// 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<void>)();
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<void>)();
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();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
},
}));
Expand Down
25 changes: 12 additions & 13 deletions apps/mobile/src/components/pr-review/pr-review-entry-screen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -92,22 +91,22 @@ 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'
? prLinkToastPasteDeniedCopy()
: prLinkToastClipboardEmptyCopy()
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();
Expand Down
58 changes: 58 additions & 0 deletions apps/mobile/src/lib/pr-review/pr-link-paste.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
decidePrLinkPaste,
prLinkToastClipboardEmptyCopy,
prLinkToastPasteDeniedCopy,
readPrLinkPasteClipboard,
selectPrLinkClearButtonVisible,
selectPrLinkPasteEmptyReason,
} from './pr-link-paste';
Expand Down Expand Up @@ -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);
Expand Down
63 changes: 63 additions & 0 deletions apps/mobile/src/lib/pr-review/pr-link-paste.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,69 @@ export function selectPrLinkPasteEmptyReason(
return (clipboard ?? '').length === 0 && clipboardHasString ? 'denied' : 'empty';
}

type PrLinkPasteClipboardReader = {
getStringAsync: () => Promise<string | null | undefined>;
hasStringAsync: () => Promise<boolean>;
};

/**
* 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<PrLinkPasteOutcome> {
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<T>(
read: () => Promise<T>
): 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');
Expand Down
Loading