Skip to content

Display Farcaster identity: nav, comments, story cards#256

Merged
realproject7 merged 3 commits intomainfrom
task/255-farcaster-identity-everywhere
Mar 17, 2026
Merged

Display Farcaster identity: nav, comments, story cards#256
realproject7 merged 3 commits intomainfrom
task/255-farcaster-identity-everywhere

Conversation

@realproject7
Copy link
Copy Markdown
Owner

Summary

  • Add FarcasterAvatar component — reusable address → avatar + @username with link to farcaster.com/{username}, falls back to truncated address
  • Add useConnectedIdentity hook — resolves connected wallet's FC identity via getFarcasterProfile server action
  • Update ConnectWallet — nav bar shows FC avatar + @username when wallet has linked FC account
  • Update CommentSection — commenter addresses replaced with FarcasterAvatar
  • Update StoryCard — writer addresses replaced with WriterIdentityClient
  • All existing behavior preserved: WriterIdentityClient on story pages unchanged

Fixes #255

Test plan

  • Nav bar shows FC avatar + @username when connected wallet has linked FC account
  • Nav bar falls back to truncated address when no FC account linked
  • Comments show FC identity for commenters with linked FC accounts
  • Story cards show FC identity for writers
  • WriterIdentityClient on story pages still works correctly
  • npm run typecheck passes

🤖 Generated with Claude Code

- Add FarcasterAvatar component: reusable address → avatar + @username
- Add useConnectedIdentity hook: resolves connected wallet's FC identity
- ConnectWallet: show FC avatar + @username in nav bar
- CommentSection: show FC identity for commenters
- StoryCard: show FC identity for story writers
- Falls back to truncated address when no FC account is linked

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.

APPROVE — with two non-blocking observations.

1. Missing .catch() on server action promises (non-blocking)
Both FarcasterAvatar.tsx:29 and useConnectedIdentity.ts:27 call getFarcasterProfile(address).then(...) without a .catch(). If the server action rejects (e.g., network error to the server), this produces an unhandled promise rejection. The underlying lookupByAddress catches internally, so this is unlikely in practice, but a .catch(() => null) on both would be strictly correct.

2. Brief flash of truncated address (non-blocking)
FarcasterAvatar renders the truncated address immediately while the server action resolves, then swaps to FC identity. This is a standard loading-state tradeoff and acceptable — just noting it in case a skeleton/shimmer is preferred later.

What looks good:

  • Clean separation: useConnectedIdentity for the nav bar (tied to useAccount), FarcasterAvatar for arbitrary addresses (comments)
  • farcaster.com URL in the link (not warpcast.com) — correct per current branding
  • StoryCard correctly reuses existing WriterIdentityClient instead of duplicating
  • Cancellation via cancelled flag in both effects prevents state updates after unmount
  • Fallback to truncated address is consistent across all three surfaces

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 Farcaster identity surfaces match the issue scope, but the story-card change introduces invalid nested links and conflicting click targets.

Findings

  • [medium] StoryCard wraps the entire card in Next Link, and this PR replaces the plain writer text with WriterIdentityClient, which itself renders an external <a href="https://farcaster.com/...">. That creates an anchor inside an anchor, which is invalid HTML and can break click behavior on the cards.
    • File: src/components/StoryCard.tsx:44
    • Suggestion: Render a non-link variant inside story cards, or make WriterIdentityClient support a non-interactive mode for already-linked containers.

Decision

Request changes. The new identity display is otherwise on the right track, but the nested-anchor regression should be fixed before merge.

WriterIdentityClient and FarcasterAvatar now accept linkProfile={false}
to render plain text instead of an anchor. StoryCard passes false since
it's already wrapped in a Next Link. Addresses T2a review on PR #256.

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 patch fixes the story-card nested-anchor regression by adding a non-link rendering mode for identity components. The Farcaster identity rollout remains aligned with issue #255.

Findings

  • [info] No blocking issues found in the updated patch.

Decision

Approve. linkProfile={false} removes the invalid anchor nesting inside story cards while preserving linked identity displays in comments and other standalone contexts. CI was still pending when reviewed.

Restructure useConnectedIdentity to derive loading state from
resolvedFor comparison instead of calling setLoading synchronously
in the effect body.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@realproject7 realproject7 merged commit 87ec960 into main Mar 17, 2026
1 check passed
@realproject7 realproject7 mentioned this pull request Mar 23, 2026
4 tasks
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.

Display Farcaster identity everywhere: nav, comments, story cards, dashboards

2 participants