feat: add newsletters feature #180
Conversation
|
@HarshJS30 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. |
WalkthroughAdds a newsletters feature: a listing page with filters and pro-only branch, a dynamic slug detail page that loads markdown and renders HTML, a client-side page transition animation, sample newsletter content and docs, data/loader utilities, a non‑pro CTA hero, and package dependency additions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Listing as NewslettersPage
participant Transition as PageTransition
participant Data as posts[] (newsletters.ts)
participant Loader as getNewsletterContent
participant FS as File System (markdown)
participant Renderer as remark -> HTML
User->>Listing: Open /newsletters
Listing->>Data: read posts metadata
Data-->>Listing: posts[]
Listing->>Listing: compute months/years, apply search/month/year filters
alt isPaidUser
Listing->>User: render full listing + sidebar
else
Listing->>User: render non‑pro CTA (News)
end
User->>Listing: Click post link
Listing->>Transition: start cover animation (blocks scaleX 0→1)
Transition->>Listing: on complete -> router.push(/newsletters/[slug])
User->>Loader: GET /newsletters/[slug]
Loader->>FS: read `apps/web/src/app/content/newsletters/[slug].md`
FS-->>Loader: markdown + front matter
Loader->>Renderer: parse → HTML
Renderer-->>Loader: htmlContent, meta
Loader-->>Listing: htmlContent, meta
Listing->>User: render post (cover image, meta, innerHTML)
Listing->>Transition: reveal animation (blocks scaleX 1→0)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Possibly related issues
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 3
🧹 Nitpick comments (6)
apps/web/src/data/newsletters.ts (2)
7-14: Minor style inconsistency.Missing trailing comma after
coverimgproperty on line 12.Apply this diff:
image: StaticImageData; - coverimg:StaticImageData, + coverimg: StaticImageData, slug: string;Also note the missing space after the colon on line 12.
16-45: Sample data uses past dates.The newsletter dates are from 2024, but the PR was created in November 2025. While this is acceptable for sample data, consider updating them to more recent dates for a production-ready feature.
apps/web/src/app/(main)/newsletters/pagetransition.tsx (1)
67-106: Consider performance optimization for link interception.The component attaches click listeners to all links on every pathname change. With many links, this could impact performance.
Consider these optimizations:
- Use event delegation on a parent container instead of individual link listeners
- Memoize
handleClickwithuseCallback- Only re-attach listeners when absolutely necessary
Example with event delegation:
useEffect(() => { createBlocks() reveal() - const links = document.querySelectorAll("a[href]") - const handleClick = (e: Event) => { - const target = e.currentTarget as HTMLAnchorElement + const handleClick = (e: Event) => { + const target = (e.target as HTMLElement).closest('a[href]') as HTMLAnchorElement + if (!target) return + const href = target.getAttribute("href") // ... rest of the logic } - links.forEach((l) => l.addEventListener("click", handleClick)) - return () => links.forEach((l) => l.removeEventListener("click", handleClick)) + document.addEventListener("click", handleClick) + return () => document.removeEventListener("click", handleClick) }, [pathname])apps/web/src/app/(main)/newsletters/page.tsx (3)
22-27: Remove the no-opuseEffectwatchingshowSidebar.This effect body does nothing and React already re-renders on
showSidebarchanges, so this hook just adds noise. You can safely delete it to simplify the component.
82-89: MakehasActiveFiltersexplicitly boolean.Right now
hasActiveFiltersholds a string or"", relying on truthiness when used in JSX. For clarity and type safety, it’s better to coerce to a boolean:- const hasActiveFilters = searchQuery || selectedMonth || selectedYear; + const hasActiveFilters = Boolean( + searchQuery || selectedMonth || selectedYear + );This avoids accidental type surprises if you later pass it around.
299-301: Use a stable key (e.g.post.slug) instead of the array index.Using
indexas the React key can cause unnecessary remounts when filters/search change the ordering, and you already have a unique slug per post:- filteredPosts.map((post, index) => ( + filteredPosts.map((post) => ( <div - key={index} + key={post.slug}This keeps component identity stable across re-sorts and filter changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
apps/web/src/assets/images/gsoc.pngis excluded by!**/*.pngapps/web/src/assets/images/opensox.jpgis excluded by!**/*.jpgapps/web/src/assets/images/photu.jpgis excluded by!**/*.jpg
📒 Files selected for processing (9)
apps/web/src/app/(main)/newsletters/[slug]/page.tsx(1 hunks)apps/web/src/app/(main)/newsletters/page.tsx(1 hunks)apps/web/src/app/(main)/newsletters/pagetransition.tsx(1 hunks)apps/web/src/app/content/newsletters/getting-started-with-nextjs.md(1 hunks)apps/web/src/app/content/newsletters/mastering-react-hooks.md(1 hunks)apps/web/src/app/content/newsletters/understanding-typescript.md(1 hunks)apps/web/src/components/non-pro-news/News.tsx(1 hunks)apps/web/src/data/newsletters.ts(1 hunks)apps/web/src/lib/newslettercontent.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
apps/web/src/components/non-pro-news/News.tsx (1)
apps/web/src/components/icons/icons.tsx (1)
Terminal(1-5)
apps/web/src/app/(main)/newsletters/[slug]/page.tsx (3)
apps/web/src/data/newsletters.ts (1)
posts(16-45)apps/web/src/lib/newslettercontent.ts (1)
getNewsletterContent(8-45)apps/web/src/app/(main)/newsletters/pagetransition.tsx (1)
PageTransition(7-124)
apps/web/src/app/(main)/newsletters/page.tsx (6)
apps/web/src/store/useShowSidebar.ts (1)
useShowSidebar(10-16)apps/web/src/hooks/useSubscription.ts (1)
useSubscription(11-77)apps/web/src/data/newsletters.ts (1)
posts(16-45)apps/web/src/components/non-pro-news/News.tsx (1)
News(5-42)apps/web/src/components/ui/IconWrapper.tsx (1)
IconWrapper(10-22)apps/web/src/app/(main)/newsletters/pagetransition.tsx (1)
PageTransition(7-124)
🪛 ast-grep (0.39.9)
apps/web/src/app/(main)/newsletters/[slug]/page.tsx
[warning] 78-78: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
apps/web/src/app/(main)/newsletters/[slug]/page.tsx
[error] 79-79: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🪛 LanguageTool
apps/web/src/app/content/newsletters/getting-started-with-nextjs.md
[grammar] ~47-~47: Use a hyphen to join words.
Context: ... and Routing Next.js uses a file-system based router. Simply create a file in th...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
apps/web/src/app/content/newsletters/getting-started-with-nextjs.md
31-31: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (7)
apps/web/src/app/content/newsletters/mastering-react-hooks.md (1)
1-796: Comprehensive newsletter content.This is a well-structured and thorough guide on React Hooks covering essential concepts, performance optimization, custom hooks, and best practices. The code examples are clear and the content aligns well with the newsletter feature requirements.
apps/web/src/lib/newslettercontent.ts (1)
8-45: Well-implemented content loader with proper security measures.The function correctly validates the slug to prevent path traversal attacks and handles errors appropriately. Since the content is team-managed (as per PR objectives), the markdown-to-HTML conversion via remark is sufficient for this use case.
Note: If you plan to accept user-generated content in the future, consider adding HTML sanitization using a library like DOMPurify.
apps/web/src/app/content/newsletters/understanding-typescript.md (1)
1-513: High-quality TypeScript guide.Comprehensive TypeScript documentation covering all essential concepts with clear examples. The content is well-organized and provides valuable information for developers.
apps/web/src/app/(main)/newsletters/[slug]/page.tsx (2)
9-17: Correct Next.js 15 async params handling.The component correctly handles async params by awaiting them and implements proper 404 handling for missing posts.
77-81: UsingdangerouslySetInnerHTMLwith team-managed content.While static analysis tools flag
dangerouslySetInnerHTML, this usage is acceptable because:
- Content comes from team-managed markdown files (per PR objectives)
- HTML is generated via remark, which provides basic safety
- No user-generated content is involved
If you plan to accept content from untrusted sources in the future, consider adding HTML sanitization using DOMPurify.
apps/web/src/app/(main)/newsletters/pagetransition.tsx (1)
16-31: Smooth page transition implementation.The block-based transition effect is well-implemented with proper initialization and cleanup. The staggered animation provides a polished user experience.
apps/web/src/components/non-pro-news/News.tsx (1)
1-1: Clarify intent regarding unused custom Terminal icon component.While a custom Terminal icon exists at
apps/web/src/components/icons/icons.tsx, it's completely unused across the codebase. The lucide-react Terminal has been standardized and is actively used in 5 files, including News.tsx. Additionally, the implementations differ: the custom Terminal has a hardcodedclassName="h-6 w-6", while News.tsx needsclassName="w-5 h-5".Before suggesting reuse, verify whether:
- The custom Terminal is legacy dead code that should be removed, or
- A conscious decision was made to standardize on lucide-react
apps/web/src/app/content/newsletters/getting-started-with-nextjs.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
apps/web/src/app/content/newsletters/getting-started-with-nextjs.md (1)
47-47: Minor style suggestion: hyphenate compound adjective.The phrase "file-system based router" could be "file-system-based router" for consistency with standard compound adjective formatting, though both forms are acceptable.
apps/web/src/app/(main)/newsletters/page.tsx (4)
31-36: Remove unnecessary useEffect.This effect serves no purpose—the component already re-renders when
showSidebarchanges because it's referenced in the JSX. The effect body is empty and can be safely deleted.Apply this diff:
- // Sidebar auto-hide hamburger when open - useEffect(() => { - if (showSidebar) { - // nothing needed, this triggers rerender & hides hamburger - } - }, [showSidebar]); -
100-168: Consider extracting repeated sidebar/hamburger logic.The sidebar, mobile overlay, and hamburger button code is duplicated across three UI branches (loading, non-Pro, Pro). Extracting this to a shared component or render function would improve maintainability.
For example, create a layout wrapper:
function NewsletterLayout({ children, showSidebar, setShowSidebar }: { children: React.ReactNode; showSidebar: boolean; setShowSidebar: (value: boolean) => void; }) { return ( <div className="flex w-screen h-screen bg-[#0a0a0b] overflow-hidden"> <aside className={`h-full z-50 ${!showSidebar && "hidden xl:block"}`}> <Sidebar /> </aside> {showSidebar && ( <div onClick={() => setShowSidebar(false)} className="fixed inset-0 bg-black/40 backdrop-blur-sm xl:hidden z-40" /> )} {!showSidebar && ( <button onClick={() => setShowSidebar(true)} className="xl:hidden fixed top-4 left-4 z-50 w-12 h-12 rounded-lg bg-[#1a1a1d] border border-white/10 flex items-center justify-center hover:bg-[#2a2a2d] transition-colors" > <Bars3Icon className="size-5 text-ox-purple" /> </button> )} {children} </div> ); }
308-347: Use post.slug instead of index as key.Using array indices as keys can cause issues with React's reconciliation, especially if the list order changes. Since each post has a unique
slug, use that instead.Apply this diff:
- <div - key={index} - className="flex mb-[50px] relative top-[-80px] max-[1279px]:top-[-60px] max-[1024px]:static max-[1024px]:flex-col max-[1024px]:mb-10 max-[768px]:flex-col max-[768px]:items-start max-[768px]:mt-[30px]" - > + <div + key={post.slug} + className="flex mb-[50px] relative top-[-80px] max-[1279px]:top-[-60px] max-[1024px]:static max-[1024px]:flex-col max-[1024px]:mb-10 max-[768px]:flex-col max-[768px]:items-start max-[768px]:mt-[30px]" + >
236-288: Consider adding ARIA labels for better accessibility.The search input and filter dropdowns lack explicit labels. While placeholders provide visual hints, ARIA labels improve screen reader experience.
Example additions:
<input type="text" placeholder="Search newsletters..." + aria-label="Search newsletters" value={searchQuery}<select value={selectedMonth} + aria-label="Filter by month" onChange={(e) => setSelectedMonth(e.target.value)}<select value={selectedYear} + aria-label="Filter by year" onChange={(e) => setSelectedYear(e.target.value)}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/package.json(1 hunks)apps/web/src/app/(main)/newsletters/page.tsx(1 hunks)apps/web/src/app/content/newsletters/getting-started-with-nextjs.md(1 hunks)apps/web/src/app/docs/how-to-add-newsletters.md(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/src/app/docs/how-to-add-newsletters.md
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/app/(main)/newsletters/page.tsx (6)
apps/web/src/store/useShowSidebar.ts (1)
useShowSidebar(10-16)apps/web/src/hooks/useSubscription.ts (1)
useSubscription(11-77)apps/web/src/data/newsletters.ts (1)
posts(16-45)apps/web/src/components/non-pro-news/News.tsx (1)
News(5-42)apps/web/src/components/ui/IconWrapper.tsx (1)
IconWrapper(10-22)apps/web/src/app/(main)/newsletters/pagetransition.tsx (1)
PageTransition(7-124)
🪛 LanguageTool
apps/web/src/app/content/newsletters/getting-started-with-nextjs.md
[grammar] ~47-~47: Use a hyphen to join words.
Context: ... and Routing Next.js uses a file-system based router. Simply create a file in th...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (3)
apps/web/src/app/(main)/newsletters/page.tsx (2)
19-22: LGTM! Date parsing correctly handles timezone issues.The
parsePostDatefunction properly parses "YYYY-MM-DD" strings as local dates, preventing the timezone-dependent off-by-one errors that were flagged in previous reviews.
38-89: LGTM! Filtering and sorting logic is well-implemented.The memoized computations correctly use
parsePostDatethroughout for consistent date handling, and the spread operator before sorting (line 86) properly avoids mutation.apps/web/package.json (1)
30-30: GSAP version 3.13.0 is confirmed secure and current.Verification shows GSAP 3.13.0 is the latest stable version, and while a HIGH severity prototype pollution vulnerability existed in versions < 3.6.0 (published 2021), it has been patched since version 3.6.0. The specified version is well above that threshold and safe to use.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/web/src/components/dashboard/Sidebar.tsx (3)
114-151: Route mapping looks good; consider reducing Pro CTA duplicationThe sidebar body refactor to map
SIDEBAR_ROUTESinto<Link><SidebarItem/></Link>is clean and keeps nav items declarative.For the Pro CTA block:
- The custom
<div>for!isCollapsed && !isPaidUserclosely duplicates the structure and classes ofSidebarItem, whileSidebarItemitself already supports abadgeprop (perSidebarItem.tsx).- You could simplify by always rendering a single
SidebarItemand toggling the badge based onisPaidUser, which would remove duplicated layout/hover styling and keep everything going through one component.For example:
- {!isCollapsed && !isPaidUser ? ( - <div - className="w-full h-[44px] flex items-center rounded-md cursor-pointer px-2 gap-3 pl-3 hover:bg-[#292929] group" - onClick={proClickHandler} - > - <span className="text-[#eaeaea] group-hover:text-white transition-colors"> - <StarIcon className="size-5" /> - </span> - <div className="flex items-center gap-1"> - <h1 className="text-xs font-medium text-[#c8c8c8] group-hover:text-white transition-colors"> - Opensox Pro - </h1> - <OpensoxProBadge className="px-1.5 py-0.5 scale-75" /> - </div> - </div> - ) : ( - <SidebarItem - itemName="Opensox Pro" - onclick={proClickHandler} - icon={<StarIcon className="size-5" />} - collapsed={isCollapsed} - /> - )} + <SidebarItem + itemName="Opensox Pro" + onclick={proClickHandler} + icon={<StarIcon className="size-5" />} + collapsed={isCollapsed} + badge={ + !isPaidUser ? ( + <OpensoxProBadge className="px-1.5 py-0.5 scale-75" /> + ) : null + } + />Functionally this keeps Pro prominent for non‑paid users while simplifying the markup.
168-176: Harden click‑outside handler against non‑HTMLElement targetsThe click‑outside logic is correct in spirit and attaches/removes the listener only when
openis true, which is good. To make it more robust against rare cases whereevent.targetisn’t anHTMLElement(e.g., certain text nodes), you can guard theclosestcall:React.useEffect(() => { - const handler = (e: MouseEvent) => { - if (open && !(e.target as HTMLElement).closest(".profile-menu-container")) { - setOpen(false); - } - }; + const handler = (e: MouseEvent) => { + if (!open) return; + const target = e.target; + if (!(target instanceof HTMLElement)) return; + + if (!target.closest(".profile-menu-container")) { + setOpen(false); + } + }; if (open) document.addEventListener("mousedown", handler); return () => document.removeEventListener("mousedown", handler); }, [open]);This keeps behavior the same while avoiding potential runtime issues if a non‑element target ever occurs.
179-250: Profile dropdown UX is solid; consider minor semantics/a11y tweaksThe profile card + dropdown look well‑structured:
profile-menu-container+ absolute dropdown positioning give a predictable anchor.AnimatePresence+motion.divprovide clean enter/exit animations.- Using the same
ProfilePic,fullName, anduserEmailin both the trigger and dropdown header keeps identity clear.- Logout now uses
ArrowRightOnRectangleIcon, which reads nicely in the menu.Two minor, non‑blocking suggestions:
Trigger semantics
The clickable profile trigger is adivwithonClick. For better accessibility and keyboard interaction, consider making it a<button type="button">with appropriatearia-expandedandaria-haspopup="menu"attributes.Buttons inside the dropdown
The “Account Settings” and “Logout” actions are correctly implemented as<button>elements. You might optionally addtype="button"to make their intent explicit, especially if this component is ever used inside a<form>.These are polish items; behavior as written is fine.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
apps/web/package.json(2 hunks)apps/web/src/components/dashboard/Sidebar.tsx(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/package.json
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/components/dashboard/Sidebar.tsx (3)
apps/web/src/components/ui/IconWrapper.tsx (1)
IconWrapper(10-22)apps/web/src/components/sidebar/SidebarItem.tsx (1)
SidebarItem(13-36)apps/web/src/components/dashboard/ProfilePic.tsx (1)
ProfilePic(11-59)
🔇 Additional comments (3)
apps/web/src/components/dashboard/Sidebar.tsx (3)
9-28: Icon imports and library mix look consistentImporting
NewspaperIconandArrowRightOnRectangleIconfrom@heroicons/react/24/outlineand chevrons fromlucide-reactis fine and keeps icons grouped logically. Just be mindful of visual consistency between the two icon sets; if you see mismatched stroke weights in the UI, it may be worth standardizing on one library for header/profile chevrons.
29-50: Newsletters route wiring and Pro routing logicThe new
SIDEBAR_ROUTESentry forpath: "/newsletters"withNewspaperIconis wired cleanly and will automatically participate in the sidebar mapping. The simplifiedproClickHandlerternary is also clearer than branching logic.Two things to double‑check:
- That the newsletters listing page actually lives at
/newsletters(and uses the same dashboard shell), since other items are under/dashboard/....- That
/dashboard/pro/dashboardis still the correct Pro destination; if this route ever changes, this constant will be easy to miss.Otherwise, this config looks solid.
Also applies to: 61-63
80-112: Mobile and desktop headers are cleaner and more consistentThe revised mobile and desktop headers (brand link + collapse toggle with lucide chevrons) are straightforward and visually consistent:
- Using
<Link href="/"> for the brand on both breakpoints is good for UX.- The collapse toggle logic is easy to follow and the
IconWrapperusage keeps hit‑area and styling consistent.- Background/border classes (
bg-ox-sidebar,border-ox-header) align with the rest of the shell.No functional issues from what I can see.
|
@HarshJS30 thanks for the submission! Unfortunately, we are moving with a different submission this time, so we won't be able to accept it. Still, you are welcome to make contributions! 🙏 |
2 participants
Summary
Fixes #155
This PR implements a complete, production-ready newsletters experience for Pro users on OpenSox.
The goal was to build something clean, scalable, and long-term friendly rather than a one-off implementation.
🎯 Features Implemented
1. Premium-Only Access
2. Scalable Architecture
Key Design Decision: Metadata/content separation for performance at scale
How it works:
src/data/newsletters.tssrc/content/newsletters/*.mdWhy this matters:
As newsletters grow to 50+, the listing page stays fast because we're not bundling all markdown content into the client.
This architecture prevents performance degradation as content scales.
3. Newsletter Listing Page
4. Markdown-Based Content System
Tech stack:
gray-matter- YAML frontmatter parsingremark- Markdown to HTML conversionSupported formatting:
Developer benefit: Writing newsletters is as simple as creating a
.mdfile. No UI changes needed for new content.5. Article Page (Slug-Based Routing)
6. GSAP Page Transitions
📁 File Structure
📝 Content Management Workflow
Adding a new newsletter takes ~2 minutes:
.mdfile insrc/content/newsletters/src/data/newsletters.tsNo database. No CMS. No admin panel.
Full step-by-step guide included in:
src/app/docs/how-to-add-newsletter.md✅ Testing Checklist
🎬 Demo
rec.mp4
📸 Screenshots
Non-Pro Landing Page
📦 Sample Data
Three example newsletters included for testing:
Content files:
Metadata:
All sample newsletters include:
💭 Design Decisions
Why Markdown instead of a CMS?
Why separate metadata from content?
Future Enhancements (Out of Scope)
Potential additions for later iterations:
📋 Submission Checklist
Closing Note
I've focused on building something genuinely scalable rather than just meeting the minimum requirements. The architecture supports growth without performance degradation, the markdown-based system makes content management trivial, and the code is clean and maintainable.
I've really enjoyed working on this feature, and if things work out, I'd love the opportunity to contribute more and work with you in the future.
Looking forward to your feedback!
Summary by CodeRabbit
New Features
Documentation
UI