[#958] Active story badges + active-first trending sort#968
[#958] Active story badges + active-first trending sort#968realproject7 merged 4 commits intomainfrom
Conversation
…ding sort Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The active/completed status styling is close, but the badge placement does not match the issue requirements. The spec explicitly calls for the status badge to sit above the plot-count badge at the bottom of the book card; this patch renders it in the top badge row next to the genre instead.
Findings
- [medium] Status badge is placed in the wrong location on the story card. The issue mock and acceptance criteria call for
Active/Completed/Expiredto appear above the plot-count badge near the bottom of the cover, but the implementation adds the status badge to the top genre badge row. That changes the card hierarchy and does not match the requested layout.- File:
src/components/StoryCard.tsx:91 - Suggestion: Move the status badge into the bottom badge cluster, above or alongside the plot-count area per the issue mock, while keeping the genre badge at the top.
- File:
Decision
Requesting changes because the UI does not yet match the approved design for the story card badge placement.
…s.ts Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The new commit improves consistency by extracting shared story-status logic, but it does not resolve the blocking UI issue from the previous review. The status badge is still rendered in the top badge row instead of above the plot-count badge near the bottom of the card.
Findings
- [medium] The story-card status badge is still in the wrong location. Issue #958 specifies that
Active/Completed/Expiredshould appear above the plot-count badge near the bottom of the book card, but the implementation still places it next to the genre badge at the top.- File:
src/components/StoryCard.tsx:80 - Suggestion: Move the status badge into the bottom badge cluster so the card matches the approved layout.
- File:
Decision
Keeping this in request-changes because the previous design mismatch is still present after the re-review update.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The latest update fixes the blocking story-card layout issue by moving the status badge to the bottom of the card, above the plot-count badge, which matches the issue mock. The shared story-status utility also keeps the active/expired/completed logic consistent between ranking and card rendering.
Findings
- No blocking findings.
Decision
Approving because the previously-requested badge placement fix is now implemented and the PR satisfies the reviewed acceptance criteria for active badges, dimming, accent border, and active-first trending ordering.
…ory-status Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The follow-up commit resolves the remaining shared-constant cleanup by having DeadlineCountdown.tsx import DEADLINE_HOURS and DEADLINE_MS from lib/story-status.ts and re-export them for compatibility. That leaves the PR aligned with the reviewed requirements.
Findings
- No blocking findings.
Decision
Approving the updated PR because the remaining duplication concern is addressed and I do not see a new regression in the follow-up patch. Checks visible to me were Vercel-only at review time.
Summary
border-2 border-accent)opacity-60) with normal thin borderFixes #958
Test plan
🤖 Generated with Claude Code