Group top nav links into Discover/Account dropdowns - #18
Draft
zowskyy wants to merge 2 commits into
Draft
Conversation
The signed-in nav had 14 flat top-level items (Explore, Wander, Make, Feed, Vibe, Policy, Ask Us, Rings, Notifications, Messages, My Page, Studio, Settings, Log out, plus Moderation for mods) crammed into one row. Adds a small accessible NavDropdown (click-to-open, closes on outside click/Escape/item activation) and regroups: - Discover ▾: Explore, Wander, Feed, Vibe, Rings — browsing/discovery pages, none of them time-sensitive enough to need one-click access. - Make stays a standalone link — it's the primary call to action. - Notifications and Messages stay standalone with their badges — these are time-sensitive and benefit from a glance, not a click-to-reveal. - Account ▾: My Page, Studio, Settings, Ask Us, Moderation (mods only), Policy, Log out. Signed-in desktop nav goes from 14 items to 5 (Discover, Make | Notifications, Messages, Account). The mobile drawer is left as a flat list — a single scrollable panel already solves the density problem there, so nesting dropdowns inside it would only add complexity. npm test: 238/238 passing. npm run build: clean. Verified live with a headless browser against both the logged-out and logged-in nav, open and closed states of both dropdowns.
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueComment |
…ebase Full-codebase audit (4 parallel passes: social/messaging, moderation/ safety, discovery/themes, API/infra) following up on earlier targeted fixes. Root-cause fixes for every confirmed finding: Privacy/block-bypass leaks (same class as the earlier Top Eight and custom-CSS bugs, found in four more places): - activityFeed.ts: a friend's activity leaked even after they went private, and guestbook-activity items exposed a private/hidden/ blocked-from target's handle purely through a mutual friend's guestbook signature. Both now respect the same visibility/block rules canViewPage already enforces on a direct visit. - proximityGraph.ts (Wander) and vibe/page.tsx (Vibe Graph): neither ever checked block relationships, so a blocked user could still appear in Wander batches or the Vibe neighbor graph, complete with their ambient status. Both now exclude blocked pairs in both directions. - collections.ts: curated collections used a narrower visibility filter than discovery.ts's shared DISCOVERABLE_WHERE, so a page hidden from discovery or a platform-blocked user's page still rendered on /explore/collection/[slug]. Race conditions (check-then-act without a transaction): - stamps.ts: concurrent requests could both pass the "already stamped today" check before either insert committed, bypassing the one-per-day limit. Now atomic (BEGIN IMMEDIATE), same pattern as rateLimit.ts and appeals.ts already use. - friends.ts: two concurrent opposite-direction friend requests could each insert a separate pending row instead of the second auto-accepting into one friendship. Now atomic. - messages.ts: the first-message-ever lookup ran before the write transaction opened, so two concurrent first messages between the same pair could crash on the conversations UNIQUE constraint. Now re-reads under the write lock instead of trusting the pre-transaction snapshot. Idempotency: - moderation.ts: reviewReport had no "already resolved" guard (unlike the equivalent appeals.ts path), so two moderators reviewing the same report could silently overwrite each other's outcome and log contradictory actions. Now rejects a second review of a non-open report. Missing rate limits (unauthenticated or otherwise-uncapped abuse vectors): - /api/visit and /api/presence: client-controlled tokens with no server-side cap let a caller inflate visit/presence counts arbitrarily by simply omitting or rotating tokens. - /api/status, /api/activity, /api/export: had no rate limit at all, unlike every comparable action route in the codebase. - Web ring join/leave actions: same gap. Correctness: - sharedThemes.ts: forking a theme never persisted forked_from_id/ attribution_handle, even though every read path selects and exposes them — provenance was always null despite the embedded CSS attribution string being set correctly. Hardening: - handleParam.ts: a percent-encoded slash in the @handle route segment decoded to a value containing "/", which nothing downstream expected. Now rejected rather than passed through. Added activityFeed.test.ts and collections.test.ts (neither had coverage before — how these leaks went unnoticed) plus regression tests for the moderation idempotency and theme-fork-attribution fixes. npm test: 248/248 passing (10 new). npm run build: clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
1. Nav cleanup — the signed-in top nav had grown to 14 flat top-level items. Grouped into two dropdowns:
Signed-in desktop nav: 14 items → 5. New
NavDropdownclient component (click to open, closes on outside click/Escape/item activation, proper ARIA), styled to match the site's existing plain-border/monospace panels.2. Full-codebase security/correctness audit — four parallel passes (social/messaging, moderation/safety, discovery/themes, API/infra) following up on earlier targeted fixes (#16, #17). Root-cause fixes for every confirmed finding:
Privacy/block-bypass leaks (same class as the earlier Top Eight and custom-CSS bugs, found in four more places):
activityFeed.ts: a friend's activity leaked after they went private; guestbook-activity items exposed a private/hidden/blocked-from target's handle through a mutual friend's guestbook signature. Both now respect the same rulescanViewPageenforces on a direct visit.proximityGraph.ts(Wander) andvibe/page.tsx(Vibe Graph): neither checked block relationships — a blocked user could still appear in Wander batches or the Vibe neighbor graph. Now excluded in both directions.collections.ts: curated collections used a narrower visibility filter than discovery's sharedDISCOVERABLE_WHERE— a hidden-from-discovery or platform-blocked user's page still rendered on/explore/collection/[slug].Race conditions (check-then-act without a transaction):
stamps.ts,friends.ts,messages.ts— each had a window where concurrent requests could bypass a limit or crash on a constraint. All now atomic, matching theBEGIN IMMEDIATEpattern already used elsewhere in the codebase.Idempotency:
moderation.ts'sreviewReporthad no "already resolved" guard (unlike the equivalentappeals.tspath) — two moderators reviewing the same report could silently overwrite each other's outcome.Missing rate limits:
/api/visit,/api/presence: client-controlled tokens with no server-side cap allowed arbitrary count inflation./api/status,/api/activity,/api/export, web ring join/leave: had no rate limit at all, unlike every comparable route.Correctness:
sharedThemes.ts: forking a theme never persistedforked_from_id/attribution_handle, despite every read path selecting and exposing them.Hardening:
handleParam.ts: a percent-encoded slash in the@handleroute segment decoded to a value containing/, now rejected.Added
activityFeed.test.tsandcollections.test.ts(neither had coverage before — how these leaks went unnoticed) plus regression tests for the moderation idempotency and theme-fork-attribution fixes.Test plan
npm test— 248/248 passing (10 new)npm run build— clean typecheck, all routes compile🤖 Generated with Claude Code