From bea723683f2e1a4e175ea9e4e2dee9889f5d425a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Thu, 30 Jul 2026 13:02:39 +0200 Subject: [PATCH 01/12] feat(web): expose thread-level diffHunk on PR review threads Select diffHunk on each thread's first comment in REVIEW_THREADS_QUERY, carry it through the review-thread DTO (nullable, '' mapped to null), and cover the mapping with mapper and router tests. --- apps/web/src/lib/github-pr-review/dtos.ts | 1 + .../src/lib/github-pr-review/mappers.test.ts | 52 ++++++++ apps/web/src/lib/github-pr-review/mappers.ts | 2 + .../routers/github-pr-review-router.test.ts | 116 ++++++++++++++++++ .../src/routers/github-pr-review-router.ts | 3 + 5 files changed, 174 insertions(+) diff --git a/apps/web/src/lib/github-pr-review/dtos.ts b/apps/web/src/lib/github-pr-review/dtos.ts index 560fa6076f8..f294dc635a1 100644 --- a/apps/web/src/lib/github-pr-review/dtos.ts +++ b/apps/web/src/lib/github-pr-review/dtos.ts @@ -143,6 +143,7 @@ export const GitHubPrReviewReviewThreadSchema = z originalLine: z.number().int().nullable(), originalStartLine: z.number().int().nullable(), diffSide: z.enum(['LEFT', 'RIGHT']).nullable(), + diffHunk: z.string().nullable(), comments: z.array(GitHubPrReviewReviewCommentSchema), }) .strict(); diff --git a/apps/web/src/lib/github-pr-review/mappers.test.ts b/apps/web/src/lib/github-pr-review/mappers.test.ts index d54edc73882..26cadfaec98 100644 --- a/apps/web/src/lib/github-pr-review/mappers.test.ts +++ b/apps/web/src/lib/github-pr-review/mappers.test.ts @@ -367,6 +367,58 @@ describe('buildReviewThreadsResult', () => { expect(result.threads[0]?.comments).toHaveLength(120); }); + it('maps diffHunk through; empty string and absent become null', () => { + const withHunk = buildReviewThreadsResult({ + page: 1, + hasNextPage: false, + endCursor: null, + conversation: [], + threads: [ + { + id: 'thread-hunk', + isResolved: false, + isOutdated: false, + diffHunk: '@@ -1,3 +1,4 @@\n line', + comments: [], + }, + ], + }); + expect(withHunk.threads[0]?.diffHunk).toBe('@@ -1,3 +1,4 @@\n line'); + + const emptyHunk = buildReviewThreadsResult({ + page: 1, + hasNextPage: false, + endCursor: null, + conversation: [], + threads: [ + { + id: 'thread-empty-hunk', + isResolved: false, + isOutdated: false, + diffHunk: '', + comments: [], + }, + ], + }); + expect(emptyHunk.threads[0]?.diffHunk).toBeNull(); + + const absentHunk = buildReviewThreadsResult({ + page: 1, + hasNextPage: false, + endCursor: null, + conversation: [], + threads: [ + { + id: 'thread-no-hunk', + isResolved: false, + isOutdated: false, + comments: [], + }, + ], + }); + expect(absentHunk.threads[0]?.diffHunk).toBeNull(); + }); + it('maps conversation comments through the same DTO shape as thread comments', () => { const result = buildReviewThreadsResult({ page: 1, diff --git a/apps/web/src/lib/github-pr-review/mappers.ts b/apps/web/src/lib/github-pr-review/mappers.ts index 9212c189e4e..8e2beb8a8cb 100644 --- a/apps/web/src/lib/github-pr-review/mappers.ts +++ b/apps/web/src/lib/github-pr-review/mappers.ts @@ -279,6 +279,7 @@ export type GraphQlReviewThreadInput = { originalLine?: number | null; originalStartLine?: number | null; diffSide?: 'LEFT' | 'RIGHT' | null; + diffHunk?: string | null; comments: GraphQlReviewCommentInput[]; }; @@ -331,6 +332,7 @@ export function buildReviewThreadsResult(args: { originalLine: t.originalLine ?? null, originalStartLine: t.originalStartLine ?? null, diffSide, + diffHunk: t.diffHunk || null, comments: t.comments.map(mapReviewComment), }; }); diff --git a/apps/web/src/routers/github-pr-review-router.test.ts b/apps/web/src/routers/github-pr-review-router.test.ts index ace5abbbea3..d1798404ed3 100644 --- a/apps/web/src/routers/github-pr-review-router.test.ts +++ b/apps/web/src/routers/github-pr-review-router.test.ts @@ -569,6 +569,122 @@ describe('githubPrReviewRouter infinite-query inputs accept the tRPC direction f { content: 'HEART', count: 1, viewerHasReacted: false }, ]); }); + + it('listReviewThreads maps first-comment diffHunk onto the thread DTO', async () => { + getGitHubUserAccessToken.mockResolvedValueOnce(connected('t1', 'auth_1', 1)); + const caller = createCaller({ user: { id: 'user-1' } as User }); + buildOctokit('t1').request.mockImplementation( + async (_path: string, body: { query: string }) => { + if (body.query.includes('PrReviewConversationComments')) { + return { + data: { + data: { + repository: { + pullRequest: { + comments: { nodes: [], pageInfo: { hasNextPage: false, endCursor: null } }, + }, + }, + }, + }, + }; + } + return { + data: { + data: { + repository: { + pullRequest: { + reviewThreads: { + pageInfo: { hasNextPage: false, endCursor: null }, + nodes: [ + { + id: 'PRRT_with_hunk', + isResolved: false, + isOutdated: false, + subjectType: 'LINE', + path: 'src/foo.ts', + line: 4, + startLine: null, + originalLine: 4, + originalStartLine: null, + diffSide: 'RIGHT', + comments: { + pageInfo: { hasNextPage: false, endCursor: null }, + nodes: [ + { + databaseId: 21, + id: 'PRRC_21', + body: 'nit', + diffHunk: '@@ -1,2 +1,3 @@\n+added', + createdAt: '2024-01-01T00:00:00Z', + author: { login: 'octocat', avatarUrl: 'https://x/y.png' }, + reactionGroups: [], + }, + ], + }, + }, + { + id: 'PRRT_no_comments', + isResolved: false, + isOutdated: false, + subjectType: 'LINE', + path: 'src/bar.ts', + line: 1, + startLine: null, + originalLine: 1, + originalStartLine: null, + diffSide: 'RIGHT', + comments: { + pageInfo: { hasNextPage: false, endCursor: null }, + nodes: [], + }, + }, + { + id: 'PRRT_no_hunk', + isResolved: false, + isOutdated: false, + subjectType: 'LINE', + path: 'src/baz.ts', + line: 2, + startLine: null, + originalLine: 2, + originalStartLine: null, + diffSide: 'RIGHT', + comments: { + pageInfo: { hasNextPage: false, endCursor: null }, + nodes: [ + { + databaseId: 22, + id: 'PRRC_22', + body: 'no hunk', + createdAt: '2024-01-01T00:00:00Z', + author: { login: 'octocat', avatarUrl: 'https://x/y.png' }, + reactionGroups: [], + }, + ], + }, + }, + ], + }, + }, + }, + }, + }, + }; + } + ); + + const result = await caller.listReviewThreads({ + owner: 'octocat', + repo: 'hello', + number: 1, + direction: 'forward', + }); + + expect(result.threads).toHaveLength(3); + expect(result.threads[0]?.diffHunk).toBe('@@ -1,2 +1,3 @@\n+added'); + expect(result.threads[1]?.diffHunk).toBeNull(); + expect(result.threads[2]?.diffHunk).toBeNull(); + }); }); describe('githubPrReviewRouter.listReviewThreads conversation comments', () => { diff --git a/apps/web/src/routers/github-pr-review-router.ts b/apps/web/src/routers/github-pr-review-router.ts index 8ad6655c720..a08fadacf95 100644 --- a/apps/web/src/routers/github-pr-review-router.ts +++ b/apps/web/src/routers/github-pr-review-router.ts @@ -236,6 +236,7 @@ const REVIEW_THREADS_QUERY = /* GraphQL */ ` databaseId id body + diffHunk createdAt author { login @@ -405,6 +406,7 @@ type GraphQlCommentNode = { databaseId: number; id: string; body: string; + diffHunk?: string | null; createdAt: string; author: { login: string; avatarUrl: string } | null; reactionGroups: GraphQlReactionGroup[]; @@ -840,6 +842,7 @@ export const githubPrReviewRouter = createTRPCRouter({ originalLine: node.originalLine, originalStartLine: node.originalStartLine, diffSide: node.diffSide, + diffHunk: node.comments.nodes[0]?.diffHunk ?? null, comments, }; }) From 17c6604d0c8bc352e9b0fa7a54838ca3f3bea1ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Thu, 30 Jul 2026 13:13:23 +0200 Subject: [PATCH 02/12] feat(mobile): quote anchored diff hunk in PR discussion threads Render the thread's diffHunk (from the first comment) above the comments when expanded, parsed via parsePatch and drawn with DiffLine, capped at 30 lines with a trailing count row. File-level and malformed anchors fall back to the label only. Resolve toggle becomes an icon-only button so the Resolved badge is the single resolved indicator. --- .../discussion/discussion-thread.tsx | 24 ++- .../discussion/thread-diff-snippet.tsx | 36 ++++ .../discussion/thread-diff-snippet.test.ts | 156 ++++++++++++++++++ .../discussion/thread-diff-snippet.ts | 58 +++++++ 4 files changed, 265 insertions(+), 9 deletions(-) create mode 100644 apps/mobile/src/components/pr-review/discussion/thread-diff-snippet.tsx create mode 100644 apps/mobile/src/lib/pr-review/discussion/thread-diff-snippet.test.ts create mode 100644 apps/mobile/src/lib/pr-review/discussion/thread-diff-snippet.ts diff --git a/apps/mobile/src/components/pr-review/discussion/discussion-thread.tsx b/apps/mobile/src/components/pr-review/discussion/discussion-thread.tsx index 742c492aef0..518c4fb734e 100644 --- a/apps/mobile/src/components/pr-review/discussion/discussion-thread.tsx +++ b/apps/mobile/src/components/pr-review/discussion/discussion-thread.tsx @@ -1,8 +1,15 @@ -// Single review-thread card: anchor header + comments list + reply input. +// Single review-thread card: anchor header + optional quoted diff + +// comments list + reply input. // // - The thread header shows the anchor label ("src/a.ts L10 (RIGHT)" // or "File comment on src/a.ts" or "Outdated on ...") and the -// "Outdated" / "Resolved" badges when applicable. +// "Outdated" / "Resolved" badges when applicable. The "Resolved" +// badge is the sole Resolved text indicator; the resolve control +// is an icon-only circular button (a11y: "Resolve thread" / +// "Unresolve thread"). +// - Expanded LINE-anchored threads render a capped quoted diff +// snippet (from thread.diffHunk) above the comments list. File- +// level / empty / unparseable hunks show no snippet. // - Resolved threads are COLLAPSED by default (tapping the header // expands them). The repo's UI/UX rule for compact product rhythm // is to keep the noise level down on the happy path, so an @@ -21,6 +28,7 @@ import { Pressable, View } from 'react-native'; import { CommentRow } from '@/components/pr-review/discussion/comment-row'; import { ReplyInput } from '@/components/pr-review/discussion/reply-input'; +import { ThreadDiffSnippet } from '@/components/pr-review/discussion/thread-diff-snippet'; import { Text } from '@/components/ui/text'; import { type ReviewComment, @@ -29,6 +37,7 @@ import { selectThreadAnchorLabel, selectThreadBadges, } from '@/lib/pr-review/discussion/review-discussion-types'; +import { selectThreadDiffSnippet } from '@/lib/pr-review/discussion/thread-diff-snippet'; import { useAddReactionMutation, useRemoveReactionMutation, @@ -65,6 +74,7 @@ export function DiscussionThread({ const anchorLabel = selectThreadAnchorLabel(thread); const badges = selectThreadBadges(thread); + const diffSnippet = selectThreadDiffSnippet(thread); const firstComment = thread.comments[0]; const isResolving = resolve.isPending || unresolve.isPending; const isReacting = addReaction.isPending || removeReaction.isPending; @@ -110,6 +120,7 @@ export function DiscussionThread({ /> {expanded ? ( <> + {diffSnippet ? : null} {thread.comments.map((comment, index) => ( 0 && 'border-t border-border pt-4')}> @@ -247,14 +258,9 @@ function ResolveToggle({ resolved, disabled, onPress }: Readonly - - - {resolved ? 'Resolved' : 'Resolve'} - + ); } diff --git a/apps/mobile/src/components/pr-review/discussion/thread-diff-snippet.tsx b/apps/mobile/src/components/pr-review/discussion/thread-diff-snippet.tsx new file mode 100644 index 00000000000..f8a1ef1726b --- /dev/null +++ b/apps/mobile/src/components/pr-review/discussion/thread-diff-snippet.tsx @@ -0,0 +1,36 @@ +// Quoted diff snippet rendered above comments in an expanded LINE-anchored +// discussion thread. Reuses DiffLine (no onTap — static quote only). + +import { View } from 'react-native'; + +import { DiffLine } from '@/components/pr-review/diff/diff-line'; +import { Text } from '@/components/ui/text'; +import { type ThreadDiffSnippet as ThreadDiffSnippetData } from '@/lib/pr-review/discussion/thread-diff-snippet'; + +type ThreadDiffSnippetProps = { + readonly snippet: ThreadDiffSnippetData; +}; + +export function ThreadDiffSnippet({ snippet }: Readonly) { + const truncatedCount = snippet.totalLineCount - snippet.lines.length; + return ( + + {snippet.lines.map((line, index) => ( + + ))} + {truncatedCount > 0 ? ( + + … {truncatedCount} more lines + + ) : null} + + ); +} diff --git a/apps/mobile/src/lib/pr-review/discussion/thread-diff-snippet.test.ts b/apps/mobile/src/lib/pr-review/discussion/thread-diff-snippet.test.ts new file mode 100644 index 00000000000..8210f2c1f49 --- /dev/null +++ b/apps/mobile/src/lib/pr-review/discussion/thread-diff-snippet.test.ts @@ -0,0 +1,156 @@ +import { describe, expect, it } from 'vitest'; + +import { + selectThreadDiffSnippet, + THREAD_SNIPPET_MAX_LINES, + type ThreadDiffSnippetInput, +} from './thread-diff-snippet'; + +const SIMPLE_HUNK = [ + '@@ -10,4 +10,5 @@ function greet() {', + ' const name = "world";', + '- console.log(name);', + '+ console.log("hello " + name);', + '+ return name;', + ' }', +].join('\n'); + +function baseThread(overrides: Partial = {}): ThreadDiffSnippetInput { + return { + diffHunk: SIMPLE_HUNK, + subjectType: 'LINE', + path: 'src/greet.ts', + ...overrides, + }; +} + +describe('selectThreadDiffSnippet', () => { + it('returns null for a null diffHunk', () => { + expect(selectThreadDiffSnippet(baseThread({ diffHunk: null }))).toBeNull(); + }); + + it('returns null for an empty diffHunk', () => { + expect(selectThreadDiffSnippet(baseThread({ diffHunk: '' }))).toBeNull(); + }); + + it('returns null for FILE subject even when a hunk is present', () => { + expect( + selectThreadDiffSnippet(baseThread({ subjectType: 'FILE', diffHunk: SIMPLE_HUNK })) + ).toBeNull(); + }); + + it('parses a valid hunk into add/del/context lines with line numbers', () => { + const snippet = selectThreadDiffSnippet(baseThread()); + expect(snippet).not.toBeNull(); + expect(snippet?.totalLineCount).toBe(5); + expect(snippet?.lines).toEqual([ + { + type: 'context', + oldLine: 10, + newLine: 10, + text: ' const name = "world";', + noNewlineAtEndOfFile: false, + }, + { + type: 'del', + oldLine: 11, + text: ' console.log(name);', + noNewlineAtEndOfFile: false, + }, + { + type: 'add', + newLine: 11, + text: ' console.log("hello " + name);', + noNewlineAtEndOfFile: false, + }, + { + type: 'add', + newLine: 12, + text: ' return name;', + noNewlineAtEndOfFile: false, + }, + { + type: 'context', + oldLine: 12, + newLine: 13, + text: '}', + noNewlineAtEndOfFile: false, + }, + ]); + }); + + it('parses a multi-line-range hunk (start/end span in header)', () => { + const hunk = [ + '@@ -18,6 +18,8 @@ export function sum(a: number, b: number) {', + ' // helpers', + ' const left = a;', + '- return a + b;', + '+ const right = b;', + '+ const total = left + right;', + '+ return total;', + ' }', + ].join('\n'); + const snippet = selectThreadDiffSnippet(baseThread({ diffHunk: hunk, path: 'src/math.ts' })); + expect(snippet).not.toBeNull(); + expect(snippet?.totalLineCount).toBe(7); + expect(snippet?.lines.filter(l => l.type === 'add')).toHaveLength(3); + expect(snippet?.lines.filter(l => l.type === 'del')).toHaveLength(1); + expect(snippet?.lines[0]?.oldLine).toBe(18); + expect(snippet?.lines[0]?.newLine).toBe(18); + }); + + it('treats an outdated-style hunk the same as any other (no special casing)', () => { + // Outdated threads still carry the original hunk string; selector + // path is identical to a live LINE thread. + const snippet = selectThreadDiffSnippet( + baseThread({ + diffHunk: SIMPLE_HUNK, + subjectType: 'LINE', + path: 'src/alpha.ts', + }) + ); + expect(snippet).not.toBeNull(); + expect(snippet?.lines).toHaveLength(5); + expect(snippet?.language).toBe('typescript'); + }); + + it('returns null for a garbage / unparseable string', () => { + expect(selectThreadDiffSnippet(baseThread({ diffHunk: 'not a diff at all' }))).toBeNull(); + expect(selectThreadDiffSnippet(baseThread({ diffHunk: '@@ broken header' }))).toBeNull(); + }); + + it('maps a .ts path to typescript', () => { + const snippet = selectThreadDiffSnippet(baseThread({ path: 'apps/web/src/foo.ts' })); + expect(snippet?.language).toBe('typescript'); + }); + + it('returns language null when path is null', () => { + const snippet = selectThreadDiffSnippet(baseThread({ path: null })); + expect(snippet).not.toBeNull(); + expect(snippet?.language).toBeNull(); + }); + + it(`caps lines at ${THREAD_SNIPPET_MAX_LINES} and keeps totalLineCount full`, () => { + const bodyLines: string[] = []; + // 40 context lines after the header — well over the cap. + for (let i = 0; i < 40; i += 1) { + bodyLines.push(` line ${i}`); + } + const hunk = [`@@ -1,40 +1,40 @@`, ...bodyLines].join('\n'); + const snippet = selectThreadDiffSnippet(baseThread({ diffHunk: hunk })); + expect(snippet).not.toBeNull(); + expect(snippet?.totalLineCount).toBe(40); + expect(snippet?.lines).toHaveLength(THREAD_SNIPPET_MAX_LINES); + expect(snippet?.lines[0]?.text).toBe('line 0'); + expect(snippet?.lines[THREAD_SNIPPET_MAX_LINES - 1]?.text).toBe( + `line ${THREAD_SNIPPET_MAX_LINES - 1}` + ); + }); + + it('parses a bare @@ fragment without a diff --git header', () => { + // GitHub comment.diffHunk is often just the @@ hunk, no file header. + const snippet = selectThreadDiffSnippet(baseThread({ diffHunk: SIMPLE_HUNK })); + expect(snippet).not.toBeNull(); + expect(snippet?.lines.length).toBeGreaterThan(0); + }); +}); diff --git a/apps/mobile/src/lib/pr-review/discussion/thread-diff-snippet.ts b/apps/mobile/src/lib/pr-review/discussion/thread-diff-snippet.ts new file mode 100644 index 00000000000..4c1ed5fa629 --- /dev/null +++ b/apps/mobile/src/lib/pr-review/discussion/thread-diff-snippet.ts @@ -0,0 +1,58 @@ +// Pure selector for the quoted diff snippet shown above comments in an +// expanded LINE-anchored discussion thread. +// +// GitHub's GraphQL has `diffHunk` on the anchor review comment (not the +// thread). The backend DTO lifts it to thread-level `diffHunk`. This +// helper parses that string with the same `parsePatch` the Files tab +// uses, flattens hunk body lines (no `@@` header — the thread header +// already names file + lines), and caps the rendered set so multi-line +// anchors don't blow card height. Outdated threads need no special +// casing: their `diffHunk` IS the original hunk. + +import { languageForPath } from '@/lib/pr-review/diff/highlight'; +import { type ParsedDiffLine, parsePatch } from '@/lib/pr-review/diff/parse-patch'; +import { type ReviewThread } from '@/lib/pr-review/discussion/review-discussion-types'; + +/** Max body lines rendered in a thread's quoted snippet. */ +export const THREAD_SNIPPET_MAX_LINES = 30; + +export type ThreadDiffSnippetInput = Pick; + +export type ThreadDiffSnippet = { + readonly lines: readonly ParsedDiffLine[]; + /** Uncapped count of parsed body lines (may exceed `lines.length`). */ + readonly totalLineCount: number; + readonly language: string | null; +}; + +/** + * Select the quoted diff snippet for a review thread, or `null` when + * there is nothing to show (file-level subject, missing/empty hunk, or + * unparseable / empty body after parse). + */ +export function selectThreadDiffSnippet(thread: ThreadDiffSnippetInput): ThreadDiffSnippet | null { + if (thread.subjectType === 'FILE') { + return null; + } + const hunk = thread.diffHunk; + if (hunk == null || hunk === '') { + return null; + } + + const parsed = parsePatch(hunk); + const allLines: ParsedDiffLine[] = []; + for (const h of parsed.hunks) { + for (const line of h.lines) { + allLines.push(line); + } + } + if (allLines.length === 0) { + return null; + } + + return { + lines: allLines.slice(0, THREAD_SNIPPET_MAX_LINES), + totalLineCount: allLines.length, + language: languageForPath(thread.path), + }; +} From 44512d67cfbe2d3a32b3effff5094bcb3e53c94f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Thu, 30 Jul 2026 13:13:24 +0200 Subject: [PATCH 03/12] test(mobile-e2e): stateful PR review stub with per-platform fixtures Build discussion-mixed via buildMixedFixture(idSuffix) registered at pulls 1 (iOS) and 11 (Android) with globally unique ids; add diffHunk strings, reaction-count overrides, outdated/file/resolved threads, and stateful ResolveThread/UnresolveThread/AddReaction/RemoveReaction mutations resolved by scanning fixtures for the unique id. --- apps/mobile/e2e/github-api-stub/server.mjs | 313 ++++++++++++++++++--- 1 file changed, 281 insertions(+), 32 deletions(-) diff --git a/apps/mobile/e2e/github-api-stub/server.mjs b/apps/mobile/e2e/github-api-stub/server.mjs index b1e28128562..98bc078f94c 100644 --- a/apps/mobile/e2e/github-api-stub/server.mjs +++ b/apps/mobile/e2e/github-api-stub/server.mjs @@ -4,9 +4,23 @@ * Node built-ins only. Logs every request; GraphQL logs operation name + variables. * * Identities: - * kilo-stub/discussion-mixed#1 — interleaved review + conversation fixture - * kilo-stub/discussion-conversation-only#2 — conversation comments only (0 review threads) - * kilo-stub/discussion-empty#3 — empty discussion + * kilo-stub/discussion-mixed/1 — iOS verifier (id suffix p1) + * kilo-stub/discussion-mixed/11 — Android verifier (id suffix p11) + * kilo-stub/discussion-conversation-only#2 — conversation comments only (0 review threads) + * kilo-stub/discussion-empty#3 — empty discussion + * + * discussion-mixed timeline (T+minutes from T0): + * T+1 threadA c1, T+2 conv1 (dave), T+3 conv2, T+4 threadB c1, + * T+5 threadA c2 (reply), T+6 conv3, T+7 outdated, T+8 file-level, T+9 resolved + * + * Id-suffix scheme (D9): every thread/comment/node id ends in `_p{suffix}` so parallel + * platform verifiers never share mutable ids. Numeric databaseIds are offset per key + * (e.g. 1001 → 11001 for /11). Mutations carry only threadId/subjectId — the stub + * resolves targets by scanning ALL fixtures for the unique id. + * + * Mutation support (stateful, process-lifetime in-place on per-key fixture objects): + * ResolveThread / UnresolveThread — { input: { threadId } } + * AddReaction / RemoveReaction — { input: { subjectId, content } } */ import http from 'node:http'; import fs from 'node:fs'; @@ -20,7 +34,8 @@ const LOG_PATH = const T0 = '2026-03-01T12:00:00.000Z'; // Interleaved timeline (T+minutes from T0): -// T+1 threadA c1, T+2 conv1, T+3 conv2, T+4 threadB c1, T+5 threadA c2, T+6 conv3 +// T+1 threadA c1, T+2 conv1, T+3 conv2, T+4 threadB c1, T+5 threadA c2, T+6 conv3, +// T+7 outdated, T+8 file-level, T+9 resolved const ts = minutes => { const d = new Date(T0); d.setUTCMinutes(d.getUTCMinutes() + minutes); @@ -39,23 +54,40 @@ const restUser = login => ({ site_admin: false, }); -const reactionGroups = () => - ['THUMBS_UP', 'THUMBS_DOWN', 'LAUGH', 'HOORAY', 'CONFUSED', 'HEART', 'ROCKET', 'EYES'].map( - content => ({ +const REACTION_CONTENTS = [ + 'THUMBS_UP', + 'THUMBS_DOWN', + 'LAUGH', + 'HOORAY', + 'CONFUSED', + 'HEART', + 'ROCKET', + 'EYES', +]; + +/** + * Full 8-group array. Optional overrides patch named contents only; + * count is written to the nested reactors.totalCount path. + * @param {Record} [overrides] + */ +const reactionGroups = (overrides = {}) => + REACTION_CONTENTS.map(content => { + const o = overrides[content] ?? {}; + return { content, - viewerHasReacted: false, - reactors: { totalCount: 0 }, - }) - ); + viewerHasReacted: o.viewerHasReacted ?? false, + reactors: { totalCount: o.count ?? 0 }, + }; + }); /** GraphQL IssueComment node for pullRequest.comments (PrReviewConversationComments). */ -const conversationComment = (databaseId, login, body, minutes) => ({ - id: `IC_stub_${databaseId}`, +const conversationComment = (databaseId, login, body, minutes, idSuffix, reactionOverrides) => ({ + id: `IC_stub_${databaseId}_p${idSuffix}`, databaseId, author: author(login), body, createdAt: ts(minutes), - reactionGroups: reactionGroups(), + reactionGroups: reactionGroups(reactionOverrides), }); /** REST pull-request file entry (GET /repos/{owner}/{repo}/pulls/{n}/files shape). */ @@ -88,15 +120,32 @@ const stubFiles = () => [ ), ]; -/** @type {Record} */ -const FIXTURES = { - 'kilo-stub/discussion-mixed/1': { +const DIFF_HUNK_ALPHA = + '@@ -8,4 +8,9 @@ export function alpha() {\n- return 1;\n+ // stub change\n+ return 2;\n+}\n+\n+export function alphaExtra() {\n+ return 3;\n }'; +const DIFF_HUNK_BETA = + '@@ -18,3 +18,6 @@ export function beta() {\n- return 1;\n+ return 2;\n+}\n+\n+export function betaExtra() {\n+ return 3;\n }'; +const DIFF_HUNK_ALPHA_OUTDATED = + '@@ -7,5 +7,5 @@ export function alpha() {\n const x = 0;\n- return 1;\n+ return 1;\n // end\n }'; + +/** + * Fresh mixed-discussion fixture per call (D9). idSuffix is the platform key + * fragment (1 = iOS, 11 = Android). databaseIds offset so nothing keys on dupes. + * @param {string|number} idSuffix + */ +function buildMixedFixture(idSuffix) { + const s = String(idSuffix); + const dbOff = (Number(idSuffix) - 1) * 1000; + const db = n => n + dbOff; + const tid = name => `PRRT_${name}_p${s}`; + const cid = name => `PRRC_${name}_p${s}`; + + return { title: 'Mixed discussion fixture', body: 'PR body for mixed fixture.', files: stubFiles(), threads: [ { - id: 'PRRT_thread_a', + id: tid('thread_a'), isResolved: false, isOutdated: false, subjectType: 'LINE', @@ -110,16 +159,20 @@ const FIXTURES = { pageInfo: { hasNextPage: false, endCursor: null }, nodes: [ { - databaseId: 1001, - id: 'PRRC_a1', + databaseId: db(1001), + id: cid('a1'), body: 'Thread A comment at T+1 (inline review)', createdAt: ts(1), author: author('alice'), - reactionGroups: reactionGroups(), + diffHunk: DIFF_HUNK_ALPHA, + reactionGroups: reactionGroups({ + THUMBS_UP: { count: 2, viewerHasReacted: false }, + HEART: { count: 1, viewerHasReacted: true }, + }), }, { - databaseId: 1005, - id: 'PRRC_a2', + databaseId: db(1005), + id: cid('a2'), body: 'Thread A reply at T+5 (inline review)', createdAt: ts(5), author: author('bob'), @@ -129,7 +182,7 @@ const FIXTURES = { }, }, { - id: 'PRRT_thread_b', + id: tid('thread_b'), isResolved: false, isOutdated: false, subjectType: 'LINE', @@ -143,11 +196,89 @@ const FIXTURES = { pageInfo: { hasNextPage: false, endCursor: null }, nodes: [ { - databaseId: 1004, - id: 'PRRC_b1', + databaseId: db(1004), + id: cid('b1'), body: 'Thread B comment at T+4 (inline review)', createdAt: ts(4), author: author('carol'), + diffHunk: DIFF_HUNK_BETA, + reactionGroups: reactionGroups(), + }, + ], + }, + }, + { + id: tid('thread_outdated'), + isResolved: false, + isOutdated: true, + subjectType: 'LINE', + path: 'src/alpha.ts', + line: null, + startLine: null, + originalLine: 9, + originalStartLine: null, + diffSide: 'LEFT', + comments: { + pageInfo: { hasNextPage: false, endCursor: null }, + nodes: [ + { + databaseId: db(1007), + id: cid('outdated1'), + body: 'Outdated thread comment at T+7', + createdAt: ts(7), + author: author('alice'), + diffHunk: DIFF_HUNK_ALPHA_OUTDATED, + reactionGroups: reactionGroups(), + }, + ], + }, + }, + { + id: tid('thread_file'), + isResolved: false, + isOutdated: false, + subjectType: 'FILE', + path: 'src/alpha.ts', + line: null, + startLine: null, + originalLine: null, + originalStartLine: null, + diffSide: null, + comments: { + pageInfo: { hasNextPage: false, endCursor: null }, + nodes: [ + { + databaseId: db(1008), + id: cid('file1'), + body: 'File-level comment at T+8', + createdAt: ts(8), + author: author('bob'), + reactionGroups: reactionGroups(), + }, + ], + }, + }, + { + id: tid('thread_resolved'), + isResolved: true, + isOutdated: false, + subjectType: 'LINE', + path: 'src/beta.ts', + line: 18, + startLine: 18, + originalLine: 18, + originalStartLine: 18, + diffSide: 'RIGHT', + comments: { + pageInfo: { hasNextPage: false, endCursor: null }, + nodes: [ + { + databaseId: db(1009), + id: cid('resolved1'), + body: 'Resolved thread comment at T+9', + createdAt: ts(9), + author: author('carol'), + diffHunk: DIFF_HUNK_BETA, reactionGroups: reactionGroups(), }, ], @@ -157,19 +288,27 @@ const FIXTURES = { // GraphQL pullRequest.comments nodes. REST issues/{n}/comments is intentionally // not served — S2 reads conversation comments over GraphQL only. conversationComments: [ - conversationComment(2002, 'dave', 'Conversation comment at T+2', 2), - conversationComment(2003, 'erin', 'Conversation comment at T+3', 3), - conversationComment(2006, 'frank', 'Conversation comment at T+6', 6), + conversationComment(db(2002), 'dave', 'Conversation comment at T+2', 2, s, { + THUMBS_UP: { count: 1, viewerHasReacted: false }, + }), + conversationComment(db(2003), 'erin', 'Conversation comment at T+3', 3, s), + conversationComment(db(2006), 'frank', 'Conversation comment at T+6', 6, s), ], - }, + }; +} + +/** @type {Record} */ +const FIXTURES = { + 'kilo-stub/discussion-mixed/1': buildMixedFixture(1), + 'kilo-stub/discussion-mixed/11': buildMixedFixture(11), 'kilo-stub/discussion-conversation-only/2': { title: 'Conversation-only fixture', body: 'PR body for conversation-only fixture.', files: stubFiles(), threads: [], conversationComments: [ - conversationComment(3001, 'dave', 'Only conversation comment one', 2), - conversationComment(3002, 'erin', 'Only conversation comment two', 6), + conversationComment(3001, 'dave', 'Only conversation comment one', 2, '2'), + conversationComment(3002, 'erin', 'Only conversation comment two', 6, '2'), ], }, 'kilo-stub/discussion-empty/3': { @@ -201,6 +340,48 @@ function getFixture(owner, repo, number) { return FIXTURES[fixtureKey(owner, repo, number)] ?? null; } +/** Walk all fixtures for a thread id (mutations carry no owner/repo/number). */ +function findThreadById(threadId) { + for (const fx of Object.values(FIXTURES)) { + const threads = fx.threads ?? []; + for (const thread of threads) { + if (thread.id === threadId) return thread; + } + } + return null; +} + +/** + * Walk all fixtures for a comment node id — review-comment nodes and + * conversation-comment nodes. + */ +function findCommentById(subjectId) { + for (const fx of Object.values(FIXTURES)) { + for (const thread of fx.threads ?? []) { + for (const node of thread.comments?.nodes ?? []) { + if (node.id === subjectId) return node; + } + } + for (const node of fx.conversationComments ?? []) { + if (node.id === subjectId) return node; + } + } + return null; +} + +function flipReaction(node, content, add) { + const groups = node.reactionGroups ?? []; + const group = groups.find(g => g.content === content); + if (!group) return; + if (add) { + group.viewerHasReacted = true; + group.reactors.totalCount = (group.reactors?.totalCount ?? 0) + 1; + } else { + group.viewerHasReacted = false; + group.reactors.totalCount = Math.max(0, (group.reactors?.totalCount ?? 0) - 1); + } +} + function restPull(owner, repo, number, fx) { const full = `${owner}/${repo}`; const sha = 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'; @@ -390,6 +571,74 @@ function handleGraphql(body, res) { }); } + // Stateful mutations — resolve target by scanning all fixtures (ids are unique + // across platform keys). Mutate fixture objects in place for process lifetime. + if (op === 'ResolveThread') { + const threadId = variables.input?.threadId; + const thread = threadId ? findThreadById(threadId) : null; + if (!thread) { + return json(res, 200, { data: { resolveReviewThread: null } }); + } + thread.isResolved = true; + return json(res, 200, { + data: { + resolveReviewThread: { + thread: { id: thread.id, isResolved: true }, + }, + }, + }); + } + + if (op === 'UnresolveThread') { + const threadId = variables.input?.threadId; + const thread = threadId ? findThreadById(threadId) : null; + if (!thread) { + return json(res, 200, { data: { unresolveReviewThread: null } }); + } + thread.isResolved = false; + return json(res, 200, { + data: { + unresolveReviewThread: { + thread: { id: thread.id, isResolved: false }, + }, + }, + }); + } + + if (op === 'AddReaction') { + const subjectId = variables.input?.subjectId; + const content = variables.input?.content; + const node = subjectId ? findCommentById(subjectId) : null; + if (!node || !content) { + return json(res, 200, { data: { addReaction: null } }); + } + flipReaction(node, content, true); + return json(res, 200, { + data: { + addReaction: { + reaction: { content }, + }, + }, + }); + } + + if (op === 'RemoveReaction') { + const subjectId = variables.input?.subjectId; + const content = variables.input?.content; + const node = subjectId ? findCommentById(subjectId) : null; + if (!node || !content) { + return json(res, 200, { data: { removeReaction: null } }); + } + flipReaction(node, content, false); + return json(res, 200, { + data: { + removeReaction: { + reaction: { content }, + }, + }, + }); + } + // Unknown GraphQL — non-401 so retry path does not rotate tokens. return json(res, 200, { data: null, From 54a71b34b40953402735f400a90d0eb21598d7ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Thu, 30 Jul 2026 13:16:37 +0200 Subject: [PATCH 04/12] feat(mobile): collapse PR comment reactions behind a picker Render only non-zero reaction buckets as pills and add a single SmilePlus button that opens a bottom-sheet picker with all eight emoji (Portal name pr-review-reactions, backdrop, BackHandler on Android). Split readOnly from disabled so conversation comments show read-only pills with no add icon while pending mutations only lock presses. --- .../pr-review/discussion/comment-row.tsx | 3 + .../discussion/pr-review-discussion-list.tsx | 6 +- .../discussion/reaction-picker-sheet.tsx | 118 +++++++++++++ .../pr-review/discussion/reactions-row.tsx | 157 ++++++++++-------- .../discussion/reaction-pills.test.ts | 54 ++++++ .../pr-review/discussion/reaction-pills.ts | 64 +++++++ 6 files changed, 325 insertions(+), 77 deletions(-) create mode 100644 apps/mobile/src/components/pr-review/discussion/reaction-picker-sheet.tsx create mode 100644 apps/mobile/src/lib/pr-review/discussion/reaction-pills.test.ts create mode 100644 apps/mobile/src/lib/pr-review/discussion/reaction-pills.ts diff --git a/apps/mobile/src/components/pr-review/discussion/comment-row.tsx b/apps/mobile/src/components/pr-review/discussion/comment-row.tsx index 00effc3ebac..7282e388559 100644 --- a/apps/mobile/src/components/pr-review/discussion/comment-row.tsx +++ b/apps/mobile/src/components/pr-review/discussion/comment-row.tsx @@ -25,12 +25,14 @@ type CommentRowProps = { readonly comment: ReviewComment; readonly onToggleReaction: (content: ReviewReactionContent) => void; readonly reactionsDisabled?: boolean; + readonly readOnly?: boolean; }; export function CommentRow({ comment, onToggleReaction, reactionsDisabled, + readOnly, }: Readonly) { const authorName = selectCommentAuthorName(comment.author); const timestamp = parseTimestamp(comment.createdAt); @@ -61,6 +63,7 @@ export function CommentRow({ reactions={comment.reactions} onToggle={onToggleReaction} disabled={reactionsDisabled} + readOnly={readOnly} /> ); diff --git a/apps/mobile/src/components/pr-review/discussion/pr-review-discussion-list.tsx b/apps/mobile/src/components/pr-review/discussion/pr-review-discussion-list.tsx index 726364f913e..43571a3ca49 100644 --- a/apps/mobile/src/components/pr-review/discussion/pr-review-discussion-list.tsx +++ b/apps/mobile/src/components/pr-review/discussion/pr-review-discussion-list.tsx @@ -70,11 +70,7 @@ export function PrReviewDiscussionList({ return ( - + ); diff --git a/apps/mobile/src/components/pr-review/discussion/reaction-picker-sheet.tsx b/apps/mobile/src/components/pr-review/discussion/reaction-picker-sheet.tsx new file mode 100644 index 00000000000..347befaed84 --- /dev/null +++ b/apps/mobile/src/components/pr-review/discussion/reaction-picker-sheet.tsx @@ -0,0 +1,118 @@ +// Bottom-sheet picker for GitHub's 8 review-comment reactions. +// Pattern-copied from kilo-chat's message-reaction-picker-sheet +// (Portal + backdrop fade + slide-up card + Android BackHandler). +// Portal name is unique so chat and PR-review sheets never clobber +// each other when both trees mount. + +import { Portal } from '@rn-primitives/portal'; +import { X } from 'lucide-react-native'; +import { useEffect } from 'react'; +import { BackHandler, Pressable, View } from 'react-native'; +import Animated, { FadeIn, FadeOut, SlideInDown, SlideOutDown } from 'react-native-reanimated'; +import { useSafeAreaInsets } from 'react-native-safe-area-context'; + +import { Text } from '@/components/ui/text'; +import { useThemeColors } from '@/lib/hooks/use-theme-colors'; +import { REACTION_EMOJI, REACTION_LABEL } from '@/lib/pr-review/discussion/reaction-pills'; +import { + REVIEW_REACTION_CONTENTS, + type ReviewReactionContent, +} from '@/lib/pr-review/discussion/review-discussion-types'; +import { cn } from '@/lib/utils'; + +type ReactionPickerSheetProps = { + readonly visible: boolean; + readonly reactions: readonly { + readonly content: string; + readonly count: number; + readonly viewerHasReacted: boolean; + }[]; + readonly onClose: () => void; + readonly onPick: (content: ReviewReactionContent) => void; +}; + +export function ReactionPickerSheet({ + visible, + reactions, + onClose, + onPick, +}: Readonly) { + const colors = useThemeColors(); + const insets = useSafeAreaInsets(); + + useEffect(() => { + if (!visible) { + return undefined; + } + const subscription = BackHandler.addEventListener('hardwareBackPress', () => { + onClose(); + return true; + }); + return () => { + subscription.remove(); + }; + }, [visible, onClose]); + + if (!visible) { + return null; + } + + const reacted = new Set(); + for (const r of reactions) { + if (r.viewerHasReacted) { + reacted.add(r.content); + } + } + + return ( + + + + + + Reactions + + + + + + {REVIEW_REACTION_CONTENTS.map(content => { + const isReacted = reacted.has(content); + return ( + { + onPick(content); + }} + > + {REACTION_EMOJI[content]} + + ); + })} + + + + + ); +} diff --git a/apps/mobile/src/components/pr-review/discussion/reactions-row.tsx b/apps/mobile/src/components/pr-review/discussion/reactions-row.tsx index 43a2f0d2d5f..c9ded74f626 100644 --- a/apps/mobile/src/components/pr-review/discussion/reactions-row.tsx +++ b/apps/mobile/src/components/pr-review/discussion/reactions-row.tsx @@ -1,49 +1,32 @@ // Reactions row for a single review comment. // -// GitHub's review-comment reactions are a fixed set of 8 emoji -// (`THUMBS_UP, THUMBS_DOWN, LAUGH, HOORAY, CONFUSED, HEART, -// ROCKET, EYES`). Each one is rendered as a small pill that shows -// the current count when > 0 and a darker fill when the viewer -// has already reacted. +// Renders non-zero known reaction buckets as pills, plus one +// smiley-plus "Add reaction" control that opens a picker sheet of +// all 8 GitHub review reactions. Tapping a pill or picking from the +// sheet toggles via `onToggle`; the optimistic cache reducer flips +// count + fill in the same frame. // -// Tapping a pill toggles: if the viewer has reacted, fire -// `removeReaction`; otherwise `addReaction`. The optimistic cache -// reducer (`applyReactionToggle`) updates the row instantly, so the -// count + fill flip in the same frame as the tap. -// -// Disabled state is exposed for callers that want to lock the row -// during the mutation's pending phase (rare — the optimistic update -// makes the row look responsive; the hook will still rollback on -// error). +// `disabled` locks presses during a pending mutation but keeps the +// add icon mounted (no flicker). `readOnly` (conversation comments) +// makes pills non-pressable and hides the add icon entirely; a +// zero-pill read-only row renders null. import * as Haptics from 'expo-haptics'; +import { SmilePlus } from 'lucide-react-native'; +import { useState } from 'react'; import { Pressable, View } from 'react-native'; +import { ReactionPickerSheet } from '@/components/pr-review/discussion/reaction-picker-sheet'; import { Text } from '@/components/ui/text'; -import { - REVIEW_REACTION_CONTENTS, - type ReviewReactionContent, -} from '@/lib/pr-review/discussion/review-discussion-types'; +import { useThemeColors } from '@/lib/hooks/use-theme-colors'; +import { REACTION_EMOJI, selectReactionPills } from '@/lib/pr-review/discussion/reaction-pills'; +import { type ReviewReactionContent } from '@/lib/pr-review/discussion/review-discussion-types'; import { cn } from '@/lib/utils'; -// Map each GitHub reaction content to the emoji that GitHub itself -// renders in its UI. Kept inline (not in a shared emoji module) so -// the discussion tab stays a self-contained slice. -const REACTION_EMOJI: Record = { - THUMBS_UP: '👍', - THUMBS_DOWN: '👎', - LAUGH: '😄', - HOORAY: '🎉', - CONFUSED: '😕', - HEART: '❤️', - ROCKET: '🚀', - EYES: '👀', -}; - type ReactionsRowProps = { // Raw reactions from the DTO — `content` is a plain string (GitHub can - // return content outside the 8 emoji). We index by string and only render - // + toggle the fixed 8 known reactions. + // return content outside the 8 emoji). We only render known contents + // with count > 0. readonly reactions: readonly { readonly content: string; readonly count: number; @@ -51,43 +34,75 @@ type ReactionsRowProps = { }[]; readonly onToggle: (content: ReviewReactionContent) => void; readonly disabled?: boolean; + readonly readOnly?: boolean; }; -export function ReactionsRow({ reactions, onToggle, disabled }: Readonly) { - // Index existing reactions by content for O(1) lookup. Missing - // reactions render as an empty pill (no count) so the user can - // discover the full set. - const byContent = new Map(); - for (const r of reactions) { - byContent.set(r.content, { count: r.count, viewerHasReacted: r.viewerHasReacted }); +export function ReactionsRow({ + reactions, + onToggle, + disabled, + readOnly, +}: Readonly) { + const colors = useThemeColors(); + const [pickerOpen, setPickerOpen] = useState(false); + const pills = selectReactionPills(reactions); + const isDisabled = Boolean(disabled); + const isReadOnly = Boolean(readOnly); + const pillDisabled = isDisabled || isReadOnly; + + if (pills.length === 0 && isReadOnly) { + return null; } + return ( - - {REVIEW_REACTION_CONTENTS.map(content => { - const existing = byContent.get(content); - const count = existing?.count ?? 0; - const reacted = existing?.viewerHasReacted ?? false; - return ( - { - void Haptics.selectionAsync(); - onToggle(content); - }} - /> - ); - })} + + {pills.map(pill => ( + { + void Haptics.selectionAsync(); + onToggle(pill.content); + }} + /> + ))} + {isReadOnly ? null : ( + { + void Haptics.selectionAsync(); + setPickerOpen(true); + }} + className={cn( + 'rounded-full border border-border bg-card p-1.5', + isDisabled && 'opacity-50' + )} + > + + + )} + { + setPickerOpen(false); + }} + onPick={content => { + void Haptics.selectionAsync(); + onToggle(content); + setPickerOpen(false); + }} + /> ); } type ReactionPillProps = { - readonly content: ReviewReactionContent; readonly emoji: string; readonly count: number; readonly viewerHasReacted: boolean; @@ -118,16 +133,14 @@ function ReactionPill({ )} > {emoji} - {count > 0 ? ( - - {count} - - ) : null} + + {count} + ); } diff --git a/apps/mobile/src/lib/pr-review/discussion/reaction-pills.test.ts b/apps/mobile/src/lib/pr-review/discussion/reaction-pills.test.ts new file mode 100644 index 00000000000..4abd4b65429 --- /dev/null +++ b/apps/mobile/src/lib/pr-review/discussion/reaction-pills.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from 'vitest'; + +import { REACTION_EMOJI, REACTION_LABEL, selectReactionPills } from './reaction-pills'; +import { REVIEW_REACTION_CONTENTS } from './review-discussion-types'; + +describe('selectReactionPills', () => { + it('filters zero-count buckets', () => { + expect( + selectReactionPills([ + { content: 'THUMBS_UP', count: 2, viewerHasReacted: false }, + { content: 'HEART', count: 0, viewerHasReacted: true }, + ]) + ).toEqual([{ content: 'THUMBS_UP', count: 2, viewerHasReacted: false }]); + }); + + it('filters unknown contents', () => { + expect( + selectReactionPills([ + { content: 'THUMBS_UP', count: 1, viewerHasReacted: false }, + { content: 'PARTY_PARROT', count: 9, viewerHasReacted: true }, + ]) + ).toEqual([{ content: 'THUMBS_UP', count: 1, viewerHasReacted: false }]); + }); + + it('preserves DTO order', () => { + expect( + selectReactionPills([ + { content: 'HEART', count: 1, viewerHasReacted: true }, + { content: 'THUMBS_UP', count: 2, viewerHasReacted: false }, + { content: 'ROCKET', count: 3, viewerHasReacted: false }, + ]) + ).toEqual([ + { content: 'HEART', count: 1, viewerHasReacted: true }, + { content: 'THUMBS_UP', count: 2, viewerHasReacted: false }, + { content: 'ROCKET', count: 3, viewerHasReacted: false }, + ]); + }); + + it('returns empty for an empty input', () => { + expect(selectReactionPills([])).toEqual([]); + }); +}); + +describe('REACTION_EMOJI and REACTION_LABEL', () => { + it('cover all 8 REVIEW_REACTION_CONTENTS', () => { + expect(REVIEW_REACTION_CONTENTS).toHaveLength(8); + for (const content of REVIEW_REACTION_CONTENTS) { + expect(REACTION_EMOJI[content]).toBeTruthy(); + expect(typeof REACTION_EMOJI[content]).toBe('string'); + expect(REACTION_LABEL[content]).toBeTruthy(); + expect(typeof REACTION_LABEL[content]).toBe('string'); + } + }); +}); diff --git a/apps/mobile/src/lib/pr-review/discussion/reaction-pills.ts b/apps/mobile/src/lib/pr-review/discussion/reaction-pills.ts new file mode 100644 index 00000000000..3a95a0c85de --- /dev/null +++ b/apps/mobile/src/lib/pr-review/discussion/reaction-pills.ts @@ -0,0 +1,64 @@ +// Pure reaction-pill selection and emoji/label maps for the Discussion tab. +// +// Single source for GitHub's 8 review-reaction contents: emoji glyphs +// (pill + picker) and human a11y labels. `selectReactionPills` keeps +// only known contents with count > 0, preserving DTO order. + +import { + REVIEW_REACTION_CONTENTS, + type ReviewReactionContent, +} from '@/lib/pr-review/discussion/review-discussion-types'; + +export const REACTION_EMOJI: Record = { + THUMBS_UP: '👍', + THUMBS_DOWN: '👎', + LAUGH: '😄', + HOORAY: '🎉', + CONFUSED: '😕', + HEART: '❤️', + ROCKET: '🚀', + EYES: '👀', +}; + +export const REACTION_LABEL: Record = { + THUMBS_UP: 'Thumbs up', + THUMBS_DOWN: 'Thumbs down', + LAUGH: 'Laugh', + HOORAY: 'Hooray', + CONFUSED: 'Confused', + HEART: 'Heart', + ROCKET: 'Rocket', + EYES: 'Eyes', +}; + +const KNOWN_CONTENTS = new Set(REVIEW_REACTION_CONTENTS); + +export type ReactionPill = { + readonly content: ReviewReactionContent; + readonly count: number; + readonly viewerHasReacted: boolean; +}; + +/** + * From DTO reaction buckets, keep only the 8 known contents with + * `count > 0`, preserving input order. + */ +export function selectReactionPills( + reactions: readonly { + readonly content: string; + readonly count: number; + readonly viewerHasReacted: boolean; + }[] +): ReactionPill[] { + const pills: ReactionPill[] = []; + for (const r of reactions) { + if (r.count > 0 && KNOWN_CONTENTS.has(r.content)) { + pills.push({ + content: r.content as ReviewReactionContent, + count: r.count, + viewerHasReacted: r.viewerHasReacted, + }); + } + } + return pills; +} From e7ba1dad7f3a499df556cb8505c7df608e8d2770 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Thu, 30 Jul 2026 13:16:37 +0200 Subject: [PATCH 05/12] fix(mobile): reserve PR entry helper slot to stop layout shift Mount one stable Text between the input row and Open button; swap only its copy (message or non-breaking-space placeholder) and color token so helper messages appear and clear without moving surrounding controls. --- .../pr-review/pr-review-entry-screen.tsx | 24 ++++++++++++------- .../src/lib/pr-review/pr-link-helper-slot.ts | 5 ++-- 2 files changed, 18 insertions(+), 11 deletions(-) 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 01f16cfd3c0..3c1238900eb 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 @@ -28,6 +28,7 @@ import { import { consumePrLinkInputEcho, pushPrLinkInputEcho } from '@/lib/pr-review/pr-link-input-echo'; import { decidePrLinkPaste } from '@/lib/pr-review/pr-link-paste'; import { getRecentPrs, type RecentPr, upsertRecentPr } from '@/lib/pr-review/recent-prs'; +import { cn } from '@/lib/utils'; const URL_PLACEHOLDER = 'https://github.com/owner/repo/pull/123'; @@ -127,16 +128,13 @@ export function PrReviewEntryScreen() { }); const showClearButton = selectPrLinkClearButtonVisible({ hasInput }); const isInvalid = helperMessage === 'invalid'; - - let helperContent: ReactNode = null; + // Stable always-mounted slot (D6): same Text line box every state — only + // string + color token change. NBSP keeps the line box at every font scale. + let helperSlotCopy = '\u00A0'; if (slotState === 'invalid') { - helperContent = {PR_LINK_HELPER_INVALID_COPY}; + helperSlotCopy = PR_LINK_HELPER_INVALID_COPY; } else if (slotState === 'clipboard-empty') { - helperContent = ( - - {PR_LINK_HELPER_CLIPBOARD_EMPTY_COPY} - - ); + helperSlotCopy = PR_LINK_HELPER_CLIPBOARD_EMPTY_COPY; } let recentsBody: ReactNode = null; @@ -288,7 +286,15 @@ export function PrReviewEntryScreen() { - {helperContent} + + {helperSlotCopy} +