Optimize club site loading and scroll performance#474
Conversation
|
Warning Review limit reached
More reviews will be available in 44 minutes and 13 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?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 credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. 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, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR optimizes the KnightHacks club site across three axes: all internal ChangesClub Site Performance Optimizations
Sequence Diagram(s)sequenceDiagram
participant Browser
participant IntersectionObserver
participant Component as HomeEvents / EventsClient / TeamsClient
participant BladeAPI as Blade API
Browser->>Component: mount, attach containerRef/eventsSectionRef/rosterSectionRef
Component->>IntersectionObserver: observe(sectionRef, rootMargin: 200px)
alt IntersectionObserver unavailable
Component->>Component: setTimeout → setShouldLoad(true)
else Element intersects viewport
IntersectionObserver-->>Component: isIntersecting = true
Component->>Component: setShouldLoad(true), observer.disconnect()
end
Component->>BladeAPI: loadClubEvents() / loadEvents() / fetchRoster()
BladeAPI-->>Component: data returned → render content
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 6 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/club/src/app/_components/club-motion-runtime.tsx (1)
47-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the now-dead
revealElementsbookkeeping.After the synchronous reveal scan was removed, this set is only mutated and never read, so it adds state without behavior.
Proposed cleanup
const observedElements = new WeakSet<Element>(); - const revealElements = new Set<Element>(); let driftElements: DriftState[] = []; let heroElements: HeroState[] = []; let timelines: TimelineState[] = []; function revealElement(element: Element) { element.classList.add("is-visible"); revealObserver.unobserve(element); - revealElements.delete(element); } function observeElement(element: Element) { if (observedElements.has(element)) return; observedElements.add(element); element.classList.add("is-motion-observed"); - revealElements.add(element); revealObserver.observe(element); }As per coding guidelines, “Do not leave debug logging, dead code, or commented-out experiments behind.”
Also applies to: 102-105, 122-128
🤖 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 `@apps/club/src/app/_components/club-motion-runtime.tsx` around lines 47 - 48, The `revealElements` Set variable is declared but only mutated and never read, making it dead code that should be removed entirely. Delete the declaration of the `revealElements` variable and remove all places where it is mutated or referenced throughout the file (including at the locations mentioned at lines 102-105 and 122-128) to clean up unused state variables per coding guidelines.Source: Coding guidelines
🤖 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 `@apps/club/src/app/_components/club-motion-runtime.tsx`:
- Around line 147-165: The current code only invalidates cached scroll targets
when DOM nodes are added or removed via the mutationObserver, but cached metrics
like offsetTop, offsetHeight, and scrollHeight can change due to layout reflow
from images, fonts, or content changes without DOM mutations. Add a
ResizeObserver alongside the existing MutationObserver to detect layout changes
on observed elements. When resize events are detected, also call
refreshScrollTargets() to ensure the cached geometry stays in sync. Apply this
ResizeObserver to the same elements being tracked by the MutationObserver in the
observeTree function to maintain consistent invalidation across all relevant
layout changes.
- Around line 145-165: The hasTiltTargets variable is calculated once before the
MutationObserver runs, so newly added tilt target elements never receive their
pointermove/pointerout listeners. In the MutationObserver callback's addedNodes
loop, after calling observeTree(node), check if the added node matches the
TILT_SELECTOR or contains descendants matching TILT_SELECTOR, and attach the
tilt event listeners to those matching elements. This ensures dynamically
inserted tilt targets get the necessary pointer event handlers even if no tilt
targets existed in the initial DOM.
---
Nitpick comments:
In `@apps/club/src/app/_components/club-motion-runtime.tsx`:
- Around line 47-48: The `revealElements` Set variable is declared but only
mutated and never read, making it dead code that should be removed entirely.
Delete the declaration of the `revealElements` variable and remove all places
where it is mutated or referenced throughout the file (including at the
locations mentioned at lines 102-105 and 122-128) to clean up unused state
variables per coding guidelines.
🪄 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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: e52b7511-4c10-4478-b15c-7afe4ead2130
📒 Files selected for processing (15)
apps/club/src/app/_components/club-content-page.tsxapps/club/src/app/_components/club-motion-runtime.tsxapps/club/src/app/_components/footer.tsxapps/club/src/app/_components/home-community-carousel.tsxapps/club/src/app/_components/home-events.tsxapps/club/src/app/_components/navbar.tsxapps/club/src/app/_components/redirect.tsxapps/club/src/app/about/page.tsxapps/club/src/app/events/events-client.tsxapps/club/src/app/globals.cssapps/club/src/app/hackathons/hackathon-history.tsxapps/club/src/app/layout.tsxapps/club/src/app/not-found.tsxapps/club/src/app/sponsors/sponsors-client.tsxapps/club/src/app/teams/teams-client.tsx
💤 Files with no reviewable changes (2)
- apps/club/src/app/_components/home-community-carousel.tsx
- apps/club/src/app/sponsors/sponsors-client.tsx
1 participant
Why
The Club site was loading and animating more than it needed to.
Some things were happening too early, like fetching events/team data and preloading routes before the user clicked anything. Fast scrolling also felt weird because some scroll-based animations were slightly delayed instead of moving with the page right away.
This PR makes the site lighter on first load and smoother while scrolling.
What
Closes: #473
Test Plan
pnpm --filter=@forge/club formatpnpm --filter=@forge/club lintpnpm --filter=@forge/club typecheckpnpm --filter=@forge/club buildAlso checked the built site:
getBoundingClientRect()transition: none0which is fire94Performance and100for Accessibility, Best Practices, and SEO which is good on local testing before deployment nglChecklist
Summary by CodeRabbit
Release Notes
Performance
Improvements