Added new page Newsletter #163
Conversation
|
@Nachiket-Roy is attempting to deploy a commit to the AJEET PRATAP SINGH's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Warning Rate limit exceeded@Nachiket-Roy has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 56 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (1)
WalkthroughAdds a timeline-based newsletter page component with filtering/sorting and an accessible full-article modal, registers a sidebar navigation entry for "Newsletter", and introduces a newsletter data module exporting a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Page as Newsletter Page
participant Observer as IntersectionObserver
participant DOM as DOM/Refs
User->>Page: Open /dashboard/newsletter
activate Page
Page->>DOM: mount, init refs, compute layout
Page->>Observer: observe newsletter card elements
deactivate Page
User->>Page: Scroll timeline
activate Page
Page->>Page: compute activeProgress
Page->>DOM: update central fill & active dot
deactivate Page
Observer->>Page: card enters viewport
activate Observer
Observer->>Page: mark visible, trigger connector animation
Page->>DOM: reveal card, update glowing/active state
deactivate Observer
User->>Page: Apply filter/sort
activate Page
Page->>Page: update filterMonth/filterYear/sortType
Page->>DOM: re-render filtered list
deactivate Page
User->>Page: Click "Read Full Article"
activate Page
Page->>DOM: open modal (selectedNewsletter), trap focus
User->>Page: Click outside or press Esc
Page->>Page: clear selection, restore focus, close modal
deactivate Page
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related issues
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
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: 6
🧹 Nitpick comments (4)
apps/web/src/data/newsletter.ts (1)
8-8: Fix formatting inconsistency.Remove the space before the colon for consistency with TypeScript conventions.
Apply this diff:
- takeaways? : string[]; + takeaways?: string[];apps/web/src/app/(main)/dashboard/newsletter/page.tsx (3)
269-278: Use Next.jsImagecomponent for optimization.Replace the native
<img>tag with Next.jsImagecomponent for automatic optimization, lazy loading, and better performance.+"use client"; import React, { useState, useEffect, useRef } from "react"; +import Image from "next/image"; import { Calendar, User, ArrowRight, ArrowLeft } from "lucide-react";Then update the image rendering:
{newsletter.image && ( <div className="relative w-full h-48 overflow-hidden"> - <img + <Image src={newsletter.image} alt={newsletter.title} + fill + sizes="(max-width: 768px) 100vw, 50vw" className="w-full h-full object-cover transition-transform duration-500 hover:scale-110" />Note: You'll need to configure the Unsplash domain in
next.config.tsunderimages.remotePatterns.
362-371: Use Next.jsImagecomponent in modal.Similar to the card images, the modal hero image should also use the Next.js
Imagecomponent for optimization.{/* Hero Image */} {selectedNewsletter.image && ( <div className="relative w-full h-96 overflow-hidden"> - <img + <Image src={selectedNewsletter.image} alt={selectedNewsletter.title} + fill + sizes="(max-width: 1024px) 100vw, 1024px" + priority className="w-full h-full object-cover" />
244-267: Consider extracting inline style calculation.The complex conditional styling logic for card expansion could be extracted into a helper function for improved readability and maintainability.
const getCardStyle = ( index: number, expandingCard: number | null, cardRefs: React.MutableRefObject<(HTMLDivElement | null)[]> ) => { if (expandingCard !== index) return {}; const rect = cardRefs.current[index]?.getBoundingClientRect(); if (!rect) return {}; return { top: rect.top, left: rect.left, width: cardRefs.current[index]?.offsetWidth, height: cardRefs.current[index]?.offsetHeight, transition: "all 0.6s cubic-bezier(0.34, 1.56, 0.64, 1)", }; }; // Then in JSX: style={{ transitionDelay: isVisible ? "0ms" : `${index * 120}ms`, ...getCardStyle(index, expandingCard, cardRefs), }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/src/app/(main)/dashboard/newsletter/page.tsx(1 hunks)apps/web/src/components/dashboard/Sidebar.tsx(2 hunks)apps/web/src/data/newsletter.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/app/(main)/dashboard/newsletter/page.tsx (1)
apps/web/src/data/newsletter.ts (2)
Newsletter(1-10)newsletters(11-147)
🔇 Additional comments (2)
apps/web/src/components/dashboard/Sidebar.tsx (1)
39-43: LGTM!The newsletter route is properly integrated into the sidebar navigation, following the existing pattern for route definitions.
apps/web/src/app/(main)/dashboard/newsletter/page.tsx (1)
336-336:animate-inclass is properly defined—no action needed.The
animate-inutility class is provided by thetailwindcss-animateplugin, which is correctly installed as a dependency inapps/web/package.json(version^1.0.7) and configured inapps/web/tailwind.config.ts. The class will work as intended.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/web/src/app/(main)/dashboard/newsletter/page.tsx (1)
372-457: Improve modal focus management for full accessibility (focus trap / initial focus).The modal now has ESC handling and ARIA attributes, which is great. However, focus isn’t moved into the dialog or trapped while it’s open, so keyboard users can still tab to background content and don’t get an obvious starting point inside the modal.
Consider:
- Moving focus to the “Back to Timeline” button (or a dedicated close button) when
selectedNewsletterbecomes non‑null.- Trapping focus within the modal and restoring focus to the triggering element on close (e.g., via a small custom hook or a library like
react-focus-lock).
🧹 Nitpick comments (3)
apps/web/src/data/newsletter.ts (1)
1-10: Document the requireddatestring format to avoid subtle breakage.The timeline’s
parseDatehelper assumes dates like"Dec 10, 2025"(3‑letter month,MMM DD, YYYY). If someone later adds data with a different format, sorting/filtering will silently misbehave.Consider documenting this invariants directly on the type:
export interface Newsletter { id: number; title: string; - date: string; + /** + * Display date in format "MMM DD, YYYY" (e.g. "Dec 10, 2025"). + * Timeline sorting/filtering logic relies on this exact format. + */ + date: string; author: string; preview: string; content: string; takeaways?: string[]; image: string; }apps/web/src/app/(main)/dashboard/newsletter/page.tsx (2)
64-73: Re-run the initial animation when filters or sort change to keep indices in sync.
visibleCardsandactiveDotare keyed by themapindex, but the init effect only runs once. After changingfilterMonth,filterYear, orsortType, the order/contents offilteredNewsletterscan change while the stored indices still refer to the old ordering, so some cards may render as “already visible” or mismatched.You can cheaply realign things by re-running the init effect whenever filters or sort change:
- useEffect(() => { + useEffect(() => { const timer = setTimeout(() => { setLineHeight(100); setActiveDot(0); setVisibleCards(new Set([0])); setGlowingDot(0); setTimeout(() => setGlowingDot(null), 300); }, 100); return () => clearTimeout(timer); - }, []); + }, [filterMonth, filterYear, sortType]);This keeps the “first card is active and visible” behavior consistent with the current filter/sort order.
218-363: Add an empty‑state message when no newsletters match the filters.If the user selects a year/month combination with no data (e.g., 2024/2023), the timeline area goes blank without explanation. A small empty state will make this much clearer.
For example, just after the
filteredNewsletters.map(...)block:- <div className="relative space-y-8 md:space-y-24 pt-8"> - {filteredNewsletters.map((newsletter, index) => { + <div className="relative space-y-8 md:space-y-24 pt-8"> + {filteredNewsletters.map((newsletter, index) => { // ... - })} + })} + + {filteredNewsletters.length === 0 && ( + <div className="pt-8 text-center text-slate-400"> + No newsletters found for the selected filters. + </div> + )} </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/src/app/(main)/dashboard/newsletter/page.tsx(1 hunks)apps/web/src/data/newsletter.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/app/(main)/dashboard/newsletter/page.tsx (1)
apps/web/src/data/newsletter.ts (2)
Newsletter(1-10)newsletters(11-147)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/web/src/app/(main)/dashboard/newsletter/page.tsx (2)
208-215: Replace unsafe type assertion.The type assertion
as "newest" | "oldest"on line 210 is unsafe and bypasses TypeScript's type checking. Although the select options are controlled, explicit typing is preferred.Apply this diff:
<select value={sortType} - onChange={(e) => setSortType(e.target.value as "newest" | "oldest")} + onChange={(e) => setSortType(e.target.value as typeof sortType)} className="bg-[#15161A] border border-[#27272A] rounded-lg px-3 py-2" >Or use a more explicit handler:
+ onChange={(e) => { + const value = e.target.value; + if (value === "newest" || value === "oldest") { + setSortType(value); + } + }}
122-154: Fix IntersectionObserver to re-initialize when filtered newsletters change.The effect has an empty dependency array, so the observer is created only once on mount. When
filteredNewsletterschanges (via filter/sort changes), React renders a different set of cards and updatescardRefs.currentvia ref callbacks, but the observer never re-runs to observe the new cards. This causes newly filtered cards to not be tracked for visibility and animations.Apply this diff to re-initialize the observer when the filtered list changes:
return () => observer.disconnect(); - }, []); + }, [filteredNewsletters]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/src/app/(main)/dashboard/newsletter/page.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/app/(main)/dashboard/newsletter/page.tsx (1)
apps/web/src/data/newsletter.ts (2)
Newsletter(1-10)newsletters(11-147)
🔇 Additional comments (3)
apps/web/src/app/(main)/dashboard/newsletter/page.tsx (3)
29-52: LGTM: Deterministic date parsing implemented.The explicit
parseDatefunction with month name mapping correctly addresses the previous concern about browser-dependent date parsing. This ensures reliable date handling across all environments.
83-119: LGTM: Focus trap implemented correctly.The focus management handles both Tab and Shift+Tab to cycle focus within the modal, stores the previous active element for restoration, and properly cleans up event listeners. This provides good keyboard accessibility for the modal.
419-509: LGTM: Modal accessibility features properly implemented.The modal includes proper ARIA attributes (
role="dialog",aria-modal="true",aria-labelledby), has a focusable back button, and integrates with the focus trap and keyboard handlers. This addresses the previous accessibility review comments.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/web/src/app/(main)/dashboard/newsletter/page.tsx (1)
122-154: Recreate observer when filtered newsletters change.The IntersectionObserver has an empty dependency array, so it's never recreated when the newsletter list changes due to filtering or sorting. This causes the observer to watch stale DOM references, breaking the visibility/animation system when filters are applied.
Apply this diff:
return () => observer.disconnect(); - }, []); + }, [filteredNewsletters.length]);This ensures the observer is recreated whenever the filtered list changes, while avoiding unnecessary recreation when only the list order changes (since
.lengthis stable for reordered arrays).
🧹 Nitpick comments (4)
apps/web/src/app/(main)/dashboard/newsletter/page.tsx (4)
54-70: Consider memoizing the filtered newsletters.The filter and sort operations run on every render. While the current dataset is small, wrapping this in
useMemowould prevent unnecessary recomputation and follow React best practices.Apply this diff:
+ const filteredNewsletters = React.useMemo(() => { + return newsletters - const filteredNewsletters = newsletters .filter((n) => { const d = parseDate(n.date); const month = d.getMonth() + 1; const year = d.getFullYear(); const monthMatches = filterMonth === "all" || Number(filterMonth) === month; const yearMatches = filterYear === "all" || Number(filterYear) === year; return monthMatches && yearMatches; }) .sort((a, b) => { const da = parseDate(a.date).getTime(); const db = parseDate(b.date).getTime(); return sortType === "newest" ? db - da : da - db; }); + }, [filterMonth, filterYear, sortType]);
83-119: Consider guarding against empty focusable elements.The focus trap logic assumes there are focusable elements in the modal. While unlikely given the modal structure, adding a guard would make the code more robust.
Add a check after line 102:
const focusable = modal.querySelectorAll<HTMLElement>( 'button, a, input, textarea, select, [tabindex]:not([tabindex="-1"])' ); + if (focusable.length === 0) return; + const first = focusable[0]; const last = focusable[focusable.length - 1];
236-245: Derive year options from newsletter data.The year filter has hardcoded values (2025, 2024, 2023) that will become outdated as new newsletters are added. Deriving available years from the newsletter data ensures the filter stays current without code changes.
Add this logic before the return statement (around line 193):
const availableYears = React.useMemo(() => { const years = new Set( newsletters.map((n) => parseDate(n.date).getFullYear()) ); return Array.from(years).sort((a, b) => b - a); }, []);Then replace the hardcoded options:
<select value={filterYear} onChange={(e) => setFilterYear(e.target.value)} className="bg-[#15161A] border border-[#27272A] rounded-lg px-3 py-2" > <option value="all">All Years</option> - <option value="2025">2025</option> - <option value="2024">2024</option> - <option value="2023">2023</option> + {availableYears.map((year) => ( + <option key={year} value={year}> + {year} + </option> + ))} </select>
329-352: Consider moving animation calculations to the button click handler.The inline style at lines 341-350 calls
getBoundingClientRect()during render, which can be fragile if refs aren't ready. Consider calculating the animation start position in the button'sonClickhandler and storing it in state for more reliable animation behavior.Example approach:
const [expandingCardPosition, setExpandingCardPosition] = useState<{ top: number; left: number; width: number; height: number; } | null>(null); // In the button onClick: onClick={() => { const rect = cardRefs.current[index]?.getBoundingClientRect(); if (rect) { setExpandingCardPosition({ top: rect.top, left: rect.left, width: rect.width, height: rect.height, }); } setExpandingCard(index); setTimeout(() => { setSelectedNewsletter(newsletter); }, 600); }} // In the style prop: ...(expandingCard === index && expandingCardPosition && { ...expandingCardPosition, transition: "all 0.6s cubic-bezier(0.34, 1.56, 0.64, 1)", })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/src/app/(main)/dashboard/newsletter/page.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/app/(main)/dashboard/newsletter/page.tsx (1)
apps/web/src/data/newsletter.ts (2)
Newsletter(1-10)newsletters(11-147)
🔇 Additional comments (3)
apps/web/src/app/(main)/dashboard/newsletter/page.tsx (3)
29-52: LGTM! Date parsing is now deterministic.The explicit
parseDatefunction addresses the previous concern about browser-dependent date parsing. The implementation correctly maps month abbreviations to indices and constructs a Date object deterministically.
180-191: LGTM! Escape key handler properly usescloseModal().The handler correctly calls
closeModal()to ensure focus restoration, addressing the previous review feedback.
418-425: LGTM! Modal accessibility attributes properly implemented.The modal includes proper ARIA attributes (
role="dialog",aria-modal="true",aria-labelledby) that address the previous accessibility concerns.
1 participant
closes #155
Demo video :-
15.11.2025_17.11.19_REC.1.mp4
Mobile view :-
Key Changes :
All newsletter entries now live in /src/data/newsletter.ts
Adding future newsletters only requires appending to this file
Ensures clean separation of data from UI logic
Implemented sorting and filtering
Newsletters load sorted by latest date by default
Users can filter by: Month, Year
Dates must follow the format: Dec 10, 2025
(The sorting logic relies on this exact format.)
Smooth fade-in animation when cards enter the viewport
Circular glowing dot animation to indicate active card on scroll
Cards alternate left/right layout on larger screens
Fully responsive for mobile and desktop
Clicking a newsletter card opens a modal with: Full article content
Summary by CodeRabbit