Extract shared FeedbackReplyForm + FeedbackResolutionFooter - #836
Merged
Conversation
The review-feedback reply form and resolution footer were duplicated (~180 lines) between diff-annotations.tsx and reviews-sidebar.tsx. Extract both blocks into feedback-card-parts.tsx with a variant prop that preserves each surface's layout classes exactly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
selfcontained added a commit
that referenced
this pull request
Jul 27, 2026
- Extend loadScopedActivityEvents with an optional includeProjectDir mode (LEFT JOIN agents, COALESCE(ae.project_dir, a.cwd)) and switch handleWorkingTimeByProject to it, removing the hand-inlined copy of the in-range + DISTINCT ON boundary carry-in queries so the two can't drift. Existing real-DB boundary tests for /activity/stats, /activity/daily-status, and /activity/working-time-by-project cover all three consumers. - Add feedback-card-parts.test.tsx (16 tests) for the FeedbackReplyForm and FeedbackResolutionFooter shared components extracted in #836: reply trigger toggle, Cmd/Ctrl+Enter submit, Escape/Cancel, whitespace Send gating, pending disable + spinner placement, inline vs sidebar action-row layout, Dismiss/Mark fixed/Reopen resolution routing, and resolution banner states. Verified non-vacuous with a 10-mutation battery (10/10 caught). Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 27, 2026
selfcontained added a commit
that referenced
this pull request
Jul 27, 2026
…eadMessage (#840) The reviews sidebar and the inline diff annotations each carried their own copy of the thread-message bubble. The sidebar copy handled resolution/reopen state-change messages; the inline copy rendered content.body ?? "", so a body-less resolution message showed an empty bubble in the Changes tab. Move the state-change-aware version into feedback-card-parts.tsx (already shared by both surfaces since #836) and use it in both. Also align the inline grouping predicate with the sidebar's (includes message type) so a state change never merges into a preceding comment group. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Extracts the two review-feedback card blocks that were duplicated (~180 lines) between
diff-annotations.tsx(inline diff annotations) andreviews-sidebar.tsx(reviews sidebar) into a sharedfeedback-card-parts.tsx:FeedbackReplyForm— the animated reply form (Textarea with Cmd/Ctrl-Enter submit + Escape cancel, Cancel/Send buttons) and the Reply trigger button.FeedbackResolutionFooter— the resolution-note box plus the Reopen / Dismiss / Mark-fixed buttons with per-variant pending spinners.Both components are purely presentational — reply state stays in the parents so the draft-reply text still survives collapse/re-expand exactly as before. A
variant: "inline" | "sidebar"prop preserves each surface's layout classes verbatim (ml-auto … max-w-smwidth-capping in the wide diff view vs full-width in the narrow sidebar).Net -115 lines. Intentionally NOT included: consolidating
ThreadMessage/InlineThreadMessage— that is a visible behavior change (state-change message rendering) and is queued as its own backlog item.Why it's tech debt
Largest duplication in the repo per the 2026-07-21 audit; the two copies had already started drifting cosmetically (an
aria-labelpresent on one Send button but not the other — now unified, the only intentional delta: the inline Send button gainsaria-label="Send reply").Validation
pnpm run check, web vitest (415 passed),pnpm run finalize:web,pnpm run test:e2e(170 passed) all green.Queued next
Top of the tech-debt backlog:
InlineThreadMessagerenders state-change messages as empty bubbles (behavior-change consolidation, own PR), then the server/web media-table divergence.🤖 Generated with Claude Code