Skip to content

feat(mobile): add bottom comment CTA and conversation comment composer to PR review Discussion tab - #6023

Merged
iscekic merged 1 commit into
mainfrom
kwf/in-the-mobile-app-add-a-bott-de54
Sep 10, 2026
Merged

feat(mobile): add bottom comment CTA and conversation comment composer to PR review Discussion tab#6023
iscekic merged 1 commit into
mainfrom
kwf/in-the-mobile-app-add-a-bott-de54

Conversation

@iscekic

@iscekic iscekic commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Changelog for users

  • The PR review Discussion tab gains a bottom "Comment on this pull request" bar on both the loaded list and the empty state; tapping it opens an "Add comment" sheet.
  • A comment sent from the sheet lands as a regular PR conversation comment — not a review-thread reply and not part of a submitted review — and appears in the discussion right after success, with a success haptic and a screen-reader announcement.
  • Empty comments are blocked with an inline "Comment body cannot be empty." error, and duplicate submissions are prevented: Comment disables while posting, and a repeated submission of the same comment replays the posted result instead of writing to GitHub twice.
  • The draft is kept per account and PR: it survives dismissing the sheet and a failed post, and is cleared only on a successful post or a confirmed discard; leaving with text (Cancel, header close, or hardware back) asks "Discard comment?" first, and Keep editing preserves the text.
  • A failed submission keeps the draft and shows actionable inline copy — "Could not post comment.", a no-permission message, or "The pull request may have changed" — with a matching toast, and Comment stays enabled for a retry; while confirmed offline, submit fails at once instead of spinning to a deadline.
  • The bottom bar stays visible and tappable above the open keyboard and clears the device safe area; focusing a thread's reply input scrolls that row above the keyboard-lifted bar, and grabbing the list cancels that scroll.
  • Existing review-thread replies still post into their threads, and now fail more usefully: reply errors show retryable copy instead of raw provider text, and a reply submitted while confirmed offline fails at once without a request.
  • The offline banner no longer covers the PR review screen's header title, and a stale offline banner clears at once when the radio is back and the app's own connectivity probe succeeds.

Changelog for maintainers

  • Start at PrConversationCommentComposer (mounted by the new conversation-comment formSheet route registered in the [number] layout) and PrCommentCta in pr-review-discussion-tab.tsx: the bar renders only on the happy-list and empty views; the loading skeleton and every terminal/error state are unchanged.
  • New backend contract: githubPrReview.addIssueComment writes via Octokit issues.createComment (the issues endpoint, not the review-comment endpoints), asserts UGC terms before any ledger row or GitHub write, and is ledger-deduped under the new add_pr_comment intent — a same-operationKey retry replays the canonical result with a single GitHub write, and the comment body stays out of the settle outbox event.
  • The add_pr_comment resource fingerprint ({"resource":[owner,repo,number],"body":...}) is pinned in app-shared tests; it is the 30-day dedupe identity, so any change to it rotates every in-flight key.
  • Client guards: local empty-body validation and submit disabled while pending; only the ledger persistence-failure marker blocks a retry, while a server bad-request keeps Comment enabled (the key rotation is ledger-safe), so a failed post never dead-ends the composer. A confirmed discard waits for the draft clear to settle and stays on the sheet with a retryable error if the clear fails.
  • invalidateDiscussionCaches is now fire-and-forget in onSettled for every discussion mutation (reply, resolve/unresolve, reactions): an awaited invalidation let an offline refetch pin the UI past the new 15 s withUiDeadline that bounds the add-comment mutation.
  • ReplyInput no longer surfaces raw provider errors (retryable copy, plus verify-before-retrying copy for the ambiguous ledger marker) and gains the same confirmed-offline local submit gate via the new getCommittedConnectivityStatus snapshot; unknown connectivity never blocks.
  • Offline fixes the flow exposed: committed-offline with the radio back up but reachability unknown now fires the app's own probe immediately (the banner used to stay painted forever), and OFFLINE_BANNER_HEIGHT (36) is exported and reserved above the PR review header while offline — keep the painted row height in sync with the constant.
  • Review the two subtlest pieces against their own suites: useReplyFocusScroll (one scroll per reply focus, anchored on the committed viewport height change; Android and iOS commit the CTA lift in opposite orders, and a user drag wins) and the keyboardLift focus gating of the CTA bar (a foreign sheet's keyboard must not shrink the list viewport behind the sheet).

E2E proof

Recording of the verified flow (waits trimmed)

e3-retry-success.mp4.trim.mp4

[e1] ux-check: Submit with the network unavailable — e2e-mobile-app/e1-retry-success.png

[e1] cta-opens-composer: the Discussion tab shows the bottom Comment CTA and tapping it opens the comment composer (fixture kilo-stub/discussion-mixed#1) — prior/e1-discussion.png

[e1] ux-check: Submit with the network unavailable — prior/e3-inline-error.png

[e1] discard-gate-preserves-then-clears-draft: back with text asks before discarding; Keep editing preserves the text; confirmed discard clears it for the next open — prior/e1-discard-dialog.png

[e1] discard-gate-preserves-then-clears-draft: back with text asks before discarding; Keep editing preserves the text; confirmed discard clears it for the next open — prior/e1-keep-editing.png

E2E proof — log excerpts

[e1] ux-check: Submit with the network unavailable -> pass :: android emulator-5556: network-unavailable submit shows retryable inline 'Could not post comment.' with draft e1retryable and enabled Comment (e1-inline-error.png, e1-after-radio.png, e1-reopen-draft.png); same-HEAD prior/e3-inline-error.png (2026-09-09 11:32) shows that error under the No internet connection banner; retry closes the sheet (e1-retry-success.png) and github-stub logs POST /repos/kilo-stub/discussion-mixed/issues/1/comments at 2026-09-09T18:24:19.076Z (e1-dispatch.log). Unsupported generated toast demand: useful persistent inline error is the owner/design feedback; sonner toasts render behind formSheets so a toast would be invisible here. UX-PREEXISTING: [pre-existing] Add com
/home/igor_kilocode_ai/.local/share/kwf/sections/in-the-mobile-app-add-a-bott-de54/e2e-mobile-app/e1-dispatch.log
e1 retry after network restore (android emulator-5556)
github-stub received the issue comment POST:
{"t":"2026-09-09T18:24:19.076Z","method":"POST","path":"/repos/kilo-stub/discussion-mixed/issues/1/comments","owner":"kilo-stub","repo":"discussion-mixed","number":"1"}
  • proved live: cta-accessible-with-keyboard: the bottom CTA stays visible and tappable above the keyboard while a thread reply input is focused — Android: with Reply body focused, 'Comment on this pull request' sits fully above the keyboard and the Reply control stays visible (e8.png).
  • proved live: cta-on-empty-discussion-posts-first-comment: the CTA renders on the empty state (fixture kilo-stub/discussion-empty#3) and the first posted comment moves the tab from empty to the list showing it. — Android: CTA on empty kilo-stub/discussion-empty#3 (e5-empty.png); after submit, Discussion badge 1 and e2eFirstComment Just now replaced the empty state (e5.png / e5-after-submit.png).
  • proved live: cta-opens-composer: the Discussion tab shows the bottom Comment CTA and tapping it opens the comment composer (fixture kilo-stub/discussion-mixed#1) — Android: Discussion CTA visible (e1-discussion.png) and tap opened the Add comment sheet with KILO-STUB/DISCUSSION-MIXED#1 (e1.png).
  • proved live: discard-gate-preserves-then-clears-draft: back with text asks before discarding; Keep editing preserves the text; confirmed discard clears it for the next open — android emulator-5556: Cancel with e1draftkeep showed Discard comment? (e1-discard-dialog.png); Keep editing left e1draftkeep in the field (e1-keep-editing.png); confirmed DISCARD then reopen showed empty Leave a comment (e1-reopen-cleared.png).
  • proved live: empty-body-blocked: submitting with an empty body shows the inline error and the composer stays open, nothing posts — Android: empty submit showed inline 'Comment body cannot be empty.', composer stayed open, Comment disabled (e2.png); stub had no issues comments POST before the later e3 submit.
  • proved live: post-comment-appears-in-discussion: typing a body and submitting posts a regular PR conversation comment that renders in the discussion. — Android: e2eTopLevelComment rendered as a top-level conversation card (e3.png); e3-dispatch.log quotes POST path "/repos/kilo-stub/discussion-mixed/issues/1/comments" (stub log has no request body field — harness gap).
  • proved live: post-failure-preserves-draft-and-shows-retryable-error: a failed post keeps the typed text, shows the inline Could not post comment error, and re-enables submit for retry — needs:fault (make the… — android emulator-5556: with No internet connection banner painted, Comment showed inline Could not post comment. with e2retryable still typed and Comment re-enabled (e2-could-not-post.png, e2-retry-enabled.png). e2-dispatch.log: 'NO POST /repos/.../issues/1/comments (gate blocked before GitHub).' Stub 500 control is not in e2e-index; used confirmed-offline gate after fault.sh nextjs reported stale port 7900.
  • proved live: thread-reply-regression: an existing review thread's inline reply still posts into its thread (fixture kilo-stub/discussion-mixed#1, stub already serves the replies POST — decisive stub log line POST… — Android: e2eThreadReply rendered inside the outdated thread under alice (e7.png); e7-dispatch.log quotes POST path "/repos/kilo-stub/discussion-mixed/pulls/1/comments/1007/replies".
  • proved live: ux-check: Discussion tab empty state: the CTA bar renders below the empty state without overlapping the 'Review files' action
  • proved live: ux-check: Discussion tab with loaded list: the 'Comment on this pull request' bar renders above the home-indicator safe area, does not overlap the last comment row, and the gap between the last row…
  • proved live: ux-check: Existing review-thread reply still posts into its thread after the change
  • proved live: ux-check: Post a comment on a real PR: the sheet closes, success feedback fires, and the comment appears in the Discussion tab as a top-level PR conversation comment, not nested in a review thread or…
  • proved live: ux-check: Submit with the network unavailable: a retryable inline error plus toast appears, the typed text is still present when reopening the sheet, and a retry succeeds — android emulator-5556: network-unavailable submit shows retryable inline 'Could not post comment.' with draft e1retryable and enabled Comment (e1-inline-error.png, e1-after-radio.png, e1-reopen-draft.png); same-HEAD prior/e3-inline-error.png (2026-09-09 11:32) shows that error under the No internet connection banner; retry closes the sheet (e1-retry-success.png) and github-stub logs POST /repos/kilo-stub/discussion-mixed/issues/1/comments at 2026-09-09T18:24:19.076Z (e1-dispatch.log). Unsupported generated toast demand: useful persistent inline error is the owner/design feedback; sonner…
  • proved live: ux-check: Tap Comment with an empty body: inline 'Comment body cannot be empty.' appears with no network submission; typing clears the error and re-enables the button
  • proved live: ux-check: Tap the CTA: the comment formSheet opens at full detent with title 'Add comment', eyebrow owner/repo#N, placeholder body field, and Comment/Cancel visible without scrolling
  • proved live: ux-check: With the keyboard open on an inline thread reply, the bottom CTA bar remains visible above the keyboard; with the keyboard open in the composer, the Comment/Cancel buttons remain visible…
Owner request

In the mobile app, add a bottom call to action on the PR review page's Discussion tab to add a regular PR comment.
The comment must belong to the PR discussion, not a review thread or a submitted review.
Use the existing comment API, composer patterns, and provider permissions where available.
Preserve existing review-thread replies.
Show the posted comment in the discussion after success.
Prevent empty comments and duplicate submissions.
Preserve the draft and show a useful error if submission fails.
Keep the bottom action accessible with the keyboard open and respect the device safe area.

Verify the flow locally in the mobile app against a real PR.
Confirm the posted comment is a regular PR discussion comment and existing thread replies still work.
Include a recording, or screenshots if recording is unavailable, in the PR body.
Do not commit fixtures, E2E-only product code, test hooks, or test-only runtime flags.
Deliver a reviewed, CI-green PR and assign it to @iscekic.
Do not merge the PR.

Follow-ups (not changed here)

  • 01-settle-app-FAIL.png — Blank white screen with no content, loading indicator, or error state.
  • e6.png — After the failed post the filled draft remains but Comment stays disabled under the non-retryable “This comment can't be posted. The pull request may have changed.” error, so retry is blocked.
  • e8.png — Reply submit button is not visible above the keyboard-lifted Comment CTA while the inline reply is focused
  • Add comment sheet — in-flight submit while nextjs was down then radio-off stayed on a spinner with Cancel disabled and no retryable error (e1-loading.png) until the request finished after radio restore; not caused by the 1-line import whitespace deletion.

@iscekic
iscekic marked this pull request as draft September 9, 2026 18:44
@kilo-code-bot

kilo-code-bot Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (14 files)
  • apps/mobile/src/components/offline-banner.mounted.test-helpers.tsx
  • apps/mobile/src/components/offline-banner.mounted.test.tsx
  • apps/mobile/src/components/pr-review/discussion/pr-comment-cta.test.tsx
  • apps/mobile/src/components/pr-review/discussion/pr-conversation-comment-composer.test-helpers.ts
  • apps/mobile/src/components/pr-review/discussion/pr-conversation-comment-composer.test.tsx
  • apps/mobile/src/components/pr-review/discussion/pr-conversation-comment-composer.tsx
  • apps/mobile/src/components/pr-review/discussion/reply-input.test.ts
  • apps/mobile/src/components/pr-review/pr-review-discussion-tab.test.tsx
  • apps/mobile/src/components/pr-review/pr-review-discussion-tab.tsx
  • apps/mobile/src/components/pr-review/pr-review-screen.tsx
  • apps/mobile/src/lib/offline-banner-state.test-helpers.ts
  • apps/mobile/src/lib/offline-banner-state.test.ts
  • apps/mobile/src/lib/persist/drafts.ts
  • apps/mobile/src/lib/pr-review/discussion/use-review-discussion-mutations.test.ts
Previous Review Summary (commit ffcb823)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit ffcb823)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (35 files)
  • apps/mobile/src/app/(app)/pr-review/[owner]/[repo]/[number]/_layout.tsx
  • apps/mobile/src/app/(app)/pr-review/[owner]/[repo]/[number]/conversation-comment.tsx
  • apps/mobile/src/components/offline-banner.tsx
  • apps/mobile/src/components/offline-banner.mounted.test.tsx
  • apps/mobile/src/components/pr-review/discussion/discussion-thread.tsx
  • apps/mobile/src/components/pr-review/discussion/pr-comment-cta.tsx
  • apps/mobile/src/components/pr-review/discussion/pr-comment-cta.test.tsx
  • apps/mobile/src/components/pr-review/discussion/pr-conversation-comment-composer.tsx
  • apps/mobile/src/components/pr-review/discussion/pr-conversation-comment-composer.test.tsx
  • apps/mobile/src/components/pr-review/discussion/pr-conversation-comment-composer.test-helpers.ts
  • apps/mobile/src/components/pr-review/discussion/pr-review-discussion-list.tsx
  • apps/mobile/src/components/pr-review/discussion/reply-input.tsx
  • apps/mobile/src/components/pr-review/discussion/reply-input.test.ts
  • apps/mobile/src/components/pr-review/pr-review-discussion-tab.tsx
  • apps/mobile/src/components/pr-review/pr-review-discussion-tab.test.tsx
  • apps/mobile/src/components/pr-review/pr-review-screen.tsx
  • apps/mobile/src/components/pr-review/pr-review-screen.test.tsx
  • apps/mobile/src/i18n/label-reference.test.ts
  • apps/mobile/src/i18n/locales/en.json
  • apps/mobile/src/lib/hooks/use-offline-banner-state.ts
  • apps/mobile/src/lib/offline-banner-state.ts
  • apps/mobile/src/lib/offline-banner-state.test.ts
  • apps/mobile/src/lib/persist/drafts.ts
  • apps/mobile/src/lib/persist/drafts.test.ts
  • apps/mobile/src/lib/pr-review/discussion/use-reply-focus-scroll.ts
  • apps/mobile/src/lib/pr-review/discussion/use-reply-focus-scroll.test.ts
  • apps/mobile/src/lib/pr-review/discussion/use-review-discussion-mutations.ts
  • apps/mobile/src/lib/pr-review/discussion/use-review-discussion-mutations.test.ts
  • apps/mobile/src/lib/pr-review/merge/pr-operation-ledger.ts
  • apps/mobile/src/lib/pr-review/merge/pr-operation-ledger.test.ts
  • apps/web/src/routers/github-pr-review-router.ts
  • apps/web/src/routers/github-pr-review-router.test.ts
  • packages/app-shared/src/analytics/event-map.ts
  • packages/app-shared/src/pr-review/intent-fingerprint.ts
  • packages/app-shared/src/pr-review/intent-fingerprint.test.ts

Reviewed by grok-4.6 · Input: 112K · Output: 9.6K · Cached: 352.1K

Review guidance: REVIEW.md from base branch main

@iscekic
iscekic force-pushed the kwf/in-the-mobile-app-add-a-bott-de54 branch from 7530872 to b247acd Compare September 9, 2026 20:20
@iscekic
iscekic marked this pull request as ready for review September 9, 2026 20:44
@iscekic iscekic added the human-ready The PR is ready for human review. label Sep 9, 2026
@iscekic
iscekic enabled auto-merge (squash) September 9, 2026 23:07
@iscekic
iscekic merged commit a928aad into main Sep 10, 2026
55 checks passed
@iscekic
iscekic deleted the kwf/in-the-mobile-app-add-a-bott-de54 branch September 10, 2026 09:16
@iscekic iscekic self-assigned this Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

human-ready The PR is ready for human review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants