SUMMA-22: Implement Tab System in Feed #119
Conversation
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FeedPage
participant ContentList
participant API
participant AuthProvider
User->>FeedPage: Visit Feed page
FeedPage->>AuthProvider: Check authentication
AuthProvider-->>FeedPage: Provide user data/loading state
FeedPage->>ContentList: Render with selected tab
ContentList->>API: Fetch content (personalized/latest/trending)
API-->>ContentList: Return content data
ContentList->>FeedPage: Render content tiles or error/loading state
User->>ContentList: Click content tile
ContentList->>User: Show preview popup
User->>ContentList: Close preview
ContentList->>User: Hide preview popup
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (11)
frontend/src/pages/loading/LoadingPage.tsx (1)
1-5: Simple yet effective loading page implementation.The LoadingPage component provides a clean fallback UI during authentication loading, which is a good practice for improving user experience. This component is appropriately used in the AuthProvider.
However, consider adding a loading indicator or text to give users more explicit feedback about the loading state.
import Background from "../../components/Background"; +import { CircularProgress } from "@mui/material"; // or any other loading indicator export default function LoadingPage() { - return <Background />; + return ( + <> + <Background /> + <div style={{ position: "absolute", top: "50%", left: "50%", transform: "translate(-50%, -50%)" }}> + <CircularProgress /> + <p>Loading...</p> + </div> + </> + ); }frontend/src/styles/navbar.scss (1)
130-130: Consistent z-index adjustments for navbar components.The z-index changes for menus, notifications, and search results (all to 10) while the overlay is set to 2 create a logical stacking order. This ensures these UI elements appear appropriately above other content while maintaining hierarchy.
Consider defining these z-index values as CSS variables for better maintainability.
:root { --profile-size: 50px; --navbar-height: 80px; --navbar-margin: 10px 5vw; + --z-index-background: 5; + --z-index-overlay: 2; + --z-index-dropdown: 10; }Then use these variables throughout the code:
.navbar-background { /* other properties */ - z-index: 5; + z-index: var(--z-index-background); } .menu { /* other properties */ - z-index: 10; + z-index: var(--z-index-dropdown); }Also applies to: 182-182, 280-280, 292-292
frontend/src/hooks/AuthProvider.tsx (2)
58-58: Remove debug logging statement.This console.log statement should be removed before production deployment.
async function getUserData(userUID: string) { - console.log("Fetching user data... for UID:", userUID); const userData = await fetchUser(userUID);
69-72: Added loading state UI fallback.Rendering the LoadingPage component while loading provides good user feedback and prevents flashes of unauthenticated UI.
Consider adding a timeout mechanism to prevent infinite loading in case of network issues.
useEffect(() => { const initializeUser = async () => { + const timeoutId = setTimeout(() => { + console.error("Authentication timed out"); + setUser(null); + setLoading(false); + }, 30000); // 30 second timeout try { const res = await axios.post( `${apiURL}/user/refresh-token`, {}, { withCredentials: true } ); if (res.status === 200) { const data = res.data; console.log("User data:", data); await getUserData(data.userUID); } } catch (err) { console.error("Auto-login failed", err); setUser(null); } finally { + clearTimeout(timeoutId); setLoading(false); } }; initializeUser(); }, []);frontend/src/styles/feed.scss (1)
6-6: Avoid using !important in CSS rules.While this ensures the grid layout takes precedence,
!importantflags make CSS harder to maintain and debug. Consider restructuring selectors for higher specificity instead..content-list { display: grid; - grid-template-columns: repeat(6, 1fr) !important; + grid-template-columns: repeat(6, 1fr); } /* Then make your media queries more specific */ @media screen and (max-width: 2824px) { - .content-list { + .feed-container .content-list { grid-template-columns: repeat(5, 1fr) !important; } }frontend/src/styles/profile/profile.scss (3)
107-110: Sticky tabs might be obscured by fixed headers
position: stickywithtop: 100pxassumes every viewport has ≥ 100 px of free space before content starts scrolling. If your main header height changes (e.g. mobile view, admin toolbar), the tabs could hide behind it.Consider using a CSS custom property or calculating the offset in JS so the value stays in-sync with the real header height.
120-122: Provide a graceful fallback for browsers that do not supportbackdrop-filter
backdrop-filter(and its-webkit-variant) is still unsupported in Firefox and some older mobile browsers. Users on those browsers will see raw background colour without blur, which might clash with the design.Add a solid semi-transparent background (e.g.
rgba(...)) or feature-detect with@supports(backdrop-filter: blur(1px))to ensure readability everywhere.
129-139: Clarify intent of comment-only sectionsThe comments “First tab button styling” / “Last tab button styling” are helpful, but the rules themselves apply to both active and inactive tabs. A quick reader might assume they affect only one state.
Rename selectors or move comments above both states, e.g.
/* Rounded edges for first / last tab */. This tiny doc fix prevents future mis-edits.frontend/src/components/feed/ContentList.tsx (2)
150-156: First item is always an advert
index % 8 === 0injects an ad at index 0, pushing the real content down and looking odd when the list is short.-{index % 8 === 0 ? ( +{index !== 0 && index % 8 === 0 ? (Alternatively, test
((index + 1) % 8 === 0).
150-166: Potential key collisionsOuter
divalready usescontent.uid || indexas a key, then the innerContentTilerepeats a similar key. React will warn only in development, but duplicate keys can hinder reconciliation.You can safely drop the inner key prop; React will inherit the stable key from its parent.
frontend/src/pages/Feed.tsx (1)
119-149: Creator card click area lacks button semanticsThe entire
.creator-profile-carddivis clickable but not semantically a button or link. This hurts accessibility and keyboard navigation.Use a
<button>or<a>withrole="link"and appropriate styling, or at least addtabIndex={0}and key handlers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
frontend/src/components/feed/ContentList.tsx(1 hunks)frontend/src/hooks/AuthProvider.tsx(3 hunks)frontend/src/pages/Feed.tsx(2 hunks)frontend/src/pages/loading/LoadingPage.tsx(1 hunks)frontend/src/styles/feed.scss(3 hunks)frontend/src/styles/navbar.scss(5 hunks)frontend/src/styles/profile/profile.scss(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
frontend/src/pages/loading/LoadingPage.tsx (1)
frontend/src/components/Background.tsx (1)
Background(1-11)
frontend/src/hooks/AuthProvider.tsx (2)
frontend/src/models/User.ts (1)
User(1-22)frontend/src/pages/loading/LoadingPage.tsx (1)
LoadingPage(3-5)
🪛 Biome (1.9.4)
frontend/src/components/feed/ContentList.tsx
[error] 81-81: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (5)
frontend/src/styles/navbar.scss (1)
29-29: Increased z-index for navbar background.Raising the z-index from 2 to 5 helps ensure proper element stacking.
frontend/src/hooks/AuthProvider.tsx (3)
7-7: Added loading page and state tracking.Good addition of loading state and importing the LoadingPage component, laying the foundation for a better user experience during authentication.
Also applies to: 13-13
16-16: Now properly awaiting user data during login.Making the login function await getUserData completion ensures that user data is fully loaded before continuing, preventing race conditions.
48-51: Improved error handling with finally block.Setting user to null on error and ensuring loading state is cleared regardless of outcome is a robust error handling approach.
frontend/src/pages/Feed.tsx (1)
23-43: Disabled exhaustive-deps hides legitimate dependenciesThe
fetchContenteffect depends onauth.userbut the linter ignore comment hides it. If the user logs in after mount, related creators are never fetched.Either include
auth.userin the dependency array or justify the ignore with a comment explaining why live updates aren’t needed.
| scrollbar-width: none; | ||
| margin: -20px; | ||
| padding: 20px; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Improved creator profiles list styling.
Hiding scrollbars and adjusting margins/padding makes the UI cleaner, but consider the accessibility implications.
Users should still be able to recognize this list is scrollable without visible scrollbars. Consider adding visual indicators to show there's more content to scroll.
🏁 Script executed:
#!/bin/bash
# Check if scroll indicators are implemented elsewhere
rg -A 2 "scroll|overflow|indicator" --glob "*.{tsx,jsx,scss,css}" | grep -v "scrollbar-width: none"Length of output: 6412
Add visual scroll indicators to the creator profiles list
Hiding the scrollbar in frontend/src/styles/feed.scss (around lines 134–136) keeps the UI clean, but users may not realize the list is scrollable. Please add one or more of the following affordances to signal overflow:
- A subtle gradient fade on the right (or bottom) edge of the container
- A custom “more” arrow or dot indicator that appears when content is truncated
- A CSS mask-image or shadow overlay to suggest additional items off-screen
Example (fade overlay):
.feed-list {
position: relative;
overflow-x: auto;
scrollbar-width: none;
/* existing margin/padding */
&::after {
content: '';
position: absolute;
top: 0; right: 0;
width: 2rem; height: 100%;
pointer-events: none;
background: linear-gradient(to left, rgba($bg-color, 1), rgba($bg-color, 0));
}
}🤖 Prompt for AI Agents (early access)
In frontend/src/styles/feed.scss around lines 134 to 136, the scrollbar is
hidden which improves UI cleanliness but reduces scrollability awareness. To fix
this, add a visual scroll indicator such as a subtle gradient fade on the right
edge of the scroll container. Implement this by adding a ::after pseudo-element
with a linear-gradient background that overlays the container's right side,
signaling that more content is available to scroll. Ensure the container has
position: relative and overflow-x: auto to support this effect.
| if (response.data && response.data.success) { | ||
| let normalizedContent; | ||
|
|
||
| switch (tab) { | ||
| case "personalized": | ||
| normalizedContent = response.data.personalizedContent.map( | ||
| (content: Content) => normalizeContentDates(content) | ||
| ); | ||
| break; | ||
| case "latest": | ||
| normalizedContent = response.data.content.map((content: Content) => | ||
| normalizeContentDates(content) | ||
| ); | ||
| break; | ||
| case "trending": | ||
| normalizedContent = response.data.trendingContent.map( | ||
| (content: Content) => normalizeContentDates(content) | ||
| ); | ||
| break; | ||
| default: | ||
| return false; | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Set error to null on success and use optional chaining
If a previous fetch failed, error remains non-null and the message persists even after a successful retry.
- if (response.data && response.data.success) {
+ if (response?.data?.success) {
...
setContents(normalizedContent);
+ setError(null);(The optional-chaining also satisfies the Biome hint.)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (response.data && response.data.success) { | |
| let normalizedContent; | |
| switch (tab) { | |
| case "personalized": | |
| normalizedContent = response.data.personalizedContent.map( | |
| (content: Content) => normalizeContentDates(content) | |
| ); | |
| break; | |
| case "latest": | |
| normalizedContent = response.data.content.map((content: Content) => | |
| normalizeContentDates(content) | |
| ); | |
| break; | |
| case "trending": | |
| normalizedContent = response.data.trendingContent.map( | |
| (content: Content) => normalizeContentDates(content) | |
| ); | |
| break; | |
| default: | |
| return false; | |
| } | |
| if (response?.data?.success) { | |
| let normalizedContent; | |
| switch (tab) { | |
| case "personalized": | |
| normalizedContent = response.data.personalizedContent.map( | |
| (content: Content) => normalizeContentDates(content) | |
| ); | |
| break; | |
| case "latest": | |
| normalizedContent = response.data.content.map((content: Content) => | |
| normalizeContentDates(content) | |
| ); | |
| break; | |
| case "trending": | |
| normalizedContent = response.data.trendingContent.map( | |
| (content: Content) => normalizeContentDates(content) | |
| ); | |
| break; | |
| default: | |
| return false; | |
| } | |
| setContents(normalizedContent); | |
| setError(null); | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 81-81: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🤖 Prompt for AI Agents (early access)
In frontend/src/components/feed/ContentList.tsx around lines 81 to 103, after a
successful fetch indicated by response.data.success, the error state is not
reset to null, causing old error messages to persist after retries. Fix this by
setting the error state to null upon success. Additionally, use optional
chaining when accessing nested properties like response.data.personalizedContent
to safely handle undefined values and satisfy the Biome hint.
| async function fetchContent(): Promise<boolean> { | ||
| try { | ||
| let response; | ||
|
|
||
| switch (tab) { | ||
| case "personalized": | ||
| if (!auth.user?.uid) { | ||
| setContents([]); | ||
| return false; | ||
| } | ||
| response = await axios.get( | ||
| `${apiURL}/content/feed/${auth.user.uid}`, | ||
| { timeout: 5000, withCredentials: true } | ||
| ); | ||
| break; | ||
| case "latest": | ||
| response = await axios.get(`${apiURL}/content`, { | ||
| timeout: 5000, | ||
| }); | ||
| break; | ||
| case "trending": | ||
| response = await axios.get(`${apiURL}/content/feed/trending`, { | ||
| timeout: 5000, | ||
| }); | ||
| break; | ||
| default: | ||
| return false; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
fetchContent duplicates endpoint logic & loses error context
- The switch block repeats three times – once for the request and again for data extraction.
- Silent
catch {}swallows the real error, making debugging difficult. - Returning
falseisn’t used anywhere.
Refactor into a lookup table and propagate the error so the caller can decide.
+interface Endpoint {
+ url: string;
+ extract: (d: any) => Content[];
+}
+
+const endpoints: Record<string, Endpoint> = {
+ personalized: {
+ url: `${apiURL}/content/feed/${auth.user?.uid}`,
+ extract: (d) => d.personalizedContent,
+ },
+ latest: {
+ url: `${apiURL}/content`,
+ extract: (d) => d.content,
+ },
+ trending: {
+ url: `${apiURL}/content/feed/trending`,
+ extract: (d) => d.trendingContent,
+ },
+};Use it in one pass and remove the boolean return value.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents (early access)
In frontend/src/components/feed/ContentList.tsx around lines 52 to 79, the
fetchContent function duplicates endpoint logic in multiple places and swallows
errors silently, losing error context. Refactor by creating a lookup table
mapping tabs to their respective API endpoints, then perform the axios request
in a single step using this table. Remove the boolean return value and instead
propagate any caught errors to the caller by rethrowing them, allowing the
caller to handle errors appropriately.
| useEffect(() => { | ||
| setIsLoading(true); | ||
|
|
||
| fetchContent(); | ||
|
|
||
| setIsLoading(false); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [tab]); |
There was a problem hiding this comment.
isLoading toggles off before the data arrives
setIsLoading(false) runs immediately after calling fetchContent(), so the spinner disappears before the AJAX request finishes. Move the flag – and the early setIsLoading(true) – inside an awaited async wrapper.
-useEffect(() => {
- setIsLoading(true);
-
- fetchContent();
-
- setIsLoading(false);
+useEffect(() => {
+ let isMounted = true;
+ const load = async () => {
+ setIsLoading(true);
+ await fetchContent(); // wait until the promise settles
+ if (isMounted) setIsLoading(false);
+ };
+ load();
+
+ return () => {
+ isMounted = false; // avoid set-state on unmounted component
+ };
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [tab]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| setIsLoading(true); | |
| fetchContent(); | |
| setIsLoading(false); | |
| // eslint-disable-next-line react-hooks/exhaustive-deps | |
| }, [tab]); | |
| useEffect(() => { | |
| let isMounted = true; | |
| const load = async () => { | |
| setIsLoading(true); | |
| await fetchContent(); // wait until the promise settles | |
| if (isMounted) { | |
| setIsLoading(false); | |
| } | |
| }; | |
| load(); | |
| return () => { | |
| isMounted = false; // avoid set-state on unmounted component | |
| }; | |
| // eslint-disable-next-line react-hooks/exhaustive-deps | |
| }, [tab]); |
🤖 Prompt for AI Agents (early access)
In frontend/src/components/feed/ContentList.tsx around lines 19 to 26, the
isLoading state is set to false immediately after calling fetchContent(),
causing the loading spinner to disappear before the data fetch completes. To fix
this, wrap the fetchContent call inside an async function within useEffect,
await the fetchContent call, and only then set isLoading to false. Also, move
setIsLoading(true) inside this async function before the fetch starts to
properly reflect the loading state during the data fetch.
| const [tab, setTab] = useState<"personalized" | "trending" | "latest">( | ||
| auth.user ? "personalized" : "trending" | ||
| ); |
There was a problem hiding this comment.
tab state never updates when auth status changes
useState initialises tab based on auth.user at mount time. If the user logs in/out later, the value stays stale (e.g. a guest logs in but still sees “Trending” selected).
Convert to useEffect watching auth.user or compute the initial value lazily in a useEffect that runs whenever auth.user changes.
-const [tab, setTab] = useState<...>(auth.user ? "personalized" : "trending");
+const [tab, setTab] = useState<...>("trending");
+
+useEffect(() => {
+ setTab(auth.user ? "personalized" : "trending");
+}, [auth.user]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [tab, setTab] = useState<"personalized" | "trending" | "latest">( | |
| auth.user ? "personalized" : "trending" | |
| ); | |
| const [tab, setTab] = useState<"personalized" | "trending" | "latest">("trending"); | |
| useEffect(() => { | |
| setTab(auth.user ? "personalized" : "trending"); | |
| }, [auth.user]); |
🤖 Prompt for AI Agents (early access)
In frontend/src/pages/Feed.tsx around lines 17 to 19, the tab state is
initialized once based on auth.user and does not update when auth status
changes, causing stale UI. Fix this by removing the initial auth.user check from
useState and instead add a useEffect hook that watches auth.user; inside the
effect, update the tab state to "personalized" if auth.user exists or "trending"
if not, ensuring the tab reflects the current auth status dynamically.
Summary by CodeRabbit
New Features
Refactor
Style