Skip to content

[#756] Reader holdings: replace inline stats with 4-box grid#760

Merged
realproject7 merged 2 commits intomainfrom
task/756-reader-holdings-boxes
Apr 2, 2026
Merged

[#756] Reader holdings: replace inline stats with 4-box grid#760
realproject7 merged 2 commits intomainfrom
task/756-reader-holdings-boxes

Conversation

@realproject7
Copy link
Copy Markdown
Owner

Summary

  • Replaces inline label-value stats (Balance, Price, Entry, Traded) in Reader holdings cards with a 2×2 stat-box grid matching the Writer tab pattern
  • Value: USD only with 24h price % change (green/red coloring)
  • Balance: compact K/M format (e.g. 240K, 1.5M)
  • PnL: USD profit/loss with %, or "—" if no entry price
  • First Traded: full date with year from first mint timestamp
  • Adds formatCompact helper and firstTraded field to PortfolioHolding

Fixes #756

Test plan

  • 4 stat boxes appear next to Moleskine book (2×2 grid)
  • Value shows USD with 24h % change, no PLOT value
  • Balance shows shortened format (K/M)
  • PnL shows USD profit/loss with % (or "—" if no entry price)
  • First Traded shows full date with year
  • No inline Price/Entry/Traded labels remaining
  • Mobile (375px): boxes fit cleanly
  • npm run build passes

🤖 Generated with Claude Code

Replaces inline label-value text (Balance, Price, Entry, Traded) with
a 2×2 stat-box grid matching the Writer tab pattern:
- Value: USD with 24h % change (colored green/red)
- Balance: compact K/M format via formatCompact helper
- PnL: USD profit/loss with % (or dash if no entry price)
- First Traded: full date with year from first mint timestamp

Also adds firstTraded field to PortfolioHolding interface, derived
from the existing first-mint query.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 2, 2026

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
plotlink Ignored Ignored Apr 2, 2026 1:45pm

Request Review

Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

T2b APPROVE

Good restructure from inline stats to 2×2 grid. Review notes:

  • formatCompact helper is clean and handles edge cases (M/K/raw)
  • reserveDecimals correctly used for value formatting; token balance uses hardcoded 18 (pre-existing pattern)
  • PnL calculation logic is correct: (current - entry) * balance * plotUsd with proper fallback to "—"
  • firstTraded efficiently reuses the existing firstMint query
  • Grid layout with grid-cols-2 gap-2 should work well at 375px

No concerns.

Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

The new 2x2 holdings grid is close, but two parts of the implementation do not match the requested behavior for issue #756.

Findings

  • [medium] The new Value box still falls back to a PLOT-denominated string when plotUsd is unavailable, even though the ticket explicitly requires USD-only display for this field.
    • File: src/app/profile/[address]/page.tsx:1513
    • Suggestion: Render or a loading placeholder until plotUsd is available instead of falling back to ${formatPrice(...)} ${RESERVE_LABEL}.
  • [medium] First Traded is currently derived from the user's first mint only, so readers who first acquired a token via a non-mint trade will incorrectly see even when they do have trade history.
    • File: src/app/profile/[address]/page.tsx:1307
    • Suggestion: Query the earliest trade for that user/storyline without restricting to event_type = "mint"; keep the entry-price logic separate if it still needs first-mint semantics.

Decision

Requesting changes because the current implementation can show the wrong Value format and can leave First Traded blank for valid reader holdings.

…trade type

- Value box now shows "—" instead of PLOT when USD price unavailable
- First Traded derives from any trade type (not just mints), so readers
  who acquired via transfer are covered

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Summary

The follow-up commit addresses both review findings: the Value box no longer falls back to a PLOT-denominated display, and First Traded now comes from the earliest trade regardless of event type.

Findings

  • None.

Decision

Approving because the Reader holdings 2x2 grid now matches the requested issue #756 behavior and the previously reported correctness gaps are resolved.

@realproject7 realproject7 merged commit a1355b4 into main Apr 2, 2026
5 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.

Reader holdings: replace inline stats with 4-box grid (Value, Balance, PnL, First Traded)

2 participants