feat(community): Sticky header and infinite scroll for Forum#54
feat(community): Sticky header and infinite scroll for Forum#54qianghan wants to merge 2 commits into
Conversation
- Header (title, New Post, search, sort, category tabs) stays fixed - Summary card shows post count and active filter - Only the post feed scrolls in a dedicated scroll container - Infinite scroll: load more posts when user scrolls near bottom - IntersectionObserver triggers load when sentinel is visible Closes #53 Co-authored-by: Cursor <cursoragent@cursor.com>
|
@seanhanca is attempting to deploy a commit to the Livepeer Foundation Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
|
@github-copilot review |
| (entries) => { | ||
| if (entries[0]?.isIntersecting) loadMore(); | ||
| }, | ||
| { root, rootMargin: '200px', threshold: 0 } |
Check warning
Code scanning / CodeQL
Superfluous trailing arguments Warning
There was a problem hiding this comment.
Pull request overview
This PR implements a sticky header with infinite scroll functionality for the Community Hub forum. The changes enhance the UX by keeping navigation controls visible while scrolling through posts and automatically loading additional content as users reach the bottom of the feed.
Changes:
- Sticky header implementation with fixed filters, search, and category navigation
- Infinite scroll mechanism using IntersectionObserver to load posts automatically
- Layout refactoring to support fixed header with scrollable content area
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ); | ||
| observer.observe(sentinel); | ||
| return () => observer.disconnect(); | ||
| }, [loadMore]); |
There was a problem hiding this comment.
The loadMore dependency in the useEffect creates a new observer on every render because loadMore is recreated each time its dependencies change. This causes the observer to disconnect and reconnect frequently. Consider wrapping loadMore in useCallback with stable dependencies or restructuring to avoid recreating the observer unnecessarily.
There was a problem hiding this comment.
Fixed in de5194b — loadMore now uses postsLengthRef and votedPostsRef so it has stable deps (no posts.length). IntersectionObserver no longer reconnects on every load.
| } finally { | ||
| setLoadingMore(false); | ||
| } | ||
| }, [categoryFilter, searchQuery, sortBy, posts.length, total, loadingMore, loading]); |
There was a problem hiding this comment.
The loadMore callback includes posts.length as a dependency, which changes every time posts are loaded, causing the function to be recreated. This triggers the IntersectionObserver useEffect to recreate the observer. Consider using a functional state update in setPosts to avoid this dependency, or use a ref to track the current offset.
There was a problem hiding this comment.
Fixed in de5194b — using refs to avoid posts.length dependency. loadMore reads offset from postsLengthRef.current.
| @@ -156,88 +211,105 @@ export const ForumPage: React.FC = () => { | |||
| const currentUser = getCurrentUser(); | |||
|
|
|||
| return ( | |||
There was a problem hiding this comment.
The combination of h-full min-h-0 on the outermost container requires that a parent element has defined height constraints. Add a comment explaining this layout dependency, as it may cause layout issues if the parent structure changes.
| return ( | |
| return ( | |
| {/* NOTE: This outer container uses `h-full min-h-0` and therefore relies on a parent element */} | |
| {/* having explicit height constraints (e.g., an app shell with `h-screen` or similar). If the */} | |
| {/* parent layout changes and no longer constrains height, scrolling and flex behavior here */} | |
| {/* may break or behave unexpectedly. */} |
There was a problem hiding this comment.
Fixed in de5194b — added layout dependency comment above the container div.
- Use refs (postsLengthRef, votedPostsRef) for loadMore to avoid recreation on every post load; keeps IntersectionObserver stable - Add layout dependency comment for h-full min-h-0 parent requirement Co-authored-by: Cursor <cursoragent@cursor.com>
|
@github-copilot review |
| const postsLengthRef = useRef(0); | ||
| const votedPostsRef = useRef<Set<string>>(new Set()); | ||
| postsLengthRef.current = posts.length; | ||
| votedPostsRef.current = votedPosts; |
Check warning
Code scanning / CodeQL
Variable not declared before use Warning
| } finally { | ||
| setLoadingMore(false); | ||
| } | ||
| }, [categoryFilter, searchQuery, sortBy, total, loadingMore, loading]); |
There was a problem hiding this comment.
| }, [categoryFilter, searchQuery, sortBy, total, loadingMore, loading]); | |
| }, [categoryFilter, searchQuery, sortBy, total]); |
The loadMore callback dependency array still includes guard condition variables (loadingMore, loading) that shouldn't be dependencies, causing IntersectionObserver to recreate on every state change instead of only when actual filters change.
| ); | ||
| setVotedPosts(votedIds); | ||
| } | ||
| setPosts((prev) => [...prev, ...newPosts]); |
There was a problem hiding this comment.
|
Closing in favor of same-repo PR to fix label/merge permissions. |
Address 35 medium-severity CodeQL alerts across 22 files: **Log injection (js/log-injection) — 21 alerts:** - Add sanitizeForLog() helper to strip control characters and newlines from user-controlled values before they reach log output - Applied across 14 files (auth, registry, lifecycle, storage, plugins) **Permissive CORS configuration (js/cors-permissive-configuration) — 3 alerts:** - Replace wildcard CORS (origin: true / '*') with origin validation callback using CORS_ALLOWED_ORIGINS environment variable **File data in outbound network request (js/file-access-to-http) — 6 alerts:** - Add path.resolve() validation to ensure files stay within expected directories before reading and sending over network **Indirect command line injection (js/indirect-command-line-injection) — 2 alerts:** - Replace execSync with shell interpolation with execFileSync using argument arrays to prevent shell injection **Missing origin verification (js/missing-origin-check) — 2 alerts:** - Add event.origin check in postMessage handlers for plugin frontends **Network data written to file (js/http-to-file-access) — 1 alert:** - Add path traversal validation before writing network data to disk Alerts fixed: #54-56, #78-82, #98-101, #129-149, #277-278, #281-282 Co-authored-by: Cursor <cursoragent@cursor.com>
* fix: resolve all medium-severity code scanning alerts Address 35 medium-severity CodeQL alerts across 22 files: **Log injection (js/log-injection) — 21 alerts:** - Add sanitizeForLog() helper to strip control characters and newlines from user-controlled values before they reach log output - Applied across 14 files (auth, registry, lifecycle, storage, plugins) **Permissive CORS configuration (js/cors-permissive-configuration) — 3 alerts:** - Replace wildcard CORS (origin: true / '*') with origin validation callback using CORS_ALLOWED_ORIGINS environment variable **File data in outbound network request (js/file-access-to-http) — 6 alerts:** - Add path.resolve() validation to ensure files stay within expected directories before reading and sending over network **Indirect command line injection (js/indirect-command-line-injection) — 2 alerts:** - Replace execSync with shell interpolation with execFileSync using argument arrays to prevent shell injection **Missing origin verification (js/missing-origin-check) — 2 alerts:** - Add event.origin check in postMessage handlers for plugin frontends **Network data written to file (js/http-to-file-access) — 1 alert:** - Add path traversal validation before writing network data to disk Alerts fixed: #54-56, #78-82, #98-101, #129-149, #277-278, #281-282 Co-authored-by: Cursor <cursoragent@cursor.com> * fix: address CodeRabbit and Vercel review comments for PR 84 - Tenant config: sanitize settings before logging (log injection) - plugin-server-sdk CORS: empty allowlist now fails closed unless explicit '*' or dev mode; trim/normalize origins - postMessage (daydream, plugin-publisher): use origin allowlist (window.location.origin + VITE_ALLOWED_MESSAGE_ORIGINS) instead of event.origin !== window.location.origin which broke cross-origin iframes; use event.origin for postMessage target (not wildcard) - lifecycle executeS3Call: sanitize args before logging - sanitizeForLog: add U+2028/U+2029 to prevent log-forging bypass - Password reset: never log token in production (gate behind NODE_ENV) - plugin-server, plugin-publisher CORS: fail closed when allowlist empty in production; allow in dev for local workflow * chore: relax CORS to allow-all when allowlist empty Avoid breaking Vercel/deployments; empty CORS_ALLOWED_ORIGINS now defaults to allow-all. TODO: tighten later by setting allowlist. --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: seanhanca <seanhanca@users.noreply.github.com>
Summary
Implements sticky header UX for the Community Hub forum (issue #53).
Changes:
Layout:
flex flex-colwithflex-shrink-0header block andflex-1 min-h-0 overflow-y-autoposts arealoadMorewhen visibleCloses #53
Test plan
Made with Cursor