Skip to content

feat: recent activity modal with grouped history#242

Merged
pbrissaud merged 4 commits intomainfrom
feat/activity-log-modal
Mar 9, 2026
Merged

feat: recent activity modal with grouped history#242
pbrissaud merged 4 commits intomainfrom
feat/activity-log-modal

Conversation

@pbrissaud
Copy link
Member

Summary

  • Dashboard "Recent Activity" card now shows only 3 items (down from 5)
  • "View All Activity" button opens a dialog with the full history (up to 20 entries)
  • Dialog has a purple header displaying total XP and activity count
  • Entries are grouped by month with dividers inside the modal
  • Backend query limit increased from 5 → 20 to support the modal

Test plan

  • Dashboard shows exactly 3 activity items
  • "View All Activity" button opens the modal
  • Modal header shows correct total XP and activity count
  • Entries are grouped by month with labels
  • Modal close button (×) is visible on the purple header
  • Each entry links to its challenge page

🤖 Generated with Claude Code

- Limit dashboard preview to 3 items (was 5)
- Increase backend query limit to 20 for modal use
- Extract client component with "View All Activity" button that opens a dialog
- Dialog features a purple header with total XP + count, and timeline grouped by month

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@vercel
Copy link

vercel bot commented Mar 9, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
website Ready Ready Preview, Comment Mar 9, 2026 2:55pm

Request Review

@claude
Copy link

claude bot commented Mar 9, 2026

Claude finished @pbrissaud's task in 1m 27s —— View job


Code Review

  • Gather context
  • Read changed files
  • Analyze and provide feedback

Overall the implementation is clean and well-structured. The server component → client component split is a nice pattern. A few issues worth addressing:


Bug: Null challenge fields can break the UI

xpTransaction.ts uses a leftJoin on challenge, so challengeSlug and challengeTitle can both be null when a transaction isn't linked to a challenge. In ActivityItem:

  • Line 59href={/challenges/${activity.challengeSlug}} renders as /challenges/null
  • Line 62{activity.challengeTitle} displays literal "null"

The component should guard against these null values, either by conditionally rendering the link/title, or by filtering out non-challenge transactions upstream. Fix this →


UX: Modal header XP total is misleading

components/dashboard-recent-gains-client.tsx line 95:

const totalXp = recentGains.reduce((sum, g) => sum + g.xpAmount, 0);

recentGains is capped at 20 entries (from the backend limit), but the modal header displays this as +{totalXp} XP alongside an activity count. A user with more than 20 transactions will see a partial total without any indication. Either:

  • Rename/clarify to "Recent XP" or "+{totalXp} XP earned recently", or
  • Fetch the true cumulative XP total separately (e.g. from the existing user stats)

Code quality: Fragile Dialog close-button CSS selector

components/dashboard-recent-gains-client.tsx lines 122–124:

// make the built-in close button visible over the purple header
"[&>button:last-child]:text-primary-foreground [&>button:last-child]:opacity-80 [&>button:last-child]:hover:opacity-100",

This relies on the internal DOM structure of shadcn/ui's DialogContent. If the component is updated, these styles will silently break. A more robust approach would be to pass hideCloseButton to DialogContent and add an explicit DialogClose button inside the purple header div. Fix this →


Minor: "View All Activity" shown with 0 items

If the user has no activity yet, the button still renders and opens an empty modal. Consider hiding the button (or the whole section) when recentGains.length === 0.


Nit: Magic number for preview count

Line 94: const preview = recentGains.slice(0, 3) — a named constant like PREVIEW_COUNT = 3 would make the intent clearer and easier to adjust.


Summary

Severity Issue
🔴 Bug Null challengeSlug/challengeTitle produce broken links and "null" text
🟡 UX Modal XP total is a partial sum (last 20 only), not a true cumulative total
🟡 Quality Fragile [&>button:last-child] selector for close button styling
🟢 Minor "View All Activity" visible with empty state
🟢 Nit Magic number 3 for preview slice

…dialog close button

- Guard against null challengeSlug/challengeTitle from leftJoin
- Rename totalXp to recentXp and label it "+X XP recently" in modal header
- Add hideCloseButton prop to DialogContent; use explicit DialogClose in purple header
- Hide "View All Activity" button and modal when recentGains is empty
- Extract PREVIEW_COUNT constant (was magic number 3)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude
Copy link

claude bot commented Mar 9, 2026

Claude finished @pbrissaud's task in 1m 26s —— View job


Code Review (Updated)

  • Gather context
  • Read changed files
  • Analyze fixes from previous review
  • Provide updated feedback

All five issues from the previous review have been addressed. Here's a summary:

