Update nav links, redirects, and cleanup#691
Conversation
- Nav: Replace "Writer" and "Reader" with single "Dashboard" link - Dashboard links to /profile/[connected-address] when connected, falls back to /writer redirect when not connected - Dashboard highlights when on any /profile/ page - Update NavBar test to reflect new nav structure + add wagmi mock 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
project7-interns
left a comment
There was a problem hiding this comment.
LGTM. Nav consolidated: Writer+Reader replaced with single Dashboard link, dynamic href based on connection state, proper isActive highlighting for /profile/ pages, key={label} for dynamic hrefs, test updated with wagmi mock. All #685 criteria met.
project7-interns
left a comment
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The nav cleanup is close, but the new Dashboard active-state logic is still wrong for one common case. When the viewer is disconnected and browsing any public /profile/... page, the Dashboard item no longer highlights even though this PR claims it should.
Findings
- [medium]
Dashboardonly highlights/profile/routes when the link href itself starts with/profile/, but the disconnected fallback href is/dashboard/writer. That means any public profile page viewed while disconnected loses the active Dashboard state.- File:
src/components/NavBar.tsx:27 - Suggestion: Decouple the Dashboard active-state check from the current href value so
pathname.startsWith("/profile/")still activates Dashboard even when the fallback destination is/dashboard/writer.
- File:
Decision
Requesting changes because the current active-state logic does not match the intended Dashboard behavior on public profile pages.
…ages Decouple Dashboard highlight from its href — always mark active on /profile/ and /dashboard/ routes regardless of connection state. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
project7-interns
left a comment
There was a problem hiding this comment.
Re-review: isActive now decoupled from href for Dashboard — highlights on /profile/ and /dashboard/ routes regardless of connection state. LGTM.
project7-interns
left a comment
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The Dashboard active-state regression is fixed, but issue #685 is still not fully implemented. The dead-code cleanup requirement remains incomplete on the latest head.
Findings
- [medium]
ReaderPortfoliois still checked into the tree even though it is no longer referenced anywhere on the PR head.- File:
src/components/ReaderPortfolio.tsx:23 - Suggestion: Delete
ReaderPortfolio.tsx(and any leftover related comments/imports) or wire it back into a live route if it is intentionally still part of the product. As written, this misses #685's "remove unused writer/reader dashboard components" requirement.
- File:
Decision
Requesting changes because the PR still leaves orphaned reader dashboard code behind, so the cleanup portion of #685 is not complete.
Dead code — functionality fully merged into Profile Portfolio tab. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
project7-interns
left a comment
There was a problem hiding this comment.
Re-review: ReaderPortfolio.tsx deleted and stale comment reference cleaned up. Dead code removed per #685 cleanup requirement. LGTM.
project7-interns
left a comment
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The follow-up resolves the remaining #685 cleanup blocker. ReaderPortfolio.tsx is deleted on the latest head and the stale profile-page comment was cleaned up, while the earlier Dashboard nav fix remains intact.
Findings
- None.
Decision
Approving because the PR now satisfies the nav, redirect, tab-link, and cleanup scope for #685.
2 participants
Summary
/profile/[connected-address]when connected, falls back to/writerredirect when not/profile/page/writer→ profile stories tab,/reader→ profile portfolio tab?tab=stories,?tab=portfolio,?tab=activitydeep links already work (from PR Merge Writer Dashboard into Profile Stories tab #689)Fixes #685
Test plan
/writerredirects to profile stories tab/readerredirects to profile portfolio tab?tab=stories,?tab=portfolio,?tab=activityURL params worknpm run buildpasses ✅🤖 Generated with Claude Code