feat: refresh Valhalla member archive - #5
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 9 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe PR replaces the archive UI with custom responsive components, adds wallet controls and fuzzy search, externalizes S3 and development-origin configuration, and expands review and session workflow documentation. ChangesArchive interface
Runtime configuration
Review and session workflows
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant WalletControl
participant Homepage
participant FilesRoute
participant S3
WalletControl->>Homepage: report wallet state
Homepage->>FilesRoute: request authorized files
FilesRoute->>S3: list archive objects
S3-->>FilesRoute: return file keys
FilesRoute-->>Homepage: return files
Homepage->>FilesRoute: request signed file URL
FilesRoute->>S3: create signed URL
S3-->>FilesRoute: return signed URL
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR modernizes the Valhalla member archive experience by introducing a redesigned UI (custom header, wallet controls, and CSS-driven layout), adding fuzzy search over archive entries, and improving configuration flexibility by making the S3 bucket and dev origins configurable via environment variables.
Changes:
- Added
S3_BUCKETandDEV_ALLOWED_ORIGINSconfiguration (includinggetS3Bucket()), replacing the hardcoded S3 bucket in API routes. - Reworked the UI layout (new
Header, newWalletControl, new CSS theme/fonts, and updatedapp/page.tsxgating + archive browsing UX). - Added
fuzzyScore()utility and wired it into channel/file searching.
Reviewed changes
Copilot reviewed 15 out of 26 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| sample.env | Documents new environment variables for dev origins and S3 bucket configuration. |
| next.config.js | Reads DEV_ALLOWED_ORIGINS and exposes allowedDevOrigins for dev server behavior. |
| AGENTS.md | Updates the repo’s known environment-variable list to include DEV_ALLOWED_ORIGINS and S3_BUCKET. |
| app/config.ts | Adds getS3Bucket() helper to enforce configured S3 bucket at runtime. |
| app/api/files/route.ts | Uses getS3Bucket() when listing S3 objects for members. |
| app/api/channel/route.ts | Uses getS3Bucket() when creating signed URLs for member file access. |
| app/utils/fuzzy.ts | Adds a fuzzy scoring helper for search. |
| app/page.tsx | Rebuilds the member gating and archive UI; adds fuzzy search and richer loading/error states. |
| app/shared/WalletControl.tsx | Introduces a custom RainbowKit wallet control used by the header and hero CTA. |
| app/shared/Header.jsx | Replaces Chakra-based header with a new branded header using Next components. |
| app/shared/Footer.jsx | Removes the old Chakra-based footer. |
| app/layout.tsx | Switches to global CSS + custom fonts and applies a customized RainbowKit theme. |
| app/fonts.ts | Adds font setup (local + Google fonts) and CSS variables. |
| app/globals.css | Introduces the new site-wide styling, layout, and component classes. |
| app/icon.svg | Adds the app icon asset. |
| public/brand/full-m800.svg | Adds new full brand logo asset. |
| public/brand/symbol-m500.svg | Adds new symbol brand logo asset. |
| public/brand/swords.svg | Adds new “swords” brand icon asset. |
| public/brand/crystal.svg | Adds new “crystal” brand icon asset (used via CSS masking). |
| docs/session-workflow.md | Updates internal workflow guidance for sessions, verification, and review gates. |
| docs/pr-review-workflow.md | Refines PR review workflow documentation for safety and clarity. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (7)
app/utils/fuzzy.ts (2)
4-5: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse
toLowerCase()for locale-independent matching.
toLocaleLowerCase()without an explicit locale uses the runtime locale. In a Turkish locale,"I"lowercases to"ı"and the match against"i"fails. Search results then differ per user for the same data. UsetoLowerCase(), or pass a fixed locale.♻️ Proposed change
- const needle = query.trim().toLocaleLowerCase(); - const haystack = candidate.toLocaleLowerCase(); + const needle = query.trim().toLowerCase(); + const haystack = candidate.toLowerCase();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/utils/fuzzy.ts` around lines 4 - 5, Update the normalization expressions for needle and haystack to use locale-independent toLowerCase() instead of toLocaleLowerCase(), preserving the existing trim behavior for query and case-insensitive matching.
3-3: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd an explicit return type.
fuzzyScorereturnsnumber | null. The caller inapp/page.tsxline 231 relies on thenullcase. An explicit annotation documents the contract and prevents an accidental widening.♻️ Proposed change
-export function fuzzyScore(query: string, candidate: string) { +export function fuzzyScore(query: string, candidate: string): number | null {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/utils/fuzzy.ts` at line 3, Update the fuzzyScore function signature to explicitly return number | null, preserving its existing behavior and documenting the null case relied on by its caller.app/page.tsx (1)
28-29: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe static analysis secret findings are false positives.
Line 28 holds a public ERC-20 contract address on Gnosis, not a credential. No change is needed. Consider a short comment above the constant to name the token, so future scans and readers have context.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/page.tsx` around lines 28 - 29, Keep SHARES_TOKEN_ADDRESS unchanged because it is a public contract address, not a credential; add a brief comment immediately above it identifying the ERC-20 token and its Gnosis-network context.Source: Linters/SAST tools
app/globals.css (1)
44-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider
100dvhfor the full-height containers.
100vhon mobile browsers includes the collapsing address bar. The shell then overflows the visible viewport.min-height: 100dvhmatches the visible area.♻️ Proposed change
.site-shell { position: relative; display: flex; - min-height: 100vh; + min-height: 100dvh; flex-direction: column;Also applies to: 97-101
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/globals.css` around lines 44 - 45, Update the body min-height declaration to use 100dvh instead of 100vh, and apply the same viewport-unit change to the related full-height container declarations in this stylesheet.app/shared/Header.jsx (2)
8-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConvert
Header.jsxto TypeScript.The rest of the new UI uses
.tsx, for exampleapp/shared/WalletControl.tsx. A.jsxfile gets no type checking against theWalletControlprops contract. Rename the file toapp/shared/Header.tsx.As per coding guidelines: "
app/**/*.{ts,tsx}: Follow the existing Next.js 16 App Router, React 19, TypeScript, and Chakra UI patterns."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/shared/Header.jsx` around lines 8 - 10, Rename the Header component file from Header.jsx to Header.tsx and preserve its existing implementation while applying the project’s TypeScript and React patterns. Update any imports or references to use the renamed module so Header participates in type checking alongside WalletControl.Source: Coding guidelines
12-29: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueRemove
priorityfrom the logo that is hidden at the current breakpoint.Both logos render at every breakpoint. CSS hides one with
display: noneinapp/globals.csslines 159-163 and 788-795.priorityadds a preload link for both files. The browser then fetches an image that is never shown. Keeppriorityon the full logo only, or drop it from both because SVG logos are small.♻️ Proposed change
<Image className="brand-logo-symbol" src="/brand/symbol-m500.svg" alt="RaidGuild" width={40} height={38} - priority unoptimized />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/shared/Header.jsx` around lines 12 - 29, Remove the priority prop from the breakpoint-hidden logo in Header’s paired Image elements, while retaining it only on the full logo (or remove it from both if preferred). Ensure the hidden symbol logo no longer generates a preload request.app/shared/WalletControl.tsx (1)
41-48: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGuard the deferred connect effect against repeated calls.
openConnectModalcomes fromConnectButton.Customand gets a new identity on each render. The effect depends on it. WhileconnectRequestedstaystrue, any re-render re-runs the effect and callsopenConnectModal()again, and the cleanup cancels the pending reset timeout. Clear the request flag before you open the modal instead of using a timeout.♻️ Proposed change
useEffect(() => { if (!ready || !connectRequested) return; + setConnectRequested(false); openConnectModal(); - const resetRequest = window.setTimeout(() => setConnectRequested(false), 0); - - return () => window.clearTimeout(resetRequest); }, [connectRequested, openConnectModal, ready]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/shared/WalletControl.tsx` around lines 41 - 48, Update the useEffect handling connectRequested to clear the request flag synchronously before calling openConnectModal, removing the deferred setConnectRequested timeout and its cleanup. Keep the existing ready and connectRequested guards, and retain the dependency on openConnectModal while ensuring rerenders cannot reopen the modal for the same request.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/globals.css`:
- Line 59: Update the affected declarations in the stylesheet to satisfy
Stylelint: change text-rendering to the lowercase optimizelegibility value,
replace the deprecated clip declaration with clip-path: inset(50%), and use the
lowercase currentcolor value at the referenced color declarations.
In `@app/page.tsx`:
- Around line 243-244: Update the isCheckingAccess calculation so
isFilesFetching does not trigger the loading gate that replaces the archive;
scope access loading to the initial connection/share checks while preserving the
existing behavior for isConnecting and isSharesLoading.
- Around line 396-436: Add an explicit pre-archive guard in the component’s
final render flow, after the existing wallet and signature checks, so the
archive is rendered only when the membership lookup has completed and the user
is confirmed as a member with a successful signature. For unresolved or
non-member states, route to the appropriate existing connect/check-in gate
instead of falling through to the “Guild archive” section.
In `@app/utils/fuzzy.ts`:
- Around line 18-38: Update the fuzzy matching loop around `searchFrom` and
`previousMatch` to account for each matched code point’s UTF-16 length: advance
`searchFrom` by `character.length` rather than one unit, and update the
adjacency check to compare the current match index with the end of the previous
matched character. Preserve existing scoring behavior for word separators and
gaps.
In `@docs/pr-review-workflow.md`:
- Around line 41-61: Update the staging/merge-readiness flow in steps 6–7 to
require the independent-review gate from docs/session-workflow.md after PR-sized
implementation and automated verification, before staging or declaring the work
ready to merge; make this conditional for PR-sized fixes or explicitly require
following the companion workflow, while preserving the existing user-approval
requirement.
In `@docs/session-workflow.md`:
- Around line 11-14: Update docs/session-workflow.md lines 11-14, 66-69, and
172-173 to define and consistently use the candidate’s intended parent or merge
base as the review baseline; retain main only when it is the intended parent,
and require the complete diff to be reviewed against that same baseline.
In `@sample.env`:
- Around line 10-11: Update the S3_BUCKET placeholder in the sample environment
configuration to use hyphens instead of underscores, ensuring the copied value
conforms to general-purpose S3 bucket naming rules while preserving the existing
placeholder intent.
---
Nitpick comments:
In `@app/globals.css`:
- Around line 44-45: Update the body min-height declaration to use 100dvh
instead of 100vh, and apply the same viewport-unit change to the related
full-height container declarations in this stylesheet.
In `@app/page.tsx`:
- Around line 28-29: Keep SHARES_TOKEN_ADDRESS unchanged because it is a public
contract address, not a credential; add a brief comment immediately above it
identifying the ERC-20 token and its Gnosis-network context.
In `@app/shared/Header.jsx`:
- Around line 8-10: Rename the Header component file from Header.jsx to
Header.tsx and preserve its existing implementation while applying the project’s
TypeScript and React patterns. Update any imports or references to use the
renamed module so Header participates in type checking alongside WalletControl.
- Around line 12-29: Remove the priority prop from the breakpoint-hidden logo in
Header’s paired Image elements, while retaining it only on the full logo (or
remove it from both if preferred). Ensure the hidden symbol logo no longer
generates a preload request.
In `@app/shared/WalletControl.tsx`:
- Around line 41-48: Update the useEffect handling connectRequested to clear the
request flag synchronously before calling openConnectModal, removing the
deferred setConnectRequested timeout and its cleanup. Keep the existing ready
and connectRequested guards, and retain the dependency on openConnectModal while
ensuring rerenders cannot reopen the modal for the same request.
In `@app/utils/fuzzy.ts`:
- Around line 4-5: Update the normalization expressions for needle and haystack
to use locale-independent toLowerCase() instead of toLocaleLowerCase(),
preserving the existing trim behavior for query and case-insensitive matching.
- Line 3: Update the fuzzyScore function signature to explicitly return number |
null, preserving its existing behavior and documenting the null case relied on
by its caller.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ae1e4dba-ead9-4dc5-9123-d8a627b827e9
⛔ Files ignored due to path filters (9)
app/favicon.icois excluded by!**/*.icoapp/icon.svgis excluded by!**/*.svgpublic/brand/crystal.svgis excluded by!**/*.svgpublic/brand/full-m800.svgis excluded by!**/*.svgpublic/brand/swords.svgis excluded by!**/*.svgpublic/brand/symbol-m500.svgis excluded by!**/*.svgpublic/fonts/MAZIUSREVIEW20.09-Regular.woffis excluded by!**/*.woffpublic/fonts/MaziusDisplay-Bold.otfis excluded by!**/*.otfpublic/fonts/MaziusDisplay-Extraitalic.otfis excluded by!**/*.otf
📒 Files selected for processing (17)
AGENTS.mdapp/api/channel/route.tsapp/api/files/route.tsapp/config.tsapp/fonts.tsapp/globals.cssapp/layout.tsxapp/page.tsxapp/shared/Footer.jsxapp/shared/Header.jsxapp/shared/WalletControl.tsxapp/utils/fuzzy.tsdocs/pr-review-workflow.mddocs/session-workflow.mdnext.config.jspublic/brand/portal-arch-c.webpsample.env
💤 Files with no reviewable changes (1)
- app/shared/Footer.jsx
👮 Files not reviewed due to content moderation or server errors (1)
- public/brand/portal-arch-c.webp
This pull request introduces several significant improvements to the codebase, focusing on configuration flexibility, UI modernization, and utility enhancements. The most important changes include making the S3 bucket configurable via environment variables, redesigning the site header and wallet controls, updating font and theme usage, and adding a fuzzy search utility. Additionally, documentation has been improved for clarity and safety in the PR review workflow.
Configuration and Environment Variables
S3_BUCKETandDEV_ALLOWED_ORIGINSto environment variables, and implemented agetS3Bucket()function to retrieve the S3 bucket name from the environment, replacing hardcoded values throughout the codebase. This makes S3 integration more flexible and production-ready. [1] [2] [3] [4] [5] [6] [7]UI and Theming
Header.jsx) and wallet control (WalletControl.tsx), using Next.js components and improved branding. Footer was removed. [1] [2] [3]fonts.ts) and updated the site to use these fonts consistently. [1] [2]Utilities
fuzzyScorefunction inutils/fuzzy.tsfor improved search and matching capabilities.Documentation
These changes modernize the user interface, enhance configuration management, and improve both developer and user experience.
Summary by CodeRabbit