Previous Issue Status
🔴 Null challengeSlug/challengeTitle → broken links ✅ Fixed — hasChallenge guard on lines 46–47, conditional render on lines 64–75
🟡 XP total was a partial sum (20-entry cap, no indication) ✅ Fixed — renamed to recentXp, displayed as +{recentXp} XP recently
🟡 Fragile [&>button:last-child] CSS selector ✅ Fixed — hideCloseButton prop added to DialogContent, explicit <DialogClose> in the purple header
🟢 "View All Activity" visible with empty state ✅ Fixed — button now only rendered inside the recentGains.length > 0 branch
🟢 Magic number 3 for preview slice ✅ Fixed — const PREVIEW_COUNT = 3 on line 16

One remaining minor nit

The modal header shows {recentGains.length} activities (line 170–172), but recentGains is capped at 20. For a user with 25 activities the count will read "20 activities" without qualification, while the XP correctly says "recently". Consider adding the same qualifier, e.g. {recentGains.length} recent activities, for consistency.


The implementation is clean and the fixes are well-done. Ready to merge once the nit is addressed (or consciously left as-is).

Signed-off-by: Paul Brissaud <pbrissaud.pro@gmail.com>
@claude
Copy link

claude bot commented Mar 9, 2026

Claude finished @pbrissaud's task in 1m 5s —— View job


Code Review (Updated)

  • Gather context
  • Read changed files
  • Analyze and provide feedback

All previously flagged issues from the first two reviews remain fixed. The implementation is clean overall. Two small things to note in this latest push:


Nit: PREVIEW_COUNT changed to 4 but PR description says 3

components/dashboard-recent-gains-client.tsx line 16:

const PREVIEW_COUNT = 4;

The PR body states "Dashboard 'Recent Activity' card now shows only 3 items (down from 5)", and the first test plan item says "Dashboard shows exactly 3 activity items." The constant was silently bumped to 4 in this push (commit 34a6158 "test with 4"). Either update the PR description or revert to 3 — the current mismatch is confusing.


Nit (carry-over): Activity count not qualified as "recent"

components/dashboard-recent-gains-client.tsx lines 170–172:

{recentGains.length}{" "}
{recentGains.length === 1 ? "activity" : "activities"}

The XP stat next to it already says +{recentXp} XP recently, but the count still reads e.g. "20 activities" without qualification. For a user with more than 20 transactions, the number will be silently capped. Adding "recent" here (→ "20 recent activities") would make both stats consistent. Fix this →


Summary

Previous Issues Status
🔴 Null challengeSlug/challengeTitle → broken links ✅ Fixed
🟡 XP total was a partial sum without indication ✅ Fixed — "XP recently" label
🟡 Fragile [&>button:last-child] CSS selector ✅ Fixed — explicit DialogClose + hideCloseButton
🟢 "View All Activity" shown with empty state ✅ Fixed
🟢 Magic number 3 for preview slice ✅ Fixed — PREVIEW_COUNT constant
New / Carry-over Severity
PREVIEW_COUNT is 4 but PR says 3 🟢 Nit
Activity count not qualified as "recent" 🟢 Nit (carry-over)

The PR is otherwise ready to merge. These are purely cosmetic; address them or close them consciously.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude
Copy link

claude bot commented Mar 9, 2026

Claude finished @pbrissaud's task in 1m 5s —— View job


Code Review (Updated)

  • Gather context
  • Read changed files
  • Analyze and provide feedback

All issues from previous reviews are resolved in this latest push. Here's the full status:

Previous Issue Status
🔴 Null challengeSlug/challengeTitle → broken links ✅ Fixed — hasChallenge guard, conditional Link render
🟡 XP total was a partial sum without indication ✅ Fixed — "+{recentXp} XP recently"
🟡 Fragile [&>button:last-child] CSS selector ✅ Fixed — hideCloseButton prop + explicit <DialogClose> in header
🟢 "View All Activity" shown with empty state ✅ Fixed — button only rendered in the non-empty branch
🟢 Magic number 3 for preview slice ✅ Fixed — PREVIEW_COUNT constant
🟢 Activity count not qualified as "recent" ✅ Fixed — "N recent activities" on lines 170–172

One remaining nit: PREVIEW_COUNT is 4, PR description says 3

components/dashboard-recent-gains-client.tsx line 16:

const PREVIEW_COUNT = 4;

The PR body still says "Dashboard 'Recent Activity' card now shows only 3 items (down from 5)" and the test plan item reads "Dashboard shows exactly 3 activity items." The value was bumped to 4 in commit 34a6158 test with 4 and never reverted. This is purely a documentation/intent mismatch — either revert to 3 or update the PR description to reflect the new value.


The implementation is clean and all substantive issues are resolved. Ready to merge once the PREVIEW_COUNT/PR description mismatch is consciously resolved.

@pbrissaud pbrissaud merged commit e2b8c5e into main Mar 9, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant