Add: added newsletter to opensox.in #176
Conversation
…down exporter function
|
@deveshru2712 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 newsletter feature: markdown content files, markdown-to-HTML processing, list and detail pages with access gating, client-side filtering UI and components, sidebar navigation entry, Tailwind typography updates, and new dependencies for markdown and typography. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ListPage as Newsletter List Page
participant Filters as useNewsletterFilters
participant Item as NewsletterItem
User->>ListPage: GET /dashboard/newsletters
ListPage->>Filters: provide initialData
Filters->>ListPage: filteredAndSorted
ListPage->>User: render list + filters
User->>ListPage: change search/month/sort
ListPage->>Filters: update state
Filters->>ListPage: new filtered results
ListPage->>Item: render items
User->>Item: click -> navigate to /newsletters/{slug}
sequenceDiagram
participant User
participant Layout as Newsletter Layout
participant Auth as useSubscription
participant Detail as Newsletter Detail Page
participant Utils as getNewsletterBySlug
participant Remark as remark
User->>Layout: request /dashboard/newsletters/{slug}
Layout->>Auth: check isLoading / isPaidUser
alt loading
Layout->>User: render NewsletterSkeleton
else not paid
Layout->>User: redirect to /pricing
else paid
Layout->>Detail: render detail
Detail->>Utils: read file & front matter
Utils->>Remark: convert markdown -> HTML
Remark->>Utils: return HTML + metadata
Detail->>Detail: sanitize HTML (normalize widths)
Detail->>User: render article
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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: 5
🧹 Nitpick comments (8)
apps/web/content/newsletters/example.md (1)
1-11: Template frontmatter looks solid; clarify slug usageThe structure and required fields are clear for future authors. Since
getAllNewsLettercurrently skips files whose name starts withexample, this file won’t appear in the listing, and theslugis derived from the filename rather than this frontmatter value. Consider either aligning theslugwith the filename or adding a short comment in docs that the frontmatterslughere is illustrative only.apps/web/tailwind.config.ts (1)
2-3: Typography extension and plugins are well wired for newsletter proseImporting
tailwindcss-animateand@tailwindcss/typography, then extendingtheme.extend.typography.DEFAULT.csswith dark-themed prose styles, is consistent and should give the newsletters a polished reading experience. The overrides for links, code, headings, blockquotes, and tables are coherent with the rest of your color system.If you find body text too bright in practice, you could slightly reduce
colorfrom#e5e7ebto better match long-form reading comfort, but that’s purely a design tweak.Also applies to: 125-210, 213-213
apps/web/content/newsletters/test-2.md (1)
1-68: Content and frontmatter shape are correct; consider markdownlint MD036The frontmatter matches the newsletter loader’s expectations and the content structure reads well. If
markdownlintMD036 is enforced in CI, you may want to convert the emphasized lines like**JUST START WITH WHATEVER YOU HAVE**into actual headings (e.g.,### Just start with whatever you have) or adjust the rule configuration so emphasis‑as‑heading is allowed for this style.apps/web/src/components/newsletter/NewsletterSkeleton.tsx (1)
1-54: Skeleton layout is clean and consistent with the article viewThe skeleton structure (back link, title/meta, body lines, footer) closely mirrors the final newsletter layout and should make loading feel smooth. You’re using only neutral block elements, so there’s no risk of misleading text being read.
If you want to be extra ARIA-friendly, you could wrap the skeleton content in a
<div role="status" aria-live="polite" aria-busy="true">, but this is not strictly required.apps/web/src/app/(main)/dashboard/newsletters/page.tsx (1)
1-8: Server → client handoff is straightforward and fits the content modelUsing a server component page to synchronously read newsletters via
getAllNewsLetter()and then passing them to the clientNewsletterPageasinitialDatais clear and easy to follow.If the number of newsletters grows or file I/O becomes noticeable, consider:
- Switching
getAllNewsLetterto asyncfs.promises, and/or- Opting into static generation (e.g.,
export const dynamic = "force-static"or usingrevalidate) depending on how often newsletter content changes.apps/web/src/app/(main)/dashboard/newsletters/NewsletterPage.tsx (1)
37-87: Overall listing/search/filter UX is well put togetherThe search input, month dropdown, sort dropdown, and “result(s) found” label provide a nice, compact filtering experience. Conditional “Clear all filters” only when filters are active is also a good touch for usability.
If you later notice frequent re-renders with larger datasets, you could debounce
setSearch(e.g., viasetTimeout/useDeferredValue) to keep typing snappy, but it’s not necessary for the current scale.apps/web/src/components/dashboard/Sidebar.tsx (1)
69-71: Newsletter item doesn’t integrate with existing “active route” stylingRight now the Newsletter entry is outside
SIDEBAR_ROUTES, so it never gets thegetSidebarLinkClassNametreatment and won’t appear active when the user is on/dashboard/newsletters(or a slug under it). This is more of a UX inconsistency than a bug.You could align it with the other routes by wrapping the
SidebarItemvariant in aLinkand reusing the same active‑class logic, e.g. derive anewsletterActiveClassfromgetSidebarLinkClassName(pathname, "/dashboard/newsletters")and apply it to aLinkaround the NewsletterSidebarItem(keeping the non‑paid custom block + badge as is).Also applies to: 137-159
apps/web/src/components/newsletter/Dropdown.tsx (1)
15-26: Scope the global click listener to the open state (optional improvement)Current behavior is correct, but the
mousedownlistener is attached for the entire lifetime of the component and fires on every click, even when the dropdown is closed. You can avoid unnecessary work and make the intent clearer by only registering the listener while the dropdown is open, e.g.:- useEffect(() => { - function handleClick(e: MouseEvent) { - if (ref.current && !ref.current.contains(e.target as Node)) { - setOpen(false); - } - } - document.addEventListener("mousedown", handleClick); - return () => document.removeEventListener("mousedown", handleClick); - }, []); + useEffect(() => { + if (!open) return; + + function handleClick(e: MouseEvent) { + if (ref.current && !ref.current.contains(e.target as Node)) { + setOpen(false); + } + } + + document.addEventListener("mousedown", handleClick); + return () => document.removeEventListener("mousedown", handleClick); + }, [open]);You might also consider adding
aria-haspopup="listbox"andaria-expanded={open}to the button for basic accessibility.Also applies to: 30-48
📜 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 (16)
apps/web/content/newsletters/example.md(1 hunks)apps/web/content/newsletters/test-1.md(1 hunks)apps/web/content/newsletters/test-2.md(1 hunks)apps/web/package.json(2 hunks)apps/web/src/app/(main)/dashboard/newsletters/NewsletterPage.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/[slug]/layout.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/page.tsx(1 hunks)apps/web/src/components/dashboard/Sidebar.tsx(4 hunks)apps/web/src/components/newsletter/Dropdown.tsx(1 hunks)apps/web/src/components/newsletter/NewsletterItem.tsx(1 hunks)apps/web/src/components/newsletter/NewsletterSkeleton.tsx(1 hunks)apps/web/src/hooks/useNewsletterFilters.ts(1 hunks)apps/web/src/utils/newsletter/getAllNewsLetter.ts(1 hunks)apps/web/src/utils/newsletter/getNewsletterBySlug.ts(1 hunks)apps/web/tailwind.config.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
apps/web/src/app/(main)/dashboard/newsletters/NewsletterPage.tsx (3)
apps/web/src/hooks/useNewsletterFilters.ts (1)
useNewsletterFilters(13-92)apps/web/src/components/newsletter/Dropdown.tsx (1)
CustomDropdown(6-77)apps/web/src/components/newsletter/NewsletterItem.tsx (1)
NewsletterItem(13-85)
apps/web/src/hooks/useNewsletterFilters.ts (1)
apps/web/src/components/newsletter/NewsletterItem.tsx (1)
NewsletterItem(13-85)
apps/web/src/app/(main)/dashboard/newsletters/page.tsx (2)
apps/web/src/utils/newsletter/getAllNewsLetter.ts (1)
getAllNewsLetter(17-50)apps/web/src/app/(main)/dashboard/newsletters/NewsletterPage.tsx (1)
NewsletterPage(18-124)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx (1)
apps/web/src/utils/newsletter/getNewsletterBySlug.ts (2)
getAllNewsletterSlugs(10-15)getNewsletterBySlug(17-32)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/layout.tsx (2)
apps/web/src/hooks/useSubscription.ts (1)
useSubscription(11-77)apps/web/src/components/newsletter/NewsletterSkeleton.tsx (1)
NewsletterSkeleton(1-54)
apps/web/src/components/dashboard/Sidebar.tsx (2)
apps/web/src/components/sheet/OpensoxProBadge.tsx (1)
OpensoxProBadge(8-28)apps/web/src/components/sidebar/SidebarItem.tsx (1)
SidebarItem(13-32)
🪛 ast-grep (0.39.9)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx
[warning] 91-91: 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)/dashboard/newsletters/[slug]/page.tsx
[error] 92-92: 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/content/newsletters/test-2.md
[style] ~30-~30: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...till publish it and let people use it. When your resume looks like a blank slate, b...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
apps/web/content/newsletters/test-1.md
[style] ~28-~28: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ing. No college degree. No connections. No safety net. Just documentation, tutoria...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~61-~61: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...mission. You don't need a fancy degree. You need a computer, internet, and the refu...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.18.1)
apps/web/content/newsletters/test-2.md
58-58: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
60-60: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
62-62: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (5)
apps/web/content/newsletters/test-1.md (1)
1-63: Frontmatter and structure align with the newsletter loaderThis newsletter’s frontmatter matches the expected fields (id, title, date, summary, keywords, readTime, slug) and should be correctly picked up by
getAllNewsLetter. The narrative style (repeated “No …” / “You …” sentence starts) is intentional and works rhetorically; LanguageTool’s suggestions there can be safely ignored unless you want stricter formal tone.apps/web/src/app/(main)/dashboard/newsletters/NewsletterPage.tsx (1)
95-121: UseuseRouter().push()for client-side navigation instead ofredirectIn Next.js 15,
redirectfrom "next/navigation" is intended for Server Components, Server Actions, and route handlers, not for event handlers in client components. For client-side onClick handlers, use the client-onlyuseRouter()withrouter.push()orrouter.replace().Update
NewsletterItemto use client-side routing:+"use client"; + +import { useRouter } from "next/navigation"; + export default function NewsletterItem({ time, title, summary, keywords, readTime, slug, }: NewsletterItemProps) { + const router = useRouter(); + return ( <div className=" w-full cursor-pointer group border border-white/10 rounded-lg p-4 hover:border-purple-400/40 transition-all duration-200 " - onClick={() => redirect(`/dashboard/newsletters/${slug}`)} + onClick={() => router.push(`/dashboard/newsletters/${slug}`)} > {/* ... */} </div> ); }Alternatively, wrap the content in a
<Link href={/dashboard/newsletters/${slug}}>for simpler markup.Likely an incorrect or invalid review comment.
apps/web/package.json (1)
30-42: Compatibility confirmed; versions are safe to use with sensible sanitization practicesGray-matter@4, remark@15, remark-gfm@4, and remark-html@16 are generally compatible with Next.js 15 (React 18/19), provided your Node version meets Next.js 15 requirements. remark-html is safe by default (it uses a safe schema).
For untrusted content, follow standard practice: sanitize HTML server-side during build/SSR and insert client-side via dangerouslySetInnerHTML. rehype-sanitize or DOMPurify (which you already depend on) are common choices. No action needed if you're already following this pattern.
apps/web/src/app/(main)/dashboard/newsletters/[slug]/layout.tsx (1)
8-30: Access gating and loading flow look correct for pro‑only detail viewThe layout cleanly gates newsletter detail pages behind
isPaidUserand shows the skeleton while subscription status is resolving; redirecting non‑paid users to/pricingonce loading completes matches the intended behavior and avoids flashing protected content.apps/web/src/hooks/useNewsletterFilters.ts (1)
44-69: Filtering pipeline looks solid.Nicely composed pipeline—search, month filter, and sort are all handled cleanly and stay memoized for cheap updates.
| export function getAllNewsletterSlugs(): string[] { | ||
| return fs | ||
| .readdirSync(newsletterDir) | ||
| .filter((f) => f.endsWith(".md")) | ||
| .map((f) => f.replace(/\.md$/, "")); | ||
| } |
There was a problem hiding this comment.
Include .mdx files when building slug list.
Right now the slug generator ignores every .mdx file, so those newsletters never get static params or routes even though the rest of the stack treats MDX as supported content. Update the filter/replace so both .md and .mdx files participate.
Apply this diff:
- .readdirSync(newsletterDir)
- .filter((f) => f.endsWith(".md"))
- .map((f) => f.replace(/\.md$/, ""));
+ .readdirSync(newsletterDir)
+ .filter((f) => f.endsWith(".md") || f.endsWith(".mdx"))
+ .map((f) => f.replace(/\.mdx?$/, ""));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function getAllNewsletterSlugs(): string[] { | |
| return fs | |
| .readdirSync(newsletterDir) | |
| .filter((f) => f.endsWith(".md")) | |
| .map((f) => f.replace(/\.md$/, "")); | |
| } | |
| export function getAllNewsletterSlugs(): string[] { | |
| return fs | |
| .readdirSync(newsletterDir) | |
| .filter((f) => f.endsWith(".md") || f.endsWith(".mdx")) | |
| .map((f) => f.replace(/\.mdx?$/, "")); | |
| } |
🤖 Prompt for AI Agents
In apps/web/src/utils/newsletter/getNewsletterBySlug.ts around lines 10 to 15,
the slug generator only includes files ending with ".md" so ".mdx" newsletters
are ignored; update the file filter to accept both ".md" and ".mdx" (e.g., test
the extension with a regex like /\.(md|mdx)$/) and update the mapping that
strips the extension to remove either ".md" or ".mdx" (e.g., replace using
/\.mdx?$/) so both file types produce slugs.
| const fullPath = path.join(newsletterDir, `${slug}.md`); | ||
| const file = fs.readFileSync(fullPath, "utf8"); | ||
|
|
||
| const { data, content } = matter(file); | ||
|
|
||
| const processed = await remark().use(gfm).use(html).process(content); | ||
|
|
||
| return { | ||
| metadata: data, | ||
| html: processed.toString(), |
There was a problem hiding this comment.
Resolve .mdx file loads in getNewsletterBySlug.
We only try ${slug}.md, so any .mdx newsletter will throw ENOENT even though we advertise MDX support. Probe both extensions (or detect the actual file) before reading.
Apply this diff:
- const fullPath = path.join(newsletterDir, `${slug}.md`);
- const file = fs.readFileSync(fullPath, "utf8");
+ const candidates = [
+ path.join(newsletterDir, `${slug}.md`),
+ path.join(newsletterDir, `${slug}.mdx`),
+ ];
+ const fullPath = candidates.find((candidate) => fs.existsSync(candidate));
+ if (!fullPath) {
+ throw new Error(`Newsletter "${slug}" not found as .md or .mdx`);
+ }
+ const file = fs.readFileSync(fullPath, "utf8");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const fullPath = path.join(newsletterDir, `${slug}.md`); | |
| const file = fs.readFileSync(fullPath, "utf8"); | |
| const { data, content } = matter(file); | |
| const processed = await remark().use(gfm).use(html).process(content); | |
| return { | |
| metadata: data, | |
| html: processed.toString(), | |
| const candidates = [ | |
| path.join(newsletterDir, `${slug}.md`), | |
| path.join(newsletterDir, `${slug}.mdx`), | |
| ]; | |
| const fullPath = candidates.find((candidate) => fs.existsSync(candidate)); | |
| if (!fullPath) { | |
| throw new Error(`Newsletter "${slug}" not found as .md or .mdx`); | |
| } | |
| const file = fs.readFileSync(fullPath, "utf8"); | |
| const { data, content } = matter(file); | |
| const processed = await remark().use(gfm).use(html).process(content); | |
| return { | |
| metadata: data, | |
| html: processed.toString(), |
🤖 Prompt for AI Agents
In apps/web/src/utils/newsletter/getNewsletterBySlug.ts around lines 21 to 30,
the function only tries `${slug}.md` and will throw ENOENT for `.mdx` files;
update it to check for both `.md` and `.mdx` (e.g., test existence with
fs.existsSync or fs.promises.stat for `${slug}.md` then `${slug}.mdx`) and pick
the first existing path before reading; if neither exists, throw a clear error;
then read and parse the chosen file path as before so MDX newsletters are
supported.
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (8)
apps/web/tailwind.config.ts (1)
126-211: Consider using theme color references for maintainability.The typography configuration uses several hardcoded color values that duplicate colors already defined in your theme:
#9455f4(line 132) matchesox-purple#7A45C3(line 134) matchesox-purple-2#252525(line 188, 207) matchesborder-primaryTo improve maintainability and ensure color consistency, consider referencing theme colors or CSS variables instead of hardcoding values. Example approach:
typography: { DEFAULT: { css: { maxWidth: "none", color: "#e5e7eb", a: { - color: "#9455f4", + color: "var(--ox-purple)", "&:hover": { - color: "#7A45C3", + color: "var(--ox-purple-2)", }, textDecoration: "none", }, // ... rest of config hr: { - borderColor: "#252525", + borderColor: "var(--border-primary)", },Note: You'll need to ensure these CSS variables are defined in your global styles, or use Tailwind's theme color references if the typography plugin supports it.
apps/web/content/newsletters/test-1.md (1)
18-18: Consider hosting images locally instead of hotlinking.The newsletter uses an external image URL from Brave search results (
imgs.search.brave.com). Hotlinking external images has several drawbacks:
- Image availability depends on external service
- No control over image content changes
- Potential performance and privacy implications
- The URL may break or change over time
Consider:
- Download and host the image in your public assets directory
- Use a proper CDN or image hosting service
- Add the image to a
public/newsletters/directory and reference it with a relative pathExample:
- +apps/web/src/app/(main)/dashboard/newsletters/[slug]/layout.tsx (2)
16-20: Potential flash of content before redirect.The redirect logic has a timing issue: the component renders
null(line 27) while theuseEffectredirect is still pending. During this brief moment, users might see a blank screen before the redirect completes. Additionally, usingrouter.replacemight cause browser history issues if users try to navigate back.Consider showing a more informative message or using
router.pushfor better UX:useEffect(() => { if (!isLoading && !isPaidUser) { - router.replace("/pricing"); + router.push("/pricing"); } }, [isLoading, isPaidUser, router]); if (isLoading) { return <NewsletterSkeleton />; } if (!isPaidUser) { - return null; + return ( + <div className="flex items-center justify-center min-h-screen"> + <p className="text-gray-400">Redirecting to pricing...</p> + </div> + ); }Alternatively, consider using Next.js middleware for authentication checks to avoid client-side redirects altogether.
8-31: Consider server-side authentication check.This layout performs access control entirely on the client side, which means:
- The newsletter content might be fetched before the redirect occurs
- Users can potentially access data by disabling JavaScript or intercepting requests
- Additional network round trip for subscription check
For sensitive content like paid newsletters, consider:
- Adding server-side authentication in the page component or using Next.js middleware
- Checking subscription status before serving the page content
- Using server-side redirects for better security and UX
This would prevent unauthorized access attempts and improve loading performance by avoiding unnecessary data fetching.
apps/web/content/newsletters/test-2.md (1)
27-30: Minor: Consider varying sentence structure for better flow.Three consecutive sentences starting with "When" may impact readability. Consider rephrasing one for variety.
Example:
-When your camera sucks, but still you hit that red button, record a scrappy and embarrassing video, and click on "post" and actually post it on YouTube. +Your camera sucks, but you still hit that red button, record a scrappy and embarrassing video, and click on "post" and actually post it on YouTube.apps/web/src/utils/newsletter/getAllNewsLetter.ts (1)
17-50: Consider adding error handling for file parsing.The function uses synchronous file operations without error handling. Malformed markdown or frontmatter could cause build failures.
Add try-catch around file operations:
export function getAllNewsLetter(): Newsletter[] { const files = fs.readdirSync(newsletterDir); return files .filter((file) => { // Skip files starting with "example" if (file.startsWith("example")) return false; // Only include .md and .mdx files return file.endsWith(".md") || file.endsWith(".mdx"); }) .map((filename) => { const filePath = path.join(newsletterDir, filename); - const raw = fs.readFileSync(filePath, "utf8"); - const { data } = matter(raw); + try { + const raw = fs.readFileSync(filePath, "utf8"); + const { data } = matter(raw); - const slug = filename.replace(/\.mdx?$/, ""); + const slug = filename.replace(/\.mdx?$/, ""); - const keywords = Array.isArray(data.keywords) - ? data.keywords - : typeof data.keywords === "string" - ? data.keywords.split(",").map((k: string) => k.trim()) - : []; + const keywords = Array.isArray(data.keywords) + ? data.keywords + : typeof data.keywords === "string" + ? data.keywords.split(",").map((k: string) => k.trim()) + : []; - return { - id: data.id ?? slug, - title: data.title ?? "Untitled Newsletter", - summary: data.summary ?? "", - keywords, - time: data.time ?? data.date ?? new Date().toISOString(), - readTime: data.readTime ?? "1 min read", - slug, - } satisfies Newsletter; + return { + id: data.id ?? slug, + title: data.title ?? "Untitled Newsletter", + summary: data.summary ?? "", + keywords, + time: data.time ?? data.date ?? new Date().toISOString(), + readTime: data.readTime ?? "1 min read", + slug, + } satisfies Newsletter; + } catch (error) { + console.error(`Failed to parse newsletter: ${filename}`, error); + throw new Error(`Invalid newsletter file: ${filename}`); + } }); }apps/web/src/components/newsletter/Dropdown.tsx (1)
55-55: Consider lowering z-index value.
z-[9999]is an extremely high z-index that might cause stacking conflicts with modals or other overlays.Consider using a more reasonable value like
z-50unless there's a specific need for such a high value.className=" absolute mt-2 w-full bg-neutral-900 text-white border border-white/10 rounded-md shadow-lg shadow-black/40 - z-[9999] + z-50 "apps/web/src/app/(main)/dashboard/newsletters/NewsletterPage.tsx (1)
8-16: Duplicate type definition: reuseNewsletterfrom utility.The
NewsletterDatainterface duplicates theNewslettertype fromgetAllNewsLetter.ts, creating maintenance overhead.+import { Newsletter } from "@/utils/newsletter/getAllNewsLetter"; import { Search } from "lucide-react"; import NewsletterItem from "@/components/newsletter/NewsletterItem"; import CustomDropdown from "@/components/newsletter/Dropdown"; import { useNewsletterFilters } from "@/hooks/useNewsletterFilters"; -interface NewsletterData { - id: string; - title: string; - summary: string; - keywords: string[]; - time: string; - readTime: string; - slug: string; -} - export default function NewsletterPage({ initialData, }: { - initialData: NewsletterData[]; + initialData: Newsletter[]; }) {
📜 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 (16)
apps/web/content/newsletters/example.md(1 hunks)apps/web/content/newsletters/test-1.md(1 hunks)apps/web/content/newsletters/test-2.md(1 hunks)apps/web/package.json(2 hunks)apps/web/src/app/(main)/dashboard/newsletters/NewsletterPage.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/[slug]/layout.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/page.tsx(1 hunks)apps/web/src/components/dashboard/Sidebar.tsx(5 hunks)apps/web/src/components/newsletter/Dropdown.tsx(1 hunks)apps/web/src/components/newsletter/NewsletterItem.tsx(1 hunks)apps/web/src/components/newsletter/NewsletterSkeleton.tsx(1 hunks)apps/web/src/hooks/useNewsletterFilters.ts(1 hunks)apps/web/src/utils/newsletter/getAllNewsLetter.ts(1 hunks)apps/web/src/utils/newsletter/getNewsletterBySlug.ts(1 hunks)apps/web/tailwind.config.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
apps/web/src/hooks/useNewsletterFilters.ts (1)
apps/web/src/components/newsletter/NewsletterItem.tsx (1)
NewsletterItem(13-85)
apps/web/src/app/(main)/dashboard/newsletters/NewsletterPage.tsx (3)
apps/web/src/hooks/useNewsletterFilters.ts (1)
useNewsletterFilters(13-92)apps/web/src/components/newsletter/Dropdown.tsx (1)
CustomDropdown(6-77)apps/web/src/components/newsletter/NewsletterItem.tsx (1)
NewsletterItem(13-85)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/layout.tsx (2)
apps/web/src/hooks/useSubscription.ts (1)
useSubscription(11-77)apps/web/src/components/newsletter/NewsletterSkeleton.tsx (1)
NewsletterSkeleton(1-54)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx (1)
apps/web/src/utils/newsletter/getNewsletterBySlug.ts (2)
getAllNewsletterSlugs(10-15)getNewsletterBySlug(17-32)
apps/web/src/app/(main)/dashboard/newsletters/page.tsx (2)
apps/web/src/utils/newsletter/getAllNewsLetter.ts (1)
getAllNewsLetter(17-50)apps/web/src/app/(main)/dashboard/newsletters/NewsletterPage.tsx (1)
NewsletterPage(18-124)
apps/web/src/components/dashboard/Sidebar.tsx (2)
apps/web/src/components/sheet/OpensoxProBadge.tsx (1)
OpensoxProBadge(8-28)apps/web/src/components/sidebar/SidebarItem.tsx (1)
SidebarItem(13-36)
🪛 ast-grep (0.39.9)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx
[warning] 91-91: 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)/dashboard/newsletters/[slug]/page.tsx
[error] 92-92: 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/content/newsletters/test-1.md
[style] ~28-~28: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ing. No college degree. No connections. No safety net. Just documentation, tutoria...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~61-~61: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...mission. You don't need a fancy degree. You need a computer, internet, and the refu...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
apps/web/content/newsletters/test-2.md
[style] ~30-~30: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...till publish it and let people use it. When your resume looks like a blank slate, b...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.18.1)
apps/web/content/newsletters/test-2.md
58-58: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
60-60: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
62-62: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (10)
apps/web/tailwind.config.ts (2)
2-3: LGTM: Modern ES module imports.The conversion from
require()to ES module imports is the correct approach for TypeScript configuration files.
214-214: LGTM: Plugin configuration updated correctly.The plugins array correctly uses the imported plugin references, consistent with the ES module conversion.
apps/web/src/hooks/useNewsletterFilters.ts (1)
44-69: LGTM: Filter and sort logic is well-structured.The filtering and sorting implementation is clean and efficient:
- Uses
useMemoappropriately to avoid unnecessary recalculations- Case-insensitive search across relevant fields
- Clear sort logic with proper date comparison
apps/web/src/components/newsletter/NewsletterSkeleton.tsx (1)
1-54: LGTM: Clean skeleton implementation.The skeleton component is well-structured with:
- Appropriate use of
animate-pulsefor loading feedback- Responsive layout matching the actual content structure
- Consistent styling with the design system
apps/web/content/newsletters/test-1.md (1)
1-16: LGTM: Front matter structure is correct.The front matter follows the expected schema with all required fields properly defined.
apps/web/content/newsletters/example.md (1)
1-11: LGTM: Example file serves its purpose.This file serves as a template/documentation for the newsletter front matter structure. According to the
getAllNewsLetterutility, files starting with "example" are intentionally filtered out from the newsletter list, so this won't appear in production.The front matter structure correctly demonstrates all required fields for new newsletters.
apps/web/package.json (1)
30-30: All package versions are secure with no active vulnerabilities.The security scan confirms that the specified versions are safe:
- gray-matter ^4.0.3: No vulnerabilities
- remark ^18.0.0: No vulnerabilities
- remark-gfm ^4.0.1: No vulnerabilities
- remark-html ^16.0.1: No vulnerabilities (version ^16.0.1 is outside known vulnerable ranges for older versions ≤14.0.0 and <13.0.2)
- @tailwindcss/typography ^0.5.15: No vulnerabilities
apps/web/content/newsletters/test-2.md (1)
58-64: Bold emphasis for impact is appropriate here.While markdown linters flag these lines as "emphasis used instead of heading," the bold styling is intentionally used for emphasis and impact rather than document structure. This is a valid stylistic choice for motivational content.
apps/web/src/components/dashboard/Sidebar.tsx (1)
136-158: Verify hover color consistency.The Newsletter item uses
hover:bg-[#121214](line 138) while other items usehover:bg-[#292929](line 162). Ensure this difference is intentional.If consistency is desired, apply this diff:
{!isCollapsed && !isPaidUser ? ( <div - className="w-full h-[44px] flex items-center rounded-md cursor-pointer transition-colors px-2 gap-3 pl-3 hover:bg-[#121214]" + className="w-full h-[44px] flex items-center rounded-md cursor-pointer transition-colors px-2 gap-3 pl-3 hover:bg-[#292929] group" onClick={handleNewsLetter} > <span className="shrink-0 text-[#eaeaea]"> <Mailbox className="size-5" /> </span> <div className="flex items-center gap-1"> <h1 className="text-xs font-medium text-[#c8c8c8] group-hover:text-ox-purple"> Newsletter </h1>apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx (1)
87-93: Review comment is incorrect - no XSS vulnerability exists with current implementation.The review references a non-existent function ("removeWidthStyles") when the actual function is "sanitizeHtml". More importantly, the actual XSS protection comes from the markdown processing pipeline, not from the width-removal function. Newsletters are processed through
remark-html, which escapes HTML by default, ensuring that any HTML in the markdown content is safe. Since newsletters are sourced from markdown files only (not user input), and remark-html escapes the output, there is no XSS vulnerability. ThesanitizeHtmlfunction only removes width attributes via regex, which is orthogonal to XSS protection but works correctly for its stated purpose.The recommendation to add DOMPurify is unnecessary given the safe markdown-to-HTML pipeline already in place.
Likely an incorrect or invalid review comment.
| export default function Page() { | ||
| const newsletters = getAllNewsLetter(); | ||
|
|
||
| return <NewsletterPage initialData={newsletters} />; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add error handling for newsletter loading failures.
The getAllNewsLetter() call has no error handling. If the function throws (e.g., due to file system errors, malformed content, or missing directory), the entire page will crash.
Consider adding error boundaries or try-catch:
export default function Page() {
- const newsletters = getAllNewsLetter();
+ try {
+ const newsletters = getAllNewsLetter();
+ return <NewsletterPage initialData={newsletters} />;
+ } catch (error) {
+ console.error("Failed to load newsletters:", error);
+ return (
+ <div className="w-full py-6 px-4 sm:px-8">
+ <h1 className="text-3xl font-bold">Newsletter</h1>
+ <p className="mt-4 text-red-400">
+ Failed to load newsletters. Please try again later.
+ </p>
+ </div>
+ );
+ }
-
- return <NewsletterPage initialData={newsletters} />;
}🤖 Prompt for AI Agents
In apps/web/src/app/(main)/dashboard/newsletters/page.tsx around lines 4 to 8,
the call to getAllNewsLetter() is unprotected and will crash the page if it
throws; wrap the call in a try-catch, log the error (with contextual message),
and return a safe fallback render—e.g., render NewsletterPage with an empty
initialData array and/or an error prop informing the UI—or rethrow a controlled
error to trigger a Next.js error boundary (or call notFound/redirect) depending
on desired UX; ensure the catch path does not expose sensitive details and
provides enough info for debugging.
| const availableMonths = useMemo(() => { | ||
| const months = new Set<string>(); | ||
| initialData.forEach((item) => { | ||
| const date = new Date(item.time); | ||
| const monthYear = date.toLocaleDateString("en-US", { | ||
| year: "numeric", | ||
| month: "long", | ||
| }); | ||
| months.add(monthYear); | ||
| }); | ||
| return Array.from(months).sort( | ||
| (a, b) => new Date(b).getTime() - new Date(a).getTime() | ||
| ); | ||
| }, [initialData]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add error handling for invalid date values.
The date parsing logic assumes item.time is always a valid date string. If an invalid value is encountered, new Date(item.time) returns an Invalid Date object, which could cause unexpected behavior.
Consider adding validation:
const availableMonths = useMemo(() => {
const months = new Set<string>();
initialData.forEach((item) => {
const date = new Date(item.time);
+ // Skip invalid dates
+ if (isNaN(date.getTime())) {
+ console.warn(`Invalid date in newsletter: ${item.id || item.slug}`);
+ return;
+ }
const monthYear = date.toLocaleDateString("en-US", {
year: "numeric",
month: "long",
});
months.add(monthYear);
});Also apply similar validation in the filteredByMonth filter (lines 53-61).
🤖 Prompt for AI Agents
In apps/web/src/hooks/useNewsletterFilters.ts around lines 18 to 31, the
month-extraction logic assumes item.time is always valid and uses new
Date(item.time) without checking for Invalid Date; update the code to validate
the parsed date (e.g., create the Date, check !isNaN(date.getTime()) or
date.toString() !== "Invalid Date") before computing monthYear and only add
valid monthYear values to the Set, and ensure you handle or skip invalid
entries; also apply the same validation pattern in the filteredByMonth logic
around lines 53 to 61 so the filter skips items with invalid dates instead of
producing incorrect matches or runtime errors.
| export function getAllNewsletterSlugs(): string[] { | ||
| return fs | ||
| .readdirSync(newsletterDir) | ||
| .filter((f) => f.endsWith(".md")) | ||
| .map((f) => f.replace(/\.md$/, "")); | ||
| } |
There was a problem hiding this comment.
Inconsistency: Handle both .md and .mdx files.
This function only processes .md files, but getAllNewsLetter.ts supports both .md and .mdx. This inconsistency could cause issues if .mdx files are added.
export function getAllNewsletterSlugs(): string[] {
return fs
.readdirSync(newsletterDir)
- .filter((f) => f.endsWith(".md"))
- .map((f) => f.replace(/\.md$/, ""));
+ .filter((f) => f.endsWith(".md") || f.endsWith(".mdx"))
+ .map((f) => f.replace(/\.mdx?$/, ""));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function getAllNewsletterSlugs(): string[] { | |
| return fs | |
| .readdirSync(newsletterDir) | |
| .filter((f) => f.endsWith(".md")) | |
| .map((f) => f.replace(/\.md$/, "")); | |
| } | |
| export function getAllNewsletterSlugs(): string[] { | |
| return fs | |
| .readdirSync(newsletterDir) | |
| .filter((f) => f.endsWith(".md") || f.endsWith(".mdx")) | |
| .map((f) => f.replace(/\.mdx?$/, "")); | |
| } |
🤖 Prompt for AI Agents
In apps/web/src/utils/newsletter/getNewsletterBySlug.ts around lines 10 to 15,
the function only filters for ".md" files while other code accepts both ".md"
and ".mdx"; update the file listing logic to include both extensions (e.g.,
filter for filenames that end with ".md" or ".mdx"), strip either extension when
mapping to slugs (remove the matching ".md" or ".mdx" suffix), and optionally
deduplicate the resulting slug array in case both versions exist for the same
slug.
| export async function getNewsletterBySlug(slug: string): Promise<{ | ||
| metadata: Record<string, any>; | ||
| html: string; | ||
| }> { | ||
| const fullPath = path.join(newsletterDir, `${slug}.md`); | ||
| const file = fs.readFileSync(fullPath, "utf8"); | ||
|
|
||
| const { data, content } = matter(file); | ||
|
|
||
| const processed = await remark().use(gfm).use(html).process(content); | ||
|
|
||
| return { | ||
| metadata: data, | ||
| html: processed.toString(), | ||
| }; | ||
| } |
There was a problem hiding this comment.
Add error handling and support .mdx files.
The function lacks error handling and only supports .md files, creating inconsistency with getAllNewsLetter.ts.
export async function getNewsletterBySlug(slug: string): Promise<{
metadata: Record<string, any>;
html: string;
}> {
- const fullPath = path.join(newsletterDir, `${slug}.md`);
- const file = fs.readFileSync(fullPath, "utf8");
+ // Try .md first, then .mdx
+ let fullPath = path.join(newsletterDir, `${slug}.md`);
+ if (!fs.existsSync(fullPath)) {
+ fullPath = path.join(newsletterDir, `${slug}.mdx`);
+ }
+
+ if (!fs.existsSync(fullPath)) {
+ throw new Error(`Newsletter not found: ${slug}`);
+ }
+
+ const file = fs.readFileSync(fullPath, "utf8");
const { data, content } = matter(file);
const processed = await remark().use(gfm).use(html).process(content);
return {
metadata: data,
html: processed.toString(),
};
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function getNewsletterBySlug(slug: string): Promise<{ | |
| metadata: Record<string, any>; | |
| html: string; | |
| }> { | |
| const fullPath = path.join(newsletterDir, `${slug}.md`); | |
| const file = fs.readFileSync(fullPath, "utf8"); | |
| const { data, content } = matter(file); | |
| const processed = await remark().use(gfm).use(html).process(content); | |
| return { | |
| metadata: data, | |
| html: processed.toString(), | |
| }; | |
| } | |
| export async function getNewsletterBySlug(slug: string): Promise<{ | |
| metadata: Record<string, any>; | |
| html: string; | |
| }> { | |
| // Try .md first, then .mdx | |
| let fullPath = path.join(newsletterDir, `${slug}.md`); | |
| if (!fs.existsSync(fullPath)) { | |
| fullPath = path.join(newsletterDir, `${slug}.mdx`); | |
| } | |
| if (!fs.existsSync(fullPath)) { | |
| throw new Error(`Newsletter not found: ${slug}`); | |
| } | |
| const file = fs.readFileSync(fullPath, "utf8"); | |
| const { data, content } = matter(file); | |
| const processed = await remark().use(gfm).use(html).process(content); | |
| return { | |
| metadata: data, | |
| html: processed.toString(), | |
| }; | |
| } |
🤖 Prompt for AI Agents
In apps/web/src/utils/newsletter/getNewsletterBySlug.ts around lines 17 to 32,
the function currently reads only `.md` files and has no error handling; update
it to attempt reading `${slug}.md` and if not found fall back to `${slug}.mdx`,
wrap the file read / parsing / remark processing in a try/catch, and in the
catch log or rethrow a clear error (or return null) so callers can handle
failures; ensure the processor supports MDX if needed (or detect .mdx and use
the proper parser), and return the same shape { metadata, html } on success.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx (2)
14-20: Good rename from past review; consider adding documentation.The rename from
sanitizeHtmltonormalizeContentWidthsaddresses the previous review feedback well—the name now accurately reflects that this function handles layout normalization, not security.To help future maintainers, consider adding a brief JSDoc comment documenting:
- Its purpose (removes width-related attributes for responsive layout)
- What it doesn't do (XSS protection)
- The trust assumption (content sourced from local markdown files in the repo)
Example:
+/** + * Removes width-related inline styles and attributes for responsive layout. + * Note: This is NOT security sanitization. Content is assumed trusted + * (sourced from local markdown files maintained by the team). + */ function normalizeContentWidths(html: string): string {
86-93: Consider adding HTML sanitization for defense-in-depth.While the current content source is trusted (local markdown files), adding proper HTML sanitization would provide defense-in-depth protection and make the codebase more resilient to future changes.
Since DOMPurify is already available in the project dependencies, you could integrate it in the content processing pipeline to sanitize the HTML before rendering. This would protect against XSS if the content source ever changes (e.g., future CMS integration, external API).
Example integration in
getNewsletterBySlug.tsor before rendering:import DOMPurify from 'isomorphic-dompurify'; // In component or utility: const sanitizedHtml = DOMPurify.sanitize(normalizedHtml, { ALLOWED_TAGS: ['p', 'br', 'strong', 'em', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'ul', 'ol', 'li', 'a', 'img', 'pre', 'code', 'blockquote', 'table', 'thead', 'tbody', 'tr', 'th', 'td'], ALLOWED_ATTR: ['href', 'src', 'alt', 'title', 'class'] });apps/web/src/components/newsletter/Dropdown.tsx (1)
70-75: Optionally focus the selected option instead of always the first.Right now opening the dropdown always focuses the first option; UX is often better if focus lands on the currently selected value, especially for keyboard users.
Example refactor:
- useEffect(() => { - if (open) { - optionsRef.current[0]?.focus(); - } - }, [open]); + useEffect(() => { + if (!open) return; + + const selectedIndex = options.findIndex((opt) => opt.value === value); + const indexToFocus = selectedIndex >= 0 ? selectedIndex : 0; + optionsRef.current[indexToFocus]?.focus(); + }, [open, options, value]);This keeps behavior unchanged when no matching
valueis found, but improves navigation when reopening an already‑selected dropdown.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web/src/app/(main)/dashboard/newsletters/NewsletterPage.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx(1 hunks)apps/web/src/components/newsletter/Dropdown.tsx(1 hunks)apps/web/src/components/newsletter/NewsletterItem.tsx(1 hunks)apps/web/src/hooks/useNewsletterFilters.ts(1 hunks)apps/web/src/utils/newsletter/getAllNewsLetter.ts(1 hunks)apps/web/src/utils/newsletter/getNewsletterBySlug.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/web/src/utils/newsletter/getNewsletterBySlug.ts
- apps/web/src/utils/newsletter/getAllNewsLetter.ts
- apps/web/src/components/newsletter/NewsletterItem.tsx
- apps/web/src/hooks/useNewsletterFilters.ts
- apps/web/src/app/(main)/dashboard/newsletters/NewsletterPage.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx (1)
apps/web/src/utils/newsletter/getNewsletterBySlug.ts (2)
getAllNewsletterSlugs(10-15)getNewsletterBySlug(17-32)
🪛 ast-grep (0.39.9)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx
[warning] 91-91: 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/components/newsletter/Dropdown.tsx
[error] 42-42: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 47-48: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx
[error] 92-92: 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/src/app/(main)/dashboard/newsletters/[slug]/page.tsx (4)
1-8: LGTM!Imports are clean and appropriate for the newsletter detail page functionality.
10-12: LGTM!Static params generation follows Next.js conventions correctly and will enable pre-rendering of all newsletter detail pages.
22-34: LGTM!Component setup and error handling follow Next.js server component patterns correctly. The try-catch with
notFound()ensures missing or invalid newsletters result in proper 404 responses.
38-110: LGTM!The UI implementation is well-structured with:
- Semantic HTML elements (
article,header,section,footer,time)- Accessible date formatting using
<time dateTime={...}>- Responsive design with appropriate breakpoints
- Proper conditional rendering for optional metadata
- Clean navigation with hover effects
The layout and user experience are solid.
apps/web/src/components/newsletter/Dropdown.tsx (1)
15-28: Overall dropdown behavior and a11y handling look solid.Aside from the labelling and switch‑case nits above, the component is well put together: controlled
value, outside‑click close, document‑level keyboard handling with Escape/Home/End, and use ofrole="listbox"/role="option"plusaria-selectedand focus management all align nicely.Also applies to: 79-106, 108-141
| aria-haspopup="listbox" | ||
| aria-expanded={open} | ||
| aria-labelledby="dropdown-label" |
There was a problem hiding this comment.
Avoid hard‑coded aria-labelledby id; make the label configurable or remove it.
Using a fixed aria-labelledby="dropdown-label" in a reusable component is risky: multiple CustomDropdown instances will either share the same ID (invalid HTML) or reference a label that doesn’t exist, which hurts screen reader behavior.
One approach is to accept an optional labelId prop and wire it through:
-export default function CustomDropdown({
- value,
- onChange,
- options,
-}: {
- value: string;
- onChange: (value: string) => void;
- options: { label: string; value: string }[];
-}) {
+export default function CustomDropdown({
+ value,
+ onChange,
+ options,
+ labelId,
+}: {
+ value: string;
+ onChange: (value: string) => void;
+ options: { label: string; value: string }[];
+ labelId?: string;
+}) {
@@
- aria-haspopup="listbox"
- aria-expanded={open}
- aria-labelledby="dropdown-label"
+ aria-haspopup="listbox"
+ aria-expanded={open}
+ aria-labelledby={labelId}
@@
- <div
- role="listbox"
- aria-labelledby="dropdown-label"
+ <div
+ role="listbox"
+ aria-labelledby={labelId}Call sites can then pass distinct IDs (or omit labelId if you prefer to rely on aria-label or the visible label inside the button).
Also applies to: 110-111
🤖 Prompt for AI Agents
In apps/web/src/components/newsletter/Dropdown.tsx around lines 90-92 (and
similarly 110-111), the component uses a hard-coded
aria-labelledby="dropdown-label" which can produce duplicate IDs or point to a
non-existent label; change the component to accept an optional prop (e.g.
labelId) and use that for aria-labelledby when provided, otherwise generate a
stable unique id internally (React's useId or a prop fallback) or fall back to
using aria-label/aria-describedby, and wire the corresponding label element to
that id at the call sites so each CustomDropdown instance references a unique,
valid label ID.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/web/src/components/newsletter/Dropdown.tsx (1)
83-105: Add missing ARIA attributes to the trigger button.The trigger button lacks
aria-haspopup="listbox"andaria-expanded={open}, which are essential for screen readers to announce that this button opens a listbox and to convey whether it is currently expanded.Apply this diff to add the required ARIA attributes:
<button ref={buttonRef} onClick={() => setOpen(!open)} + aria-haspopup="listbox" + aria-expanded={open} onKeyDown={(e) => { if (e.key === "ArrowDown" || e.key === "ArrowUp") { e.preventDefault(); setOpen(true); } }} className=" w-full bg-black/20 border border-white/10 rounded-lg px-3 py-2 text-sm text-gray-200 flex items-center justify-between hover:bg-black/30 transition-all duration-200 focus:outline-none focus:ring-2 focus:ring-blue-500/50 " >
🧹 Nitpick comments (1)
apps/web/src/components/newsletter/Dropdown.tsx (1)
34-66: Consider guarding against empty options array.While unlikely in practice, if
options.lengthis 0, the modulo operations on lines 42 and 49 could produce unexpected results (NaN). Adding an early return improves defensive coding.Add this guard at the top of
handleKeyDown:function handleKeyDown(e: KeyboardEvent) { + if (options.length === 0) return; + const currentIndex = optionsRef.current.findIndex( (el) => el === document.activeElement );
| } | ||
| }, [open]); | ||
|
|
||
| const selected = options.find((opt) => opt.value === value); |
There was a problem hiding this comment.
Consider adding a fallback for invalid values.
If the value prop doesn't exist in the options array, the button will display nothing (line 99: selected?.label becomes undefined). This could occur if the parent passes an invalid value or the options array changes.
Consider adding a fallback:
>
- {selected?.label}
+ {selected?.label || "Select..."}
{open ? (Alternatively, validate the value prop or document that callers must ensure value exists in options.
Also applies to: 99-99
🤖 Prompt for AI Agents
In apps/web/src/components/newsletter/Dropdown.tsx around lines 79 and 99, the
selected lookup uses options.find(...) and then selected?.label is rendered,
which yields nothing when value isn't in options; add a fallback by resolving a
default label when selected is undefined (e.g., use a placeholder like
"Select..." or the first option's label) and ensure the component either
validates the value prop (type/check) or documents that callers must pass a
valid value; update the render to display that fallback and/or add a runtime
console warning or prop validation when an invalid value is detected.
| {open && ( | ||
| <div | ||
| className=" | ||
| absolute mt-2 w-full | ||
| bg-neutral-900 text-white | ||
| border border-white/10 | ||
| rounded-md shadow-lg shadow-black/40 | ||
| z-[9999] | ||
| " | ||
| > | ||
| {options.map((opt, index) => ( | ||
| <button | ||
| type="button" | ||
| key={opt.value} | ||
| ref={(el) => { optionsRef.current[index] = el; }} | ||
| role="option" | ||
| aria-selected={opt.value === value} | ||
| onClick={() => { | ||
| onChange(opt.value); | ||
| setOpen(false); | ||
| buttonRef.current?.focus(); | ||
| }} | ||
| className=" | ||
| w-full text-left px-3 py-2 text-sm rounded-md text-gray-100 cursor-pointer | ||
| hover:bg-neutral-700 transition-all duration-200 | ||
| focus:outline-none focus:bg-neutral-700 | ||
| aria-selected:bg-neutral-800 | ||
| " | ||
| > | ||
| {opt.label} | ||
| </button> | ||
| ))} | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Add role="listbox" to the dropdown container.
The container div renders options with role="option" but lacks role="listbox" itself. Screen readers require the parent to have role="listbox" to properly announce the relationship between the container and its options.
Apply this diff to add the missing role:
{open && (
<div
+ role="listbox"
className="
absolute mt-2 w-full
bg-neutral-900 text-white
border border-white/10
rounded-md shadow-lg shadow-black/40
z-[9999]
"
>🤖 Prompt for AI Agents
In apps/web/src/components/newsletter/Dropdown.tsx around lines 107 to 140, the
dropdown container div that wraps the option buttons currently lacks an
accessible container role; add role="listbox" to that div (the element rendered
when open is true) so screen readers perceive the options' relationship; keep
the existing option buttons with role="option" unchanged and ensure the role
attribute is added directly on the outer div.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/src/utils/newsletter/getAllNewsLetter.ts(1 hunks)
🔇 Additional comments (5)
apps/web/src/utils/newsletter/getAllNewsLetter.ts (5)
1-3: LGTM!Imports are appropriate for reading and parsing markdown files with frontmatter.
5-13: LGTM!Type definition is clear and matches the newsletter frontmatter structure described in the PR objectives.
26-32: LGTM!The filtering logic correctly excludes example files and includes only
.mdfiles as intended.
41-45: LGTM!The keyword normalization logic handles both array and comma-separated string formats with appropriate fallbacks.
17-24: Synchronous file operations are appropriate for this Server Component context.Verification confirms that
getAllNewsLetter()is called in a Next.js Server Component (no "use client" directive present inapps/web/src/app/(main)/dashboard/newsletters/page.tsx). In Server Components, synchronous file system operations are standard and acceptable practice, as they execute server-side during request handling without blocking the event loop for other concurrent requests.
| return { | ||
| id: (data.id as string | undefined) ?? slug, | ||
| title: (data.title as string | undefined) ?? "Untitled Newsletter", | ||
| summary: (data.summary as string | undefined) ?? "", | ||
| keywords, | ||
| time: | ||
| (data.time as string | undefined) ?? | ||
| (data.date as string | undefined) ?? | ||
| new Date().toISOString(), | ||
| readTime: (data.readTime as string | undefined) ?? "1 min read", | ||
| slug, | ||
| } satisfies Newsletter; |
There was a problem hiding this comment.
Time fallback may produce unexpected results.
Line 52-55: The fallback chain ends with new Date().toISOString(), meaning newsletters without time or date fields will get the current timestamp. This could lead to incorrect chronological ordering and confusing dates displayed to users.
Consider either:
- Making
time/daterequired in frontmatter and throwing an error if missing - Using a sentinel value (e.g., epoch zero) to identify newsletters with missing dates
- Logging a warning when falling back to the current date
Additionally, consider validating frontmatter with a schema library (e.g., zod) rather than relying on type assertions. This would catch malformed frontmatter early with clear error messages.
Apply this diff to add basic validation:
+ // Validate required fields
+ if (!data.time && !data.date) {
+ console.warn(`Newsletter ${filename} is missing time/date field`);
+ }
+
return {Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/src/utils/newsletter/getAllNewsLetter.ts around lines 47 to 58, the
current fallback for time/date uses new Date().toISOString() which can produce
misleading current timestamps for newsletters missing frontmatter; replace this
by validating the frontmatter and handling missing dates explicitly: add a
lightweight validation (preferably with zod) that requires either time or date
and fails the build or throws a clear error when both are missing, or if you
prefer non-blocking behavior set time to a sentinel value (e.g., new
Date(0).toISOString()) and emit a console/processLogger.warn when falling back
so consumers can detect missing dates; update the returned object to use the
validated/normalized value (or sentinel) instead of defaulting to now and stop
using unchecked type assertions for frontmatter.
| } catch { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Add error logging for debugging.
The empty catch blocks silently swallow all errors, making it difficult to diagnose issues with newsletter files (malformed frontmatter, encoding problems, missing fields, etc.). This will make debugging and content authoring harder for the team.
Apply this diff to add error logging:
- } catch {
+ } catch (error) {
+ console.error(`Failed to parse newsletter file ${filename}:`, error);
return null;
}
})
.filter((newsletter): newsletter is Newsletter => newsletter !== null);
- } catch {
+ } catch (error) {
+ console.error('Failed to read newsletter directory:', error);
return [];
}Also applies to: 64-66
🤖 Prompt for AI Agents
In apps/web/src/utils/newsletter/getAllNewsLetter.ts around lines 59-61 and
64-66, the catch blocks silently swallow errors; modify each catch to accept the
error (catch (error)) and log the error before returning null — e.g., call your
project logger or console.error with a clear message including the newsletter
file path or identifier and the error object (message/stack) to aid debugging,
then return null as before.
| return null; | ||
| } | ||
| }) | ||
| .filter((newsletter): newsletter is Newsletter => newsletter !== null); |
There was a problem hiding this comment.
Missing required sorting by date.
The PR objectives and issue #155 explicitly require newsletters to be "ordered by date (latest first)", but this function returns newsletters in filesystem order without sorting. This means the newsletter listing page won't display items in the correct chronological order.
Apply this diff to sort newsletters by date descending:
.filter((newsletter): newsletter is Newsletter => newsletter !== null);
+ .sort((a, b) => new Date(b.time).getTime() - new Date(a.time).getTime());
} catch {Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/src/utils/newsletter/getAllNewsLetter.ts around line 63, after
filtering out null newsletters, the list is not being sorted; update the
pipeline to sort newsletters by their date field in descending order (latest
first) — e.g. after the .filter(...) call, add a .sort(...) that compares parsed
Date values (or timestamps) from newsletter.date, handling missing/invalid dates
by treating them as oldest so they appear last.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/web/src/hooks/useNewsletterFilters.ts (1)
46-56: Add date validation in month filtering logic.Similar to the month options generation, lines 50-54 create a Date and format it without validating whether the date is valid. Invalid dates will cause incorrect filtering behavior.
Apply this diff to add validation:
// Filter by month const filteredByMonth = selectedMonth === "all" ? filtered : filtered.filter((item) => { const date = new Date(item.time); + // Skip items with invalid dates + if (isNaN(date.getTime())) { + return false; + } const monthYear = date.toLocaleDateString("en-US", { month: "long", year: "numeric", }); return monthYear === selectedMonth; });
🧹 Nitpick comments (1)
apps/web/src/hooks/useNewsletterFilters.ts (1)
12-56: Consider extracting date formatting logic to reduce duplication.The pattern of creating a Date and formatting it to "Month Year" appears multiple times (lines 15-19 and 50-54). Extracting this to a helper function would reduce duplication and make future maintenance easier.
For example:
function getMonthYear(timeString: string): string | null { const date = new Date(timeString); if (isNaN(date.getTime())) { return null; } return date.toLocaleDateString("en-US", { month: "long", year: "numeric", }); }Then use it in both places:
// In monthOptions generation const monthYear = getMonthYear(item.time); if (monthYear) months.add(monthYear); // In month filtering const monthYear = getMonthYear(item.time); if (!monthYear) return false; return monthYear === selectedMonth;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/src/app/(main)/dashboard/newsletters/NewsletterPage.tsx(1 hunks)apps/web/src/hooks/useNewsletterFilters.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/app/(main)/dashboard/newsletters/NewsletterPage.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/hooks/useNewsletterFilters.ts (1)
apps/web/src/utils/newsletter/getAllNewsLetter.ts (1)
Newsletter(5-13)
🔇 Additional comments (4)
apps/web/src/hooks/useNewsletterFilters.ts (4)
1-4: LGTM - clean imports and type definition.The import of
Newslettertype from the utility file follows the single source of truth pattern, and theSortOrdertype provides good type safety for the sort options.
58-68: LGTM - sort logic properly handles invalid dates.The sort logic correctly validates dates using
isNaN(dateA) || isNaN(dateB)before comparing, preventing crashes from malformed date strings.
33-43: LGTM - comprehensive search filter implementation.The search filter appropriately checks across title, summary, and keywords with case-insensitive matching, providing a good user experience.
71-92: LGTM - clean utility functions and comprehensive return interface.The
clearFiltersfunction properly resets state,hasActiveFiltersappropriately excludes sort order (since sorting isn't technically filtering), and the return statement exposes a well-structured API for consuming components.
| const monthOptions = useMemo(() => { | ||
| const months = new Set<string>(); | ||
| initialData.forEach((item) => { | ||
| const date = new Date(item.time); | ||
| const monthYear = date.toLocaleDateString("en-US", { | ||
| month: "long", | ||
| year: "numeric", | ||
| }); | ||
| months.add(monthYear); | ||
| }); | ||
|
|
||
| return [ | ||
| { label: "All Months", value: "all" }, | ||
| ...Array.from(months) | ||
| .sort((a, b) => new Date(b).getTime() - new Date(a).getTime()) | ||
| .map((month) => ({ label: month, value: month })), | ||
| ]; | ||
| }, [initialData]); |
There was a problem hiding this comment.
Add missing date validation in month options generation.
Despite the previous review comment being marked as addressed, the current code at lines 15-19 does not validate whether item.time produces a valid Date. If new Date(item.time) returns an Invalid Date, toLocaleDateString returns "Invalid Date" as a string, which pollutes the month options dropdown.
Additionally, line 26 assumes month strings like "November 2025" parse correctly, which may fail with unexpected formats.
Apply this diff to add validation:
const monthOptions = useMemo(() => {
const months = new Set<string>();
initialData.forEach((item) => {
const date = new Date(item.time);
+ // Skip invalid dates
+ if (isNaN(date.getTime())) {
+ console.warn(`Invalid date in newsletter: ${item.id || item.slug}`);
+ return;
+ }
const monthYear = date.toLocaleDateString("en-US", {
month: "long",
year: "numeric",
});
months.add(monthYear);
});
return [
{ label: "All Months", value: "all" },
...Array.from(months)
.sort((a, b) => new Date(b).getTime() - new Date(a).getTime())
.map((month) => ({ label: month, value: month })),
];
}, [initialData]);🤖 Prompt for AI Agents
In apps/web/src/hooks/useNewsletterFilters.ts around lines 12-29, the code
currently creates month options without validating dates and later sorts by
constructing new Dates from the month label which may fail; update the
generation to (1) for each item, create a Date from item.time and only add month
strings when date is valid (e.g., check isFinite(date.getTime()) or
!isNaN(date.getTime())) so "Invalid Date" is never inserted, and (2) when
sorting the unique month labels, do not rely on parsing the label directly with
new Date(label); instead parse the label into month name and year (or map month
names to month indexes) and construct a deterministic Date (e.g., new Date(year,
monthIndex, 1) or Date.UTC) to sort by timestamp descending before mapping to
{label,value} entries.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
apps/web/src/app/(main)/dashboard/newsletters/page.tsx (1)
4-8: Consider error handling for better debugging.While
getAllNewsLetter()has internal error handling that returns an empty array on failure (preventing crashes), the page has no visibility into whether the empty state is due to no newsletters or a loading error. This makes debugging content issues harder.Consider logging or distinguishing between empty and error states:
export default function Page() { const newsletters = getAllNewsLetter(); + + // Optional: Log for debugging if empty (could indicate an error) + if (newsletters.length === 0) { + console.log('[Newsletter Page] No newsletters loaded - check content/newsletters directory'); + } return <NewsletterPage initialData={newsletters} />; }apps/web/src/utils/newsletter/newsletter-metadata.ts (4)
17-67: Consider adding default date-based sorting.While the UI (NewsletterPage) handles sorting via
useNewsletterFilters, having a default sort order here ensures newsletters are consistently ordered regardless of wheregetAllNewsLetter()is called. This would align with the PR objective of "ordered by date (latest first)".Apply this diff to add default sorting:
.filter((newsletter): newsletter is Newsletter => newsletter !== null); + .sort((a, b) => { + const dateA = new Date(a.time).getTime(); + const dateB = new Date(b.time).getTime(); + return dateB - dateA; // Latest first + }); } catch (error) {
59-61: Add error logging for debugging malformed newsletter files.The empty catch block silently swallows all parsing errors (malformed frontmatter, encoding issues, missing fields), making it difficult to diagnose issues when adding new newsletter content.
Apply this diff:
- } catch { + } catch (error) { + console.error(`Failed to parse newsletter file ${filename}:`, error); return null; }
64-66: Add error logging for directory read failures.The empty catch block hides directory-level errors, making it hard to debug when the newsletter directory is misconfigured or inaccessible.
Apply this diff:
- } catch { + } catch (error) { + console.error('Failed to read newsletter directory:', error); return []; }
52-55: Time fallback may produce misleading timestamps.Using
new Date().toISOString()as a fallback means newsletters missingtimeordatefields will get the current timestamp, leading to incorrect chronological ordering and confusing dates in the UI.Consider one of these approaches:
- Validate and require time/date (recommended):
+ if (!data.time && !data.date) { + console.error(`Newsletter ${filename} is missing required time/date field`); + return null; + } + return { id: (data.id as string | undefined) ?? slug, title: (data.title as string | undefined) ?? "Untitled Newsletter", summary: (data.summary as string | undefined) ?? "", keywords, time: (data.time as string | undefined) ?? - (data.date as string | undefined) ?? - new Date().toISOString(), + (data.date as string | undefined)!,
- Or use a sentinel value to mark missing dates:
time: (data.time as string | undefined) ?? (data.date as string | undefined) ?? - new Date().toISOString(), + new Date(0).toISOString(), // Epoch zero signals missing date
🧹 Nitpick comments (1)
apps/web/src/app/(main)/dashboard/newsletters/NewsletterPage.tsx (1)
48-59: Add aria-label to search input for accessibility.The search input is missing an
aria-labelattribute, which is important for screen reader users since the placeholder text is not read as a label.Apply this diff:
<input type="text" placeholder="Search newsletter..." + aria-label="Search newsletters" value={search} onChange={(e) => setSearch(e.target.value)} className="
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/web/content/newsletters/example.md(1 hunks)apps/web/content/newsletters/test-1.md(1 hunks)apps/web/content/newsletters/test-2.md(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/NewsletterPage.tsx(1 hunks)apps/web/src/app/(main)/dashboard/newsletters/page.tsx(1 hunks)apps/web/src/hooks/useNewsletterFilters.ts(1 hunks)apps/web/src/utils/newsletter/newsletter-metadata.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/hooks/useNewsletterFilters.ts
- apps/web/content/newsletters/example.md
🧰 Additional context used
🧬 Code graph analysis (2)
apps/web/src/app/(main)/dashboard/newsletters/NewsletterPage.tsx (4)
apps/web/src/utils/newsletter/newsletter-metadata.ts (1)
Newsletter(5-13)apps/web/src/hooks/useNewsletterFilters.ts (1)
useNewsletterFilters(6-93)apps/web/src/components/newsletter/Dropdown.tsx (1)
CustomDropdown(6-143)apps/web/src/components/newsletter/NewsletterItem.tsx (1)
NewsletterItem(13-86)
apps/web/src/app/(main)/dashboard/newsletters/page.tsx (2)
apps/web/src/utils/newsletter/newsletter-metadata.ts (1)
getAllNewsLetter(17-67)apps/web/src/app/(main)/dashboard/newsletters/NewsletterPage.tsx (1)
NewsletterPage(9-130)
🪛 LanguageTool
apps/web/content/newsletters/test-2.md
[style] ~30-~30: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...till publish it and let people use it. When your resume looks like a blank slate, b...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
apps/web/content/newsletters/test-1.md
[style] ~28-~28: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ing. No college degree. No connections. No safety net. Just documentation, tutoria...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~61-~61: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...mission. You don't need a fancy degree. You need a computer, internet, and the refu...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (3)
apps/web/content/newsletters/test-1.md (1)
1-63: LGTM! Well-structured newsletter content.The frontmatter correctly follows the Newsletter schema and the content is well-organized with proper Markdown formatting. The repetitive sentence structures flagged by static analysis (lines 28 and 61) are intentional rhetorical devices (anaphora) used for emphasis and don't need to be changed.
apps/web/content/newsletters/test-2.md (1)
1-68: LGTM! Engaging newsletter content with proper structure.The frontmatter follows the Newsletter schema correctly and the content is well-formatted. The repetitive "When" structure flagged by static analysis (line 30) is an intentional stylistic choice (anaphora) that enhances the message and doesn't require changes.
apps/web/src/app/(main)/dashboard/newsletters/NewsletterPage.tsx (1)
9-129: Well-structured component with comprehensive filtering and responsive design.The component effectively integrates filtering, sorting, and responsive layouts. The empty state handling with conditional "Clear all filters" action provides good UX.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/src/components/dashboard/Sidebar.tsx (1)
64-66: Newsletter navigation handler is fine (minor naming nit only)
handleNewsLettercorrectly routes to/dashboard/newsletters. If you touch this again, consider renaming tohandleNewsletterfor consistency with the route and label, but this is purely optional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/src/components/dashboard/Sidebar.tsx(3 hunks)apps/web/tailwind.config.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/tailwind.config.ts
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/components/dashboard/Sidebar.tsx (2)
apps/web/src/components/sheet/OpensoxProBadge.tsx (1)
OpensoxProBadge(8-28)apps/web/src/components/sidebar/SidebarItem.tsx (1)
SidebarItem(13-42)
🔇 Additional comments (1)
apps/web/src/components/dashboard/Sidebar.tsx (1)
24-24: Mailbox icon import is consistent with existing icon usageImporting
Mailboxfromlucide-reactaligns with how other lucide icons (e.g.,ChevronLeftIcon,ChevronRightIcon) are used; no issues here.
| {!isCollapsed && !isPaidUser ? ( | ||
| <div | ||
| className="w-full h-[44px] flex items-center rounded-md cursor-pointer transition-colors px-2 gap-3 pl-3 hover:bg-[#121214]" | ||
| onClick={handleNewsLetter} | ||
| > | ||
| <span className="shrink-0 text-[#eaeaea]"> | ||
| <Mailbox className="size-5" /> | ||
| </span> | ||
| <div className="flex items-center gap-1"> | ||
| <h1 className="text-xs font-medium text-[#c8c8c8] group-hover:text-ox-purple"> | ||
| Newsletter | ||
| </h1> | ||
| <OpensoxProBadge className="px-1.5 py-0.5 scale-75" /> | ||
| </div> | ||
| </div> | ||
| ) : ( | ||
| <SidebarItem | ||
| itemName="Newsletter" | ||
| onclick={handleNewsLetter} | ||
| icon={<Mailbox className="size-5" />} | ||
| collapsed={isCollapsed} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
🧩 Analysis chain
Badge click likely conflicts with parent onClick; consider refactor to reuse SidebarItem
-
Conflicting navigation on badge click (minor UX bug)
In the expanded, non‑paid case, the parent<div>usesonClick={handleNewsLetter}(routing to/dashboard/newsletters), whileOpensoxProBadgerenders a nested<Link href="/pricing">. Clicking the badge will trigger both handlers; the final navigation is likely/dashboard/newsletters, so the badge won’t reliably act as a pricing link. Consider either:- Removing the nested
Linkfor this specific usage and routing to pricing via a click handler that callsevent.stopPropagation()and thenrouter.push("/pricing"), or - Keeping the badge purely decorative here (no
Link) and relying on the newsletter page to handle the upgrade flow.
Please manually test clicking exactly on the “OX Pro” badge inside the Newsletter row and confirm whether you land on
/pricingor/dashboard/newsletters. - Removing the nested
-
Reuse
SidebarItemfor consistency and less duplication
The custom<div>duplicatesSidebarItem’s layout/hover behavior and partially diverges from existing design tokens (hard‑coded hex colors instead oftext-text-*andhover:bg-dash-hover). SinceSidebarItemalready accepts abadgeprop, you could simplify to something like:<SidebarItem itemName="Newsletter" onclick={handleNewsLetter} icon={<Mailbox className="size-5" />} collapsed={isCollapsed} badge={!isPaidUser ? <OpensoxProBadge className="px-1.5 py-0.5 scale-75" /> : undefined} />
and then conditionally render the whole block based on
isPaidUseras you do now. This keeps styling consistent and avoids a custom one‑off row. -
Minor Tailwind group-hover issue
The heading usesgroup-hover:text-ox-purplebut the root<div>doesn’t include thegroupclass, so that hover style won’t take effect. Either addgroupto the root or change to a simplehover:modifier on the text as desired.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find and examine the OpensoxProBadge component
fd -t f "OpensoxProBadge" --exec cat {} \;
Length of output: 759
🏁 Script executed:
#!/bin/bash
# Search for OpensoxProBadge definition
rg -n "export.*OpensoxProBadge\|const OpensoxProBadge\|function OpensoxProBadge" -A 20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Examine SidebarItem component to check props
rg -n "export.*SidebarItem\|const SidebarItem\|function SidebarItem" -A 30 --type=tsxLength of output: 27
🏁 Script executed:
#!/bin/bash
# View the full Sidebar.tsx file around the target lines
cat apps/web/src/components/dashboard/Sidebar.tsx | head -170 | tail -50Length of output: 1959
🏁 Script executed:
#!/bin/bash
# Check for design token definitions (text-text-*, hover:bg-dash-hover)
rg -n "text-text-|hover:bg-dash-hover|@apply.*text-text" --type=tsx --type=css | head -20Length of output: 27
---
🏁 Script executed:
```shell
#!/bin/bash
# Find SidebarItem file without type filter
fd -t f "SidebarItem" --exec cat {} \;
Length of output: 1007
🏁 Script executed:
#!/bin/bash
# Search for SidebarItem definition with broader pattern
rg -n "SidebarItem\s*=" -A 30 | head -60Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look for prop interfaces or type definitions for SidebarItem
rg -n "interface.*SidebarItem|type.*SidebarItem" -A 15Length of output: 1093
🏁 Script executed:
#!/bin/bash
# Check if SidebarItem accepts badge prop anywhere in usage or definition
rg -n "badge\s*=" -C 3 | grep -A 3 -B 3 "SidebarItem"Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check full newsletter section in context
sed -n '130,170p' apps/web/src/components/dashboard/Sidebar.tsxLength of output: 1703
Fix badge click event handling and CSS styling inconsistencies in Newsletter section
The expanded, non-paid Newsletter row has three issues:
-
Event handler conflict on badge click:
OpensoxProBadgewraps its content in<Link href="/pricing">without callingevent.stopPropagation(). Clicking the badge will fire both the parentonClick={handleNewsLetter}and the nested Link navigation, likely landing on/dashboard/newslettersinstead of/pricing. Addevent.stopPropagation()insideOpensoxProBadgeor userouter.push()with a click handler that prevents default behavior. -
Missing
groupclass: The root<div>lacks thegroupclass, so thegroup-hover:text-ox-purplemodifier on the heading won't work. Addgroupto the root div className. -
Design token mismatch: Replace hardcoded hex colors (
#eaeaea,#c8c8c8,#121214) with design tokens already used elsewhere in the sidebar (text-text-secondary,text-text-tertiary,hover:bg-dash-hover). Also, consider usingSidebarItemwith thebadgeprop instead of a custom div to maintain consistency:
<SidebarItem
itemName="Newsletter"
onclick={handleNewsLetter}
icon={<Mailbox className="size-5" />}
collapsed={isCollapsed}
badge={!isPaidUser ? <OpensoxProBadge className="px-1.5 py-0.5 scale-75" /> : undefined}
/>🤖 Prompt for AI Agents
In apps/web/src/components/dashboard/Sidebar.tsx around lines 136-158, the
expanded non-paid Newsletter row needs three fixes: ensure the OpensoxProBadge
click does not bubble to the parent by adding event.stopPropagation() in the
badge’s click handler or converting the badge to a button that calls
router.push('/pricing') while preventing propagation; add the "group" class to
the root div so group-hover styles on the heading take effect; and replace
hard‑coded hex colors (#eaeaea, #c8c8c8, #121214) with existing design tokens
(text-text-secondary, text-text-tertiary, hover:bg-dash-hover) for consistency —
alternatively simplify by using the SidebarItem component with a badge prop when
!isPaidUser to keep behavior and styling consistent.
|
@deveshru2712 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
Add Newsletter Support to Opensox.in
This PR introduces a dedicated newsletter section to Opensox.in, enabling the platform to display newsletter content alongside existing blog posts.
For Non-Pro Users
non2.mp4
For Pro Users
pro2.mp4
Additional Enhancements
🔧 Implementation Details
📁 Content Management
content/newsletters/.titlesummarykeywordsdatereadTimeslugexamplefile is included to define the required metadata structure.📝 Content Processing
📦 Dependencies Added (Use Cases)
gray-matter— extracts frontmatter metadata from MD/MDX files.remark— processes Markdown content.remark-gfm— enables GitHub-style markdown features (tables, task lists, etc.).remark-html— converts Markdown into HTML.@tailwindcss/typography— improves typography styling for rendered HTML content.I have also added sample newsletters for testing.
fixes #155
Summary by CodeRabbit
New Features
Documentation
Style
Chores