-
Notifications
You must be signed in to change notification settings - Fork 0
UI Polish: Card Hover Animation, Dynamic Topic Count, and Homepage Layout Improvements #25
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
Conversation
WalkthroughAdds a global Navbar to the app layout, replaces the homepage’s rendered component with a new composite HomePage, introduces several new homepage UI components, adjusts BullishMarketCard styling, refactors PostGallery and related imports, and removes legacy sorting and sentiment components. One stray unused import was introduced in PostsList. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Browser
participant App as Next.js App
participant Providers as Web3Providers
participant UI as Navbar
participant Page as HomePage
Browser->>App: Request /
App->>Providers: Render provider context
Providers->>UI: Render Navbar
Providers->>Page: Render HomePage
Note over Page: Composes PostSearch, PostFilter,<br/>BullishMarketCard, PostGalleryTitle, PostsGallery
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal). Please share your feedback with us on this Discord post. 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (18)
src/components/HomepageComponents/PostSearch.tsx (1)
3-6
: Rename props type + add basic a11y to the search input.
- Rename PostFiltersProps → PostSearchProps for clarity.
- Add type="search", aria-label, and keep width responsive.
-interface PostFiltersProps { +interface PostSearchProps { search: string; onSearchChange: (e: React.ChangeEvent<HTMLInputElement>) => void; } -export default function PostSearch({ +export default function PostSearch({ search, onSearchChange, -}: PostFiltersProps) { +}: PostSearchProps) { return ( <div className="mb-8"> <div className="flex justify-center"> <Input value={search} onChange={onSearchChange} - placeholder="Search posts by keyword..." - className="md:w-[600px] h-10 shadow-md rounded-lg bg-violet-50 border-2 focus-visible:!border-blue-500 focus:!ring-0 focus-visible:!ring-0" + type="search" + aria-label="Search posts" + placeholder="Search posts by keyword..." + className="w-full md:w-[600px] h-10 shadow-md rounded-lg bg-violet-50 border-2 focus-visible:!border-blue-500 focus:!ring-0 focus-visible:!ring-0" /> </div> </div> ); }Also applies to: 8-11, 15-20
src/components/HomepageComponents/PostGalleryTitle.tsx (1)
14-17
: Handle singular/plural and use a semantic heading.Render an h2 and pluralize “topic(s)”.
- <div className="text-2xl font-bold">Market Insights</div> - <div className="text-muted-foreground"> - {current} of {total} topics - </div> + <h2 className="text-2xl font-bold">Market Insights</h2> + <div className="text-muted-foreground"> + {current} of {total} {total === 1 ? "topic" : "topics"} + </div>src/components/Navbar/navbar.tsx (1)
10-13
: Fix token misuse and add semantic markup.
- text-background will be dark in dark mode (poor contrast on blue). Use text-white or a token designed for contrast.
- Prefer for semantics; set border color token for dark mode.
- <div className="sticky top-0 bg-transparent backdrop-blur-md w-full h-[80px] border-b-2 px-10 py-4 font-sans"> + <nav className="sticky top-0 bg-transparent backdrop-blur-md w-full h-[80px] border-b px-10 py-4 font-sans border-border" aria-label="Main navigation"> @@ - <div className="flex justify-center items-center bg-blue-500 h-10 w-10 text-background font-bold rounded-lg"> + <div className="flex justify-center items-center bg-blue-500 h-10 w-10 text-white font-bold rounded-lg"> <IoMdTrendingUp size={26} /> </div> @@ - </div> + </nav>Also applies to: 15-21
src/components/HomepageComponents/BullishMarketCard.tsx (1)
35-38
: Avoid potential SSR/client mismatch on “minutes ago”.Computing Date.now() during SSR can mismatch on hydration. Compute on client with state/effect or format a static timestamp.
- get minutesAgo() { - return Math.floor((Date.now() - this.updatedAt.getTime()) / 60000); - }, + // Derive this on the client to avoid SSR hydration drift. + minutesAgo: 0,And in the component:
// inside BullishMarketCard() const [minutesAgo, setMinutesAgo] = useState(0); useEffect(() => { const calc = () => setMinutesAgo(Math.floor((Date.now() - MockData.updatedAt.getTime()) / 60000)); calc(); const id = setInterval(calc, 60_000); return () => clearInterval(id); }, []);src/app/layout.tsx (1)
33-36
: Wrap Navbar with a header for semantics.Minor a11y polish.
- <Web3Providers> - <Navbar/> - {children} - </Web3Providers> + <Web3Providers> + <header> + <Navbar /> + </header> + {children} + </Web3Providers>src/app/page.tsx (1)
3-7
: Nit: name the default export Page for Next.js convention.Purely stylistic.
-export default function Allpage() { +export default function Page() { return ( <main className="max-w-7xl mx-auto px-4 sm:px-6 lg:px-8 py-8"> <HomePage/> </main> ); }src/components/HomepageComponents/PostFilter.tsx (3)
5-12
: Use a real button for a11y; add focus ring and dark-mode tokens.This div is visually a button but isn’t keyboard-accessible, has no focus styles, and hard-codes light colors. Prefer a semantic button with proper focus states and dark variants.
- <div className="h-10 w-20 flex flex-row gap-2 border rounded-md px-2 bg-violet-50 hover:bg-blue-500 hover:text-background"> + <button + type="button" + className="h-10 inline-flex items-center gap-2 px-3 rounded-md border bg-muted text-foreground + hover:bg-blue-500 hover:text-background + focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-blue-500 + disabled:opacity-50 disabled:pointer-events-none + dark:hover:bg-blue-400" + aria-label="Filter posts" + > @@ - </div> + </button>
9-11
: Tailwind doesn’t include text-md by default.Use text-sm or text-base unless you’ve extended the scale.
- <div className="flex justify-center items-center text-md font-semibold"> + <div className="flex justify-center items-center text-sm font-medium">
7-7
: Nit: inline strokeWidth on react-icons is often ignored.If you need thinner lines, consider a different icon weight or CSS class; otherwise drop the style.
src/components/HomepageComponents/PostGallery.tsx (9)
9-10
: Confirm CardAction export or remove import if unused.If CardAction isn’t used here and not needed later, remove to avoid dead imports. If it’s newly exported from ui/card, all good.
198-198
: Clickable card lacks semantics; add role/tabIndex or wrap with Link.You advertise clickability (cursor-pointer) but there’s no keyboard access. Either wrap the Card with a Next.js Link to the destination or add role/button semantics and key handling.
- <Card className="w-80 h-80 font-sans transition-transform duration-200 hover:-translate-y-2 group cursor-pointer"> + <Card + className="w-80 h-80 font-sans transition-transform duration-200 hover:-translate-y-2 group cursor-pointer" + role="button" + tabIndex={0} + onKeyDown={(e) => (e.key === 'Enter' || e.key === ' ') && /* trigger navigation */ null} + >
201-209
: Make the arrow appear only on hover and mark decorative icons aria-hidden.Currently the arrow is always visible. If the intent is hover-only, toggle opacity and transition.
- <div className="flex flex-row justify-between"> - <div className="group-hover:text-blue-500"> + <div className="flex flex-row justify-between"> + <div className="group-hover:text-blue-500"> {postGroup.title} - </div> - <div className="text-muted-foreground group-hover:text-blue-500"> - <FaArrowRight/> - </div> + </div> + <div className="text-muted-foreground group-hover:text-blue-500 opacity-0 group-hover:opacity-100 transition-opacity"> + <FaArrowRight aria-hidden="true" /> + </div> </div>
210-213
: Use theme tokens for text color to honor dark mode.Hard-coded text-gray-600 won’t adapt. Prefer text-muted-foreground.
- <CardDescription className="text-xs text-gray-600 mb-3 line-clamp-3"> + <CardDescription className="text-xs text-muted-foreground mb-3 line-clamp-3">
176-182
: Add dark-mode variants for sentiment pill colors.Ensure contrast works in dark mode.
- const sentimentColor = - dominantSentiment.sentiment === "BULLISH" - ? "text-green-700 bg-green-100" - : dominantSentiment.sentiment === "BEARISH" - ? "text-red-700 bg-red-100" - : "text-gray-700 bg-gray-100"; + const sentimentColor = + dominantSentiment.sentiment === "BULLISH" + ? "text-green-700 bg-green-100 dark:text-green-400 dark:bg-green-900/30" + : dominantSentiment.sentiment === "BEARISH" + ? "text-red-700 bg-red-100 dark:text-red-400 dark:bg-red-900/30" + : "text-gray-700 bg-gray-100 dark:text-gray-300 dark:bg-gray-800"
241-251
: Neutral bar color should adapt in dark mode.Add a dark variant to avoid low contrast on dark backgrounds.
- <div - className="bg-gray-300" + <div + className="bg-gray-300 dark:bg-gray-700" style={{ flex: sentimentBar.neutral }} ></div>
261-273
: Source pill colors: add dark variants.These BG/text combinations are tuned for light mode only.
- pillClass += " bg-blue-50 text-blue-600 border-blue-200"; + pillClass += " bg-blue-50 text-blue-600 border-blue-200 dark:bg-blue-900/20 dark:text-blue-300 dark:border-blue-800"; @@ - pillClass += " bg-orange-50 text-orange-600 border-orange-200"; + pillClass += " bg-orange-50 text-orange-600 border-orange-200 dark:bg-orange-900/20 dark:text-orange-300 dark:border-orange-800"; @@ - pillClass += " bg-gray-50 text-gray-600 border-gray-200"; + pillClass += " bg-gray-50 text-gray-600 border-gray-200 dark:bg-gray-800 dark:text-gray-300 dark:border-gray-700";
231-234
: Avoid drift: derive the post count from posts.length.Displaying totalposts while computing from posts.length risks inconsistency.
- {postGroup.totalposts} posts + {totalPosts} posts
298-302
: Responsive direction: use flex-col on small screens and stable keys.
- flex defaults to row; use flex-col for small screens to prevent overflow.
- Avoid index as key; use the group id.
- <div className="flex md:flex-row justify-start md:ml-20 gap-6 p-2"> - {postGroups.map((postGroup, index) => ( - <PostCard key={index} postGroup={postGroup} /> + <div className="flex flex-col md:flex-row justify-start md:ml-20 gap-6 p-2"> + {postGroups.map((postGroup) => ( + <PostCard key={postGroup.id} postGroup={postGroup} /> ))}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/app/layout.tsx
(2 hunks)src/app/page.tsx
(1 hunks)src/components/HomepageComponents/BullishMarketCard.tsx
(3 hunks)src/components/HomepageComponents/HomePage.tsx
(1 hunks)src/components/HomepageComponents/PostFilter.tsx
(1 hunks)src/components/HomepageComponents/PostGallery.tsx
(3 hunks)src/components/HomepageComponents/PostGalleryTitle.tsx
(1 hunks)src/components/HomepageComponents/PostSearch.tsx
(2 hunks)src/components/Navbar/navbar.tsx
(1 hunks)src/components/PostComponents/PostSortSelect.tsx
(0 hunks)src/components/PostComponents/PostsList.tsx
(2 hunks)src/components/PostComponents/SentimentBar.tsx
(0 hunks)
💤 Files with no reviewable changes (2)
- src/components/PostComponents/SentimentBar.tsx
- src/components/PostComponents/PostSortSelect.tsx
🧰 Additional context used
🧬 Code graph analysis (5)
src/app/layout.tsx (2)
src/components/Web3Providers.tsx (1)
Web3Providers
(23-31)src/components/Navbar/navbar.tsx (1)
Navbar
(4-29)
src/app/page.tsx (1)
src/components/HomepageComponents/HomePage.tsx (1)
HomePage
(7-19)
src/components/HomepageComponents/HomePage.tsx (5)
src/components/HomepageComponents/PostSearch.tsx (1)
PostSearch
(8-24)src/components/HomepageComponents/PostFilter.tsx (1)
PostFilter
(3-14)src/components/HomepageComponents/BullishMarketCard.tsx (1)
BullishMarketCard
(41-137)src/components/HomepageComponents/PostGalleryTitle.tsx (1)
PostGalleryTitle
(8-20)src/components/HomepageComponents/PostGallery.tsx (1)
PostsGallery
(294-304)
src/components/HomepageComponents/PostGallery.tsx (1)
src/components/ui/card.tsx (3)
Card
(91-91)CardHeader
(92-92)CardTitle
(94-94)
src/components/PostComponents/PostsList.tsx (1)
src/components/HomepageComponents/PostSearch.tsx (1)
PostSearch
(8-24)
🔇 Additional comments (2)
src/components/HomepageComponents/BullishMarketCard.tsx (1)
10-10
: Confirm CardAction is exported from ../ui/card.If not, this import will break. Please verify or inline the UI.
src/components/HomepageComponents/PostGallery.tsx (1)
7-7
: LGTM: arrow icon import.Import looks correct and aligns with the hover affordance in the header.
@ginzahatemi please send a new pr for further ui polish to match our mockups. this is good enough for now. merging |
PR Description:
hover:text-blue-500
for color change).Summary by CodeRabbit
New Features
UI Improvements
Refactor