Feat: Newsletters #174
Feat: Newsletters #174
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: listing and detail pages with markdown content, a markdown-to-HTML loader, page transition animation that intercepts internal links, sample newsletter data and UI, docs, Tailwind typography plugin and markdown/runtime deps, and sidebar/layout animation/overlay changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Browser
participant PageTransition
participant Router
participant Server
User->>Browser: click internal newsletter link
Browser->>PageTransition: anchor click intercepted
PageTransition->>PageTransition: run cover() animation (cover → left→right)
PageTransition->>Router: router.push(url)
Router->>Server: request /newsletters/[slug]
Server->>Server: getNewsletterContent(slug) -> read file, gray-matter, remark→HTML
Server-->>Router: htmlContent + meta (or 404)
Router->>PageTransition: render new page
PageTransition->>PageTransition: run reveal() animation (reveal → right→left)
PageTransition-->>Browser: display rendered newsletter
sequenceDiagram
autonumber
participant User
participant NewslettersPage
participant Posts as PostsData
participant Filters as FilterState
User->>NewslettersPage: load /newsletters
NewslettersPage->>Posts: derive months & years (memoized)
User->>Filters: set search / month / year
Filters->>NewslettersPage: update filter state
NewslettersPage->>Posts: filter & sort posts (date desc)
NewslettersPage-->>User: render filtered list
User->>NewslettersPage: click "Read more"
NewslettersPage->>PageTransition: start cover animation -> navigate to detail
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (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. 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: 4
🧹 Nitpick comments (6)
apps/web/src/components/dashboard/Sidebar.tsx (2)
17-19: Remove unused icon imports.
HeartIconandEnvelopeIconare imported but never used in this file. OnlyNewspaperIconis actually utilized for the Newsletters navigation entry.Apply this diff to remove the unused imports:
SparklesIcon, StarIcon, - HeartIcon, - EnvelopeIcon, NewspaperIcon, DocumentTextIcon,
122-122: Consider removing the comment marker.The comment
{/* Find projects entry */}appears to be a leftover development note and doesn't add meaningful context since the code below is self-explanatory with the routes mapping.apps/web/src/components/non-pro-news/News.tsx (1)
1-1: Prefer the existing custom Terminal icon for consistency.The codebase already has a custom
Terminalicon component atsrc/components/icons/icons.tsx. Importing fromlucide-reactcreates an unnecessary duplication and potential inconsistency in icon styling across the application.Apply this diff to use the custom icon:
-import { Terminal } from "lucide-react"; +import { Terminal } from "../icons/icons";apps/web/src/app/content/newsletters/understanding-typescript.md (1)
514-514: Update the author placeholder.The author field contains a placeholder
[Your Name]that should be replaced with the actual author name before publishing.apps/web/src/app/content/newsletters/getting-started-with-nextjs.md (1)
47-47: Minor: Hyphenate the compound adjective.The phrase "file-system based router" should use a hyphen: "file-system-based router" when used as a compound adjective before a noun.
-Next.js uses a file-system based router. Simply create a file in the `app` directory: +Next.js uses a file-system-based router. Simply create a file in the `app` directory:apps/web/src/data/newsletters.ts (1)
12-12: Add space after colon for consistent formatting.The
coverimgproperty is missing a space after the colon, which is inconsistent with TypeScript formatting conventions.Apply this diff:
- coverimg:StaticImageData, + coverimg: StaticImageData,
📜 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 (13)
apps/web/package.json(2 hunks)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/app/docs/how-to-add-newsletter.md(1 hunks)apps/web/src/components/dashboard/Sidebar.tsx(3 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)apps/web/tailwind.config.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/page.tsx (7)
apps/web/src/store/useShowSidebar.ts (1)
useShowSidebar(10-16)apps/web/src/hooks/useSubscription.ts (1)
useSubscription(11-77)apps/web/src/components/dashboard/Sidebar.tsx (1)
Sidebar(57-170)apps/web/src/components/non-pro-news/News.tsx (1)
News(5-42)apps/web/src/data/newsletters.ts (1)
posts(16-45)apps/web/src/components/ui/IconWrapper.tsx (1)
IconWrapper(10-22)apps/web/src/app/(main)/newsletters/pagetransition.tsx (1)
PageTransition(7-100)
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-20)apps/web/src/app/(main)/newsletters/pagetransition.tsx (1)
PageTransition(7-100)
🪛 ast-grep (0.39.9)
apps/web/src/app/(main)/newsletters/[slug]/page.tsx
[warning] 71-71: 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] 72-72: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
apps/web/package.json
[error] 48-49: unexpected character <
(parse)
[error] 49-49: unexpected character <
(parse)
[error] 49-49: unexpected character <
(parse)
[error] 49-49: unexpected character <
(parse)
[error] 49-49: unexpected character <
(parse)
[error] 49-49: unexpected character <
(parse)
[error] 49-49: unexpected character <
(parse)
[error] 49-49: String values must be double quoted.
(parse)
[error] 49-49: String values must be double quoted.
(parse)
[error] 50-50: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 50-50: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 50-50: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 50-51: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 50-51: unexpected character =
(parse)
[error] 51-51: unexpected character =
(parse)
[error] 51-51: unexpected character =
(parse)
[error] 51-51: unexpected character =
(parse)
[error] 51-51: unexpected character =
(parse)
[error] 51-51: unexpected character =
(parse)
[error] 51-51: unexpected character =
(parse)
[error] 52-52: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 52-52: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 52-52: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 52-53: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 52-53: unexpected character >
(parse)
[error] 53-53: unexpected character >
(parse)
[error] 53-53: unexpected character >
(parse)
[error] 53-53: unexpected character >
(parse)
[error] 53-53: unexpected character >
(parse)
[error] 53-53: unexpected character >
(parse)
[error] 53-53: unexpected character >
(parse)
🪛 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 (9)
apps/web/src/app/content/newsletters/mastering-react-hooks.md (2)
1-50: Verify front-matter/metadata requirements.The markdown file lacks front-matter (YAML/gray-matter metadata block). Per the PR objectives, metadata and content are separated—metadata lives in
src/data/newsletters.tswhile content is in markdown files. Verify that this architecture means front-matter is not required here, or if the markdown files should include metadata such as title, date, author, and description for the content processing pipeline.
1-797: Content quality and technical accuracy are solid.The newsletter provides comprehensive, well-structured React Hooks documentation with accurate code examples (TypeScript/JavaScript). Sections cover rules, essential hooks, performance hooks, advanced hooks, custom hooks, patterns, best practices, common mistakes, testing, and resources. Examples are syntactically correct and demonstrate best practices. Structure is logical and suitable for educational content.
apps/web/src/app/docs/how-to-add-newsletter.md (1)
1-110: LGTM!The documentation is clear, comprehensive, and accurately describes the newsletter creation workflow. The step-by-step guide aligns well with the implementation in
src/data/newsletters.tsand the content loading utilities.apps/web/src/components/non-pro-news/News.tsx (1)
5-42: LGTM!The component structure is clean with proper responsive design using Tailwind classes. The gradient effects, layout hierarchy, and CTA button integration are well implemented.
apps/web/src/app/content/newsletters/understanding-typescript.md (1)
1-513: LGTM!The TypeScript tutorial is comprehensive, well-structured, and provides clear examples covering fundamental to advanced concepts. The content quality is excellent for a newsletter article.
apps/web/src/app/content/newsletters/getting-started-with-nextjs.md (1)
1-187: LGTM!The Next.js tutorial provides clear, practical guidance with good code examples. The content structure flows well from basics to advanced topics and deployment.
apps/web/tailwind.config.ts (1)
125-127: LGTM!The addition of the
@tailwindcss/typographyplugin is correctly implemented and will provide prose styling utilities needed for rendering the markdown newsletter content.apps/web/src/data/newsletters.ts (1)
1-45: LGTM!The
Postinterface andpostsarray are well-structured with proper TypeScript typing. The slug values correctly match the markdown filenames, and the data model aligns well with the content loading pipeline documented inhow-to-add-newsletter.md.apps/web/package.json (1)
30-31: Dependencies verified as secure and stable.All newly added dependencies are using their latest stable versions with no known vulnerabilities:
- gray-matter ^4.0.3: No CVEs
- gsap ^3.13.0: Secure (prototype pollution vulnerability only affects < 3.6.0)
- remark 15.0.1: No CVEs
- remark-html 16.0.1: Secure (critical vulnerabilities only affect ≤14.0.0 and <13.0.2)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apps/web/src/app/(main)/newsletters/[slug]/page.tsx (3)
31-43: Specify explicit positioning for fixed back button.The back button uses
fixedpositioning without explicittoporleftvalues, which defaults to the element's static position. This could lead to inconsistent placement across browsers or viewport sizes.Apply this diff to add explicit positioning:
<div className=" fixed + top-4 left-4 flex items-center bg-[#9455f4] rounded-2xl text-[15px] px-3 py-1.5 ">
46-56: Improve image alt text for accessibility.The cover image uses a generic alt text "img" which provides no meaningful information for screen readers or when images fail to load.
Apply this diff to use descriptive alt text:
<Image src={post.coverimg} width={650} - alt="img" + alt={`Cover image for ${post.heading}`} className=" mx-auto mt-10 mb-16 max-w-full lg:w-[650px] " />
77-80: Consider sanitizing HTML content for defense-in-depth security.DOMPurify is already available in your dependencies. While the markdown content originates from repository-controlled files and is processed by remark (a trusted library), adding explicit HTML sanitization provides an additional security layer against potential supply-chain attacks or markdown processor bugs.
Update the component to sanitize before rendering:
import DOMPurify from 'dompurify'; // In the component: const sanitizedContent = DOMPurify.sanitize(htmlContent); // ... <div dangerouslySetInnerHTML={{ __html: sanitizedContent }} />This is an optional best practice; your current implementation is secure given the trusted content source.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/package.json(3 hunks)apps/web/src/app/(main)/newsletters/[slug]/page.tsx(1 hunks)apps/web/src/app/(main)/newsletters/pagetransition.tsx(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)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/app/content/newsletters/mastering-react-hooks.md
- apps/web/src/app/(main)/newsletters/pagetransition.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
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-20)apps/web/src/app/(main)/newsletters/pagetransition.tsx (1)
PageTransition(7-118)
🪛 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)
🔇 Additional comments (5)
apps/web/package.json (2)
49-50: ✅ Git merge conflict resolved.The previous critical issue with unresolved merge conflict markers has been properly resolved. Both
@types/dompurifyand@tailwindcss/typographyare now correctly present in devDependencies without conflict markers.
30-31: Verification confirms package security status—but verify gray-matter usage patterns.Security findings:
- gsap@3.13.0, remark@15.0.1, remark-html@16.0.1, and @tailwindcss/typography@0.5.19 have no known vulnerabilities.
- gray-matter@4.0.3 itself has no direct CVE, but known advisories describe unsafe usage patterns (CVE-2022-25863 / CVE-2023-22491) involving JavaScript deserialization of untrusted frontmatter.
Action required: Confirm that the newsletter implementation processes frontmatter from trusted sources only (e.g., author-controlled markdown files). If processing untrusted user-submitted content, disable JavaScript execution in gray-matter's parser.
apps/web/src/app/content/newsletters/understanding-typescript.md (1)
1-514: Well-structured TypeScript tutorial with comprehensive coverage.The newsletter content is well-organized, covers essential TypeScript concepts progressively, and includes practical code examples. The structure flows logically from basics to advanced topics, and the inclusion of React integration and external resources adds value.
apps/web/src/app/(main)/newsletters/[slug]/page.tsx (2)
12-17: Excellent! Past issue resolved.The missing slug validation has been properly implemented as suggested in the previous review. The code now checks if the post exists and returns a 404 via
notFound()before attempting to access post properties or load the markdown file.
9-86: Clean implementation of newsletter detail page.The component structure is well-organized with proper async/await handling, responsive design considerations, and good reading experience optimizations (70% width constraint on large screens with prose styling). The integration with PageTransition provides smooth navigation.
apps/web/src/app/content/newsletters/understanding-typescript.md
Outdated
Show resolved
Hide resolved
apps/web/src/app/content/newsletters/understanding-typescript.md
Outdated
Show resolved
Hide resolved
HarshJS30
force-pushed
the
feat/newsletters
branch
from
2509fcd to
6c90a26
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/components/dashboard/Sidebar.tsx (1)
83-95: Fix Tailwind class typo on mobile header background.The class name
bg-ox-sidebaat line 83 ofapps/web/src/components/dashboard/Sidebar.tsxis a misspelling of the defined Tailwind tokenbg-ox-sidebar(confirmed intailwind.config.ts). The mobile sidebar header will not render with the intended background color.- <div className="flex justify-between items-center h-16 px-4 border-b border-ox-header xl:hidden bg-ox-sideba"> + <div className="flex justify-between items-center h-16 px-4 border-b border-ox-header xl:hidden bg-ox-sidebar">
♻️ Duplicate comments (1)
apps/web/src/app/(main)/newsletters/pagetransition.tsx (1)
71-99: Still hijacking modifier/middle‑clicks on links (breaks open‑in‑new‑tab behavior).The guards for hashes, external URLs, and
target="_blank"look good, but this handler still intercepts Cmd/Ctrl/Shift/Alt clicks and non‑left mouse buttons on internal links. That prevents users from opening newsletter links in a new tab or background window, which is what the earlier review comment was warning about.Consider tightening the handler to only intercept plain left‑clicks on same‑origin internal links, e.g.:
- const handleClick = (e: Event) => { - const target = e.currentTarget as HTMLAnchorElement - const href = target.getAttribute("href") - - // don't hijack external links, new tabs, or modified clicks - if (!href || href.startsWith("#")) { - return - } - - // don't hijack external URLs - if (href.startsWith("http") || href.startsWith("mailto") || href.startsWith("tel")) { - return - } - - // don't hijack links that open in new tab - if (target.target === "_blank") { - return - } - - // only hijack internal navigation - if (href !== pathname) { - e.preventDefault() - cover(href) - } - } + const handleClick = (e: MouseEvent) => { + const target = e.currentTarget as HTMLAnchorElement + const href = target.getAttribute("href") ?? "" + + // let browser handle same‑page anchors, external URLs, new‑tab or modified clicks + if ( + !href || + href.startsWith("#") || + target.target === "_blank" || + target.rel?.includes("external") || + /^(https?:|mailto:|tel:)/.test(href) || + e.button !== 0 || + e.metaKey || + e.ctrlKey || + e.shiftKey || + e.altKey + ) { + return + } + + if (href !== pathname) { + e.preventDefault() + cover(href) + } + }This keeps the transition animation for normal internal navigation but restores standard browser behaviors for everything else.
🧹 Nitpick comments (5)
apps/web/src/data/newsletters.ts (1)
7-14: Minor formatting: add space after colon in interface.Line 12 is missing a space after the colon:
coverimg:StaticImageData,should becoverimg: StaticImageData,for consistency with other properties.Apply this diff:
export interface Post { date: string; heading: string; description: string; image: StaticImageData; - coverimg:StaticImageData, + coverimg: StaticImageData, slug: string; }apps/web/src/app/content/newsletters/getting-started-with-nextjs.md (1)
31-41: Optional: specify language for fenced code block.The directory tree structure at lines 31-41 uses a fenced code block without a language specifier. While this works, adding a language hint (e.g.,
textorplaintext) improves accessibility and potential syntax highlighting.Based on static analysis hints.
Apply this diff:
-``` +```text my-next-app/ ├── app/ │ ├── layout.tsxapps/web/src/app/(main)/newsletters/[slug]/page.tsx (1)
78-80: Consider using a React markdown renderer to avoid dangerouslySetInnerHTML.While the current implementation works for controlled content (team-only markdown files), using
dangerouslySetInnerHTMLbypasses React's XSS protection. Consider using a React markdown component likereact-markdownor sanitizing with DOMPurify for defense-in-depth.Since the content source is controlled (committed markdown files, not user input), the immediate risk is low.
Based on static analysis hints.
Example with react-markdown:
import ReactMarkdown from 'react-markdown' // Replace dangerouslySetInnerHTML with: <div className="prose prose-invert max-w-none"> <ReactMarkdown>{content}</ReactMarkdown> </div>Or sanitize with DOMPurify:
import DOMPurify from 'dompurify' // In component: const sanitizedHtml = DOMPurify.sanitize(htmlContent) <div dangerouslySetInnerHTML={{ __html: sanitizedHtml }} />apps/web/src/app/(main)/newsletters/page.tsx (2)
221-243: Avoid nesting a<button>inside Next.js<Link>; style the anchor directly instead.Wrapping a
<button>inside the anchor thatLinkrenders is invalid HTML and can cause inconsistent focus/interaction behavior. You can get the same visual result by styling the<a>element fromLinkas a button:- <Link href={`/newsletters/${post.slug}`}> - <button className="bg-[#9455f4] text-[15px] py-[5px] px-[10px] rounded-[20px] relative top-[30px] font-medium w-[130px] cursor-pointer transition-all duration-300 ease-in-out hover:bg-[#7c3dd8] max-[1024px]:static max-[1024px]:mt-[10px]"> - Read more... - </button> - </Link> + <Link + href={`/newsletters/${post.slug}`} + className="inline-block bg-[#9455f4] text-[15px] py-[5px] px-[10px] rounded-[20px] relative top-[30px] font-medium w-[130px] text-center cursor-pointer transition-all duration-300 ease-in-out hover:bg-[#7c3dd8] max-[1024px]:static max-[1024px]:mt-[10px]" + > + Read more... + </Link>
221-232: Consider givingImagemeaningful dimensions instead ofwidth={0}/height={0}.Using
width={0}andheight={0}with CSS sizing works but is a bit of a hack and may trip Next Image warnings. Sincepostsimages are static imports, you can pass their intrinsic dimensions or switch tofill:- <Image - src={post.image} - alt={post.heading} - width={0} - height={0} - className="object-cover shrink-0 w-[220px] h-[380px] border border-white ..." - /> + <Image + src={post.image} + alt={post.heading} + width={220} + height={380} + className="object-cover shrink-0 w-[220px] h-[380px] border border-white ..." + />(or adjust the numbers to match the actual aspect ratio).
📜 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 (15)
apps/web/package.json(3 hunks)apps/web/src/app/(main)/dashboard/layout.tsx(2 hunks)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/app/docs/how-to-add-newsletter.md(1 hunks)apps/web/src/components/dashboard/Sidebar.tsx(9 hunks)apps/web/src/components/non-pro-news/News.tsx(1 hunks)apps/web/src/components/sidebar/SidebarItem.tsx(1 hunks)apps/web/src/data/newsletters.ts(1 hunks)apps/web/src/lib/newslettercontent.ts(1 hunks)apps/web/tailwind.config.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/web/src/app/content/newsletters/understanding-typescript.md
- apps/web/src/components/non-pro-news/News.tsx
- apps/web/tailwind.config.ts
- apps/web/src/app/docs/how-to-add-newsletter.md
🧰 Additional context used
🧬 Code graph analysis (4)
apps/web/src/app/(main)/newsletters/page.tsx (7)
apps/web/src/store/useShowSidebar.ts (1)
useShowSidebar(10-16)apps/web/src/hooks/useSubscription.ts (1)
useSubscription(11-77)apps/web/src/components/dashboard/Sidebar.tsx (1)
Sidebar(52-166)apps/web/src/components/non-pro-news/News.tsx (1)
News(5-42)apps/web/src/data/newsletters.ts (1)
posts(16-45)apps/web/src/components/ui/IconWrapper.tsx (1)
IconWrapper(10-22)apps/web/src/app/(main)/newsletters/pagetransition.tsx (1)
PageTransition(7-118)
apps/web/src/app/(main)/dashboard/layout.tsx (1)
apps/web/src/components/dashboard/Sidebar.tsx (1)
Sidebar(52-166)
apps/web/src/components/dashboard/Sidebar.tsx (3)
apps/web/src/store/useShowSidebar.ts (1)
useShowSidebar(10-16)apps/web/src/components/ui/IconWrapper.tsx (1)
IconWrapper(10-22)apps/web/src/components/dashboard/ProfilePic.tsx (1)
ProfilePic(11-59)
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-20)apps/web/src/app/(main)/newsletters/pagetransition.tsx (1)
PageTransition(7-118)
🪛 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/page.tsx
[error] 44-44: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 61-61: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
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 (8)
apps/web/package.json (1)
30-31: LGTM! Dependency additions support the newsletter feature.The merge conflict flagged in the previous review has been resolved. The added dependencies are appropriate:
gray-matterandremark/remark-htmlenable markdown parsing for newsletter contentgsappowers page transitions@tailwindcss/typographyprovides prose styling for rendered articlesAlso applies to: 41-42, 50-50
apps/web/src/app/content/newsletters/mastering-react-hooks.md (1)
1-796: LGTM! Comprehensive React Hooks guide with resolved placeholder.The placeholder author attribution flagged in the previous review has been corrected. The content is thorough, well-structured, and includes practical code examples covering essential and advanced hooks, custom hooks, patterns, and best practices.
apps/web/src/components/sidebar/SidebarItem.tsx (1)
18-18: LGTM! Improved hover interactions with group-based coordination.The updated hover styling provides a more cohesive user experience. The
groupclass enables coordinated hover effects across the icon and text, with smoothtransition-colorsfor visual polish.Also applies to: 22-25, 28-28
apps/web/src/data/newsletters.ts (1)
16-45: LGTM! Newsletter metadata structure is well-defined.The posts array provides appropriate metadata for the newsletter feature, with slugs correctly matching the markdown content files.
apps/web/src/app/(main)/dashboard/layout.tsx (1)
21-30: LGTM! Mobile overlay pattern enables different animation behaviors.The dual-Sidebar approach (desktop always-mounted + mobile conditionally-mounted with overlay) enables different animation behaviors for each viewport. The AnimatePresence wrapper properly handles exit animations for the mobile overlay.
apps/web/src/app/content/newsletters/getting-started-with-nextjs.md (1)
1-187: LGTM! Comprehensive Next.js introduction for newsletters.The content is well-structured, covers essential Next.js concepts, and includes practical examples. The minor formatting suggestions above are optional improvements.
apps/web/src/app/(main)/newsletters/[slug]/page.tsx (2)
12-17: LGTM! 404 handling correctly implemented.The previous review's concern about missing slug validation has been addressed. The
notFound()guard prevents errors when accessing non-existent newsletters.
21-85: LGTM! Newsletter article page with proper routing and styling.The implementation correctly fetches newsletter content, handles 404s, and renders articles with appropriate styling and page transitions. The layout is responsive and follows the design system.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
apps/web/src/app/(main)/newsletters/page.tsx (3)
72-83: Consider enhancing the loading state with a skeleton or spinner.The current loading state displays plain text, which works but could provide better user feedback with a skeleton loader or spinner component.
Example enhancement:
<div className="flex-1 flex items-center justify-center"> - <div className="text-white/60">Loading...</div> + <div className="flex flex-col items-center gap-3"> + <div className="w-8 h-8 border-2 border-white/20 border-t-white/60 rounded-full animate-spin" /> + <div className="text-white/60 text-sm">Loading newsletters...</div> + </div> </div>
176-220: Add labels and ARIA attributes for accessibility.The search input and filter controls lack labels, making them inaccessible to screen reader users. Interactive controls need accessible names for WCAG compliance.
Apply these improvements:
<div className="relative flex-1"> + <label htmlFor="newsletter-search" className="sr-only">Search newsletters</label> <input + id="newsletter-search" type="text" placeholder="Search newsletters..." value={searchQuery} onChange={(e) => setSearchQuery(e.target.value)} className="w-full py-[14px] pr-[45px] pl-5 text-base border border-white/20 rounded-xl bg-white/10 backdrop-blur-[10px] text-white placeholder:text-white/60" /> {searchQuery && ( <button + aria-label="Clear search" onClick={() => setSearchQuery("")} className="absolute right-[15px] top-1/2 -translate-y-1/2 bg-white/20 w-7 h-7 rounded-full text-white flex items-center justify-center hover:bg-white/30 transition-colors" > ✕ </button> )} </div> {/* Month Filter */} + <label htmlFor="month-filter" className="sr-only">Filter by month</label> <select + id="month-filter" value={selectedMonth} onChange={(e) => setSelectedMonth(e.target.value)} className="py-[14px] px-5 text-base border border-white/20 rounded-xl bg-white/10 backdrop-blur-[10px] text-white cursor-pointer max-[768px]:w-full" > <option value="" className="bg-[#0f0f0f]">All Months</option> {months.map((month) => ( <option key={month} value={month} className="bg-[#0f0f0f]"> {month} </option> ))} </select> {/* Year Filter */} + <label htmlFor="year-filter" className="sr-only">Filter by year</label> <select + id="year-filter" value={selectedYear} onChange={(e) => setSelectedYear(e.target.value)} className="py-[14px] px-5 text-base border border-white/20 rounded-xl bg-white/10 backdrop-blur-[10px] text-white cursor-pointer max-[768px]:w-full" >Add
aria-labelto sidebar toggle buttons at lines 95-100 and 120-122:<button + aria-label="Open sidebar" onClick={() => setShowSidebar(true)}<IconWrapper onClick={() => setShowSidebar(true)}>For the IconWrapper, you may need to add
aria-labelsupport to the IconWrapper component or replace it with a button element that accepts aria-label.
138-167: Consider simplifying the complex relative positioning.The current implementation uses multiple relative positioning offsets to create the staggered "NEWSLETTER" text layout. While functional, this approach is fragile and difficult to maintain across breakpoints.
Consider using CSS Grid or Flexbox with gaps for a more maintainable approach, or extracting this into a dedicated component with clearer positioning logic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/src/app/(main)/newsletters/page.tsx(1 hunks)apps/web/src/lib/newslettercontent.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/lib/newslettercontent.ts
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/app/(main)/newsletters/page.tsx (7)
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/dashboard/Sidebar.tsx (1)
Sidebar(52-166)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-118)
🔇 Additional comments (1)
apps/web/src/app/(main)/newsletters/page.tsx (1)
15-20: LGTM! Hook placement follows Rules of Hooks.All hooks are correctly placed at the top of the component before any conditional returns, which resolves the previous critical issue.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/web/src/app/content/newsletters/understanding-typescript.md (1)
448-466: Clarify that these redefine built-in utility types.The examples redefine
Readonly<T>and createOptional<T>(equivalent to built-inPartial<T>), which could confuse readers into thinking they need to define these types themselves.Add a note above the code block:
### Mapped Types + +> **Note**: These examples demonstrate how mapped types work by recreating built-in utility types. In practice, use TypeScript's built-in `Readonly<T>` and `Partial<T>` instead of redefining them. ```typescript
🧹 Nitpick comments (1)
apps/web/src/app/(main)/newsletters/pagetransition.tsx (1)
67-106: Excellent improvements to click handler guards!The click handler now properly prevents hijacking of external links, new-tab navigation, and modified clicks, addressing all concerns from the previous review. The guard clauses (lines 77-95) correctly filter out:
- Hash/fragment links
- External protocols (http/https/mailto/tel)
target="_blank"andrel="external"anchors- Modified clicks (cmd/ctrl/shift, non-left button)
Optional refinement: Line 82 could use a more precise regex pattern:
- if (href.startsWith("http") || href.startsWith("mailto") || href.startsWith("tel")) { + if (/^(https?:|mailto:|tel:)/.test(href)) { return }This ensures protocol matching is exact (e.g., won't accidentally match "httpsomething"), though the practical risk is minimal.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
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/understanding-typescript.md(1 hunks)apps/web/src/lib/newslettercontent.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/app/(main)/newsletters/page.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/app/content/newsletters/understanding-typescript.md (1)
apps/web/src/app/(main)/sheet/[moduleId]/page.tsx (1)
ModuleDocPage(59-243)
🪛 markdownlint-cli2 (0.18.1)
apps/web/src/app/content/newsletters/understanding-typescript.md
446-446: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (1)
apps/web/src/lib/newslettercontent.ts (1)
8-45: LGTM! All previous concerns have been addressed.The function correctly implements:
- Path traversal prevention (line 11)
- Async file I/O (line 24)
- Comprehensive error handling (lines 23-27, 40-44)
- Structured return with safe defaults for missing metadata
apps/web/src/app/content/newsletters/understanding-typescript.md
Outdated
Show resolved
Hide resolved
|
hey there @apsinghdev , all code rabbit issues are resolved , please review at your convenience thanks :p |
HarshJS30
force-pushed
the
feat/newsletters
branch
from
83f8936 to
97e699a
Compare
railway-app
bot
temporarily deployed
to
opensox (insightful-communication / prod)
1 participant
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🎬 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:
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
Refactor
Documentation
Chores