-
-
Notifications
You must be signed in to change notification settings - Fork 52
Fixed the overlapping card issue in post tab #173
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughUpdated PostsTab layout by removing the outer grid classes from the top-level container, leaving only full-width/min-height styling. The inner grid that arranges posts remains unchanged. This affects how the loading state is displayed (no longer as a grid item), with no logic or API changes. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes an overlapping card issue in the posts tab by removing grid layout classes from the container div. The change simplifies the layout structure to resolve visual overlap problems between post cards.
- Removes grid layout styling that was causing card overlap issues
- Simplifies container div to basic width and min-height properties
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| return ( | ||
| <div className='w-full min-h-[50vh] grid sm:grid-cols-2 lg:grid-cols-3 gap-2 md:gap-4 flex-wrap '> | ||
| <div className='w-full min-h-[50vh]'> |
Copilot
AI
Sep 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing the grid layout classes may fix the overlapping issue but could break the intended card layout structure. Consider replacing with a proper grid or flexbox implementation rather than removing all layout styling, as this may cause cards to stack vertically without proper spacing or responsive behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should i change it to this ?
{loading ? (
<Loading />
) : filteredPosts && filteredPosts.length > 0 ? (
<div className='w-full flex flex-wrap gap-2 md:gap-4'>
{filteredPosts.map((entry, index) => (
<PostCard entry={entry} key={index} />
))}
</div>
) : (
<div className="text-center text-slate-500 max-w-lg mx-auto mt-10 p-6">
<h3 className="text-xl font-semibold text-slate-800 mb-2">Hmm, that's a rare combo!</h3>
<p>
We couldn't find any hackathons that match all of your selected filters.
Don't worry! Your next great project is likely just a click away. Try broadening your search.
</p>
</div>
)}
</div>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you must include the grid layout as without it the structure may break, you may want to adjust on the spacing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the spacing looks good to me.like if u want i can change it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (7)
app/(dashboard)/(routes)/teams/components/PostsTab.tsx (7)
11-25: TypeScript: use primitive types (avoidNumber).
Prefernumber(orstring | numberif IDs can be ObjectIds) for better typing and interop.Apply:
interface HackathonEntry { hackathonName: string; skills: string[]; role: string; experience: string; location: string; regDate: string; teamName: string; memberCount: string; regURL: string; hackathonMode: string; - id: Number; + id: string | number; createdAt: string; description: string; }If the API guarantees numeric IDs, switch to
numberonly.
30-31: Remove unused local state and stale comment.
Cleans up noise and avoids confusion.- const [data,setData]=useState([]); - // const [posts, setPostss] = useState<HackathonEntry[]>([]);
35-49: Prevent state updates after unmount; add request cancellation and defensive dispatch.
Aborts in-flight fetches and avoids undefined payloads.useEffect(() => { - const fetch = async () => { + const controller = new AbortController(); + const fetch = async () => { setLoading(true); try { - const res = await axios.get('/api/teams'); - console.log(res.data.Hackathon); - dispatch(setPosts(res.data.Hackathon)); - } catch (error) { + const res = await axios.get('/api/teams', { signal: controller.signal }); + dispatch(setPosts(res.data?.Hackathon ?? [])); + } catch (error: any) { + if (error?.code === 'ERR_CANCELED') return; console.error('Error fetching data:', error); } finally { setLoading(false); } }; fetch(); -}, [dispatch]); + return () => controller.abort(); +}, [dispatch]);
51-53: Drop dev logs.
Avoid noisy console output in production.-useEffect(() => { - console.log("POSTSS", posts); -}, [posts]); +// (Optional) Re-enable logging locally if needed
55-66: Memoize filtering to cut re-computation on every render.
Keeps UI snappy with large lists.- const filteredPosts = posts?.filter((post) => { + const filteredPosts = useMemo(() => posts?.filter((post) => { const matchesSearch = matchSearch(post.teamName) || matchSearch(post.hackathonName) || matchSearch(post.location) || matchSearch(post.experience) || matchSearch(post.hackathonMode) || matchSearch(post.role) || post.skills.some(s => matchSearch(s)); const matchesMode = modeOptions.length ? modeOptions.some(m => post.hackathonMode.toLowerCase().includes(m.toLowerCase())) : true; const matchesRole = expOptions.length ? expOptions.some(e => post.experience.toLowerCase().includes(e.toLowerCase())) : true; const matchesSkills = skillOptions.length ? skillOptions.some(skill => post.skills.some(s => s.toLowerCase().includes(skill.toLowerCase()))) : true; return matchesSearch && (matchesMode && matchesRole && matchesSkills); - }); + }), [posts, search, modeOptions, expOptions, skillOptions]);Additional import (supporting change outside the selected range):
import React, { useEffect, useMemo, useState } from 'react'
71-73: Center the loading state without reintroducing grid.
Keeps spinner nicely positioned while avoiding layout side effects.- {loading ? ( - <Loading /> + {loading ? ( + <div className="flex justify-center py-10"> + <Loading /> + </div> ) : filteredPosts && filteredPosts.length > 0 ? (
75-77: Use a stable key (avoid index) forPostCarditems.
Prevents subtle UI bugs on reordering/filters.- {filteredPosts.map((entry, index) => ( - <PostCard entry={entry} key={index} /> + {filteredPosts.map((entry) => ( + <PostCard entry={entry} key={String(entry.id)} /> ))}Confirm
idis unique across the list; otherwise add a composite key.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
app/(dashboard)/(routes)/teams/components/PostsTab.tsx(1 hunks)
🔇 Additional comments (1)
app/(dashboard)/(routes)/teams/components/PostsTab.tsx (1)
69-69: Overlap fix looks correct — removing the outer grid resolves nested-grid collisions.
This should stop cards from overlapping while keeping the inner grid intact.
fixes : #167
Summary by CodeRabbit