[bounty-to-hire]: create ui for newsletter #162
Conversation
|
@ad1tyayadav 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. |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR introduces a newsletter feature to the web app, including dashboard pages for browsing and viewing individual newsletters with search and month filtering capabilities. It also upgrades Prisma dependencies and adds markdown rendering support. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant NewsletterPage as Newsletter Page
participant Container as NewsletterContainer
participant List as NewsletterList
participant Card as NewsletterCard
participant ArticlePage as Article Page [slug]
User->>NewsletterPage: Navigate to /newsletters
NewsletterPage->>Container: Render container
Container->>List: Pass query & monthFilter
List->>List: Filter & group by date
List->>Card: Render per newsletter
Card-->>User: Display card grid
User->>Container: Type search or select month
Container->>List: Update props
List->>List: Recompute filtered list
List-->>User: Update display
User->>Card: Click newsletter
Card->>ArticlePage: Navigate to /newsletters/[slug]
ArticlePage->>ArticlePage: Find newsletter by slug
ArticlePage->>ArticlePage: Render header, content, footer
ArticlePage-->>User: Display article with reading time
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
1 similar 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/src/app/(main)/dashboard/newsletters/page.tsx (1)
5-6: Consider removing duplicate background styling.The page wrapper sets
bg-[#101010]whileNewsletterContaineralso setsbg-black(as seen in the relevant code snippets). Since the container already handles the background, the wrapper's background is redundant.Apply this diff to simplify:
export default function Page() { return ( - <div className="min-h-screen bg-[#101010]"> + <div className="min-h-screen"> <NewsletterContainer /> </div> )apps/web/src/data/newsletters.ts (1)
12-445: Consider migrating to a database for better scalability.While hardcoded newsletter data works for the initial implementation, it has limitations:
- Requires code deployment to add/update newsletters
- No content management workflow
- Difficult to scale with many newsletters
- No ability for non-developers to manage content
For production readiness, consider migrating to a database with a CMS or admin interface. For now, this approach is acceptable as an MVP.
Would you like me to open an issue to track the migration to a database-backed solution with a content management interface?
apps/web/src/components/newsletters/NewsletterCrad.tsx (2)
13-18: Improve image alt text for accessibility.Using
alt={n.title}(line 15) makes the alt text identical to the visible title, which doesn't provide additional context for screen reader users. The alt text should describe what the image depicts, not repeat the title.Consider one of these approaches:
Option 1: Add a descriptive alt field to the Newsletter type:
// In newsletters.ts export type Newsletter = { // ...existing fields hero?: string; heroAlt?: string; // Add this // ... };Option 2: Use a descriptive fallback:
<Image src={n.hero} - alt={n.title} + alt={`Hero image for ${n.title}`} fill className="object-cover group-hover:scale-[1.02] transition-transform duration-700 ease-out" />Option 3: If images are purely decorative, use empty alt:
- alt={n.title} + alt=""
44-48: Consider supporting internationalization for date formatting.The locale is hardcoded to
"en-US"(line 44). For better internationalization support, consider using the user's locale or making it configurable.{new Date(n.date).toLocaleDateString("en-US", { + // TODO: Use user's locale from context/settings + // locale: userLocale || "en-US" year: "numeric", month: "short", day: "numeric", })}Or use the browser's default locale:
- {new Date(n.date).toLocaleDateString("en-US", { + {new Date(n.date).toLocaleDateString(undefined, { year: "numeric", month: "short", day: "numeric", })}apps/web/src/components/newsletters/NewsletterContainer.tsx (1)
12-22: Avoid hardcoding month filters; derive from data insteadThe month filter list is currently hardcoded (and wrapped in an unnecessary
useMemo). This will get out of sync as more newsletters are added or dates move beyond 2025.Consider deriving the available months from
NEWSLETTERS(or a helper) and keeping a stable"all"option so the UI always reflects real data, e.g.:// pseudo‑example const months = useMemo( () => getAvailableMonthsFromNewsletters(NEWSLETTERS), [] );You can also drop
useMemoentirely if this stays cheap.apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx (1)
3-13: Watch bundle size from client-side NEWSLETTERS importLooking up the newsletter client‑side via
NEWSLETTERS.findworks, but it also ships all newsletter bodies to the browser. As the number and length of newsletters grow, this page’s JS payload could become large.Longer term, consider:
- Moving the heavy content lookup to a server component or route handler, or
- Splitting bodies into MD files and loading only the requested article.
For the current small data set this is fine; just something to keep in mind as content scales.
apps/web/src/components/newsletters/NewsletterContent.tsx (2)
1-1: Avoid file-wide eslint disable for unused varsDisabling
@typescript-eslint/no-unused-varsfor the whole file is heavy‑handed. Here it seems driven by destructured props likenodethat aren’t used.Prefer tightening the code instead, e.g.:
- Drop unused destructured fields or rename them to
_node, or- Use
// eslint-disable-next-line @typescript-eslint/no-unused-varson a specific line if absolutely needed.This keeps the rest of the file benefiting from the rule.
99-105: Verifyborder-l-3utility exists in your CSS setupThe
blockquoteusesborder-l-3, which isn’t a standard Tailwind utility unless you’ve added a custom border width. If it isn’t configured, you’ll silently get no left border.If you intended a 3px border with default Tailwind config, something like
border-l-[3px](or a standardborder-l,border-l-2, etc.) would be safer.
📜 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 (12)
apps/api/package.json(1 hunks)apps/web/next.config.js(1 hunks)apps/web/package.json(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(2 hunks)apps/web/src/components/newsletters/NewsletterContainer.tsx(1 hunks)apps/web/src/components/newsletters/NewsletterContent.tsx(1 hunks)apps/web/src/components/newsletters/NewsletterCrad.tsx(1 hunks)apps/web/src/components/newsletters/NewsletterList.tsx(1 hunks)apps/web/src/data/newsletters.ts(1 hunks)apps/web/tsconfig.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
apps/web/src/components/newsletters/NewsletterContainer.tsx (1)
apps/web/src/components/newsletters/NewsletterList.tsx (1)
NewsletterList(5-91)
apps/web/src/components/newsletters/NewsletterCrad.tsx (1)
apps/web/src/data/newsletters.ts (1)
Newsletter(2-10)
apps/web/src/components/newsletters/NewsletterList.tsx (2)
apps/web/src/data/newsletters.ts (2)
NEWSLETTERS(12-445)Newsletter(2-10)apps/web/src/components/newsletters/NewsletterCrad.tsx (1)
NewsletterCard(6-55)
apps/web/src/app/(main)/dashboard/newsletters/page.tsx (1)
apps/web/src/components/newsletters/NewsletterContainer.tsx (1)
NewsletterContainer(7-129)
apps/web/src/components/dashboard/Sidebar.tsx (1)
apps/web/src/components/sidebar/SidebarItem.tsx (1)
SidebarItem(12-28)
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx (2)
apps/web/src/data/newsletters.ts (1)
NEWSLETTERS(12-445)apps/web/src/components/newsletters/NewsletterContent.tsx (1)
NewsletterContent(49-177)
🔇 Additional comments (7)
apps/web/next.config.js (1)
9-13: LGTM!The remote pattern configuration correctly enables Next.js Image optimization for Pexels images used in the newsletter hero images.
apps/web/src/data/newsletters.ts (1)
2-10: LGTM!The Newsletter type is well-structured with appropriate required and optional fields. The date as a string enables easy serialization.
apps/api/package.json (1)
22-29: Prisma 6 upgrade is compatible—no breaking changes detected in codebase.Verification against all seven Prisma 6.0 breaking changes confirms compatibility:
- Node.js/TypeScript versions: Codebase requires Node >=18 and TypeScript 5.4.5, both meeting or exceeding Prisma 6 minimums
- PostgreSQL implicit many-to-many relations: Schema uses explicit foreign keys with
@relation; no implicit relations requiring migration- Full-text search: Not used in schema
- Bytes type: No Prisma Bytes fields; Buffer usage is limited to crypto operations unrelated to Prisma
- NotFoundError: Not used; codebase uses safe patterns (findUnique, findFirst, etc.) without custom error handling
- Reserved model names: No models named async, await, or using
- Migrations: Six migration files exist and appear current
Database migrations have been versioned and no schema compatibility issues detected.
apps/web/package.json (1)
36-39: Dependency versions verified and confirmed current with no security vulnerabilities.Both
react-markdown@10.1.0andremark-gfm@4.0.1are the latest stable releases in their respective major versions. Security advisory queries returned no known vulnerabilities for either package.apps/web/src/components/newsletters/NewsletterContainer.tsx (1)
59-101: Dropdown implementation looks solidThe month dropdown, overlay click‑to‑close, and chevron rotation are all wired correctly and avoid z‑index traps. No functional issues spotted here.
apps/web/src/app/(main)/dashboard/newsletters/[slug]/page.tsx (1)
31-86: Reading time and meta bar logic look correctThe word count and
Math.ceil(wordCount / 200)reading time computation are sound, and date formatting viatoLocaleDateStringmatches the rest of the UI.apps/web/src/components/newsletters/NewsletterList.tsx (1)
45-90: Empty state and grouping behavior look goodThe empty state correctly distinguishes between “no content yet” and “no results for current filters,” and the grouping by year/month with responsive grid layout is clean and readable. No issues here.
| <Link | ||
| href="newsletters" | ||
| className={getSidebarLinkClassName(pathname, "/newsletters")} | ||
| > | ||
| <SidebarItem | ||
| itemName="Newsletters" | ||
| icon={<NewspaperIcon className="size-5" />} | ||
| collapsed={isCollapsed} | ||
| /> | ||
| </Link> |
There was a problem hiding this comment.
Fix the inconsistent route path for Newsletters.
The href "newsletters" uses a relative path, while other sidebar items use absolute paths like "/dashboard/home" and "/dashboard/projects" (lines 30, 35). This inconsistency can cause navigation issues and breaks the established pattern.
Apply this diff to use an absolute path:
<Link
- href="newsletters"
+ href="/dashboard/newsletters"
className={getSidebarLinkClassName(pathname, "/newsletters")}
>Also update the className computation to use the full path:
<Link
href="/dashboard/newsletters"
- className={getSidebarLinkClassName(pathname, "/newsletters")}
+ className={getSidebarLinkClassName(pathname, "/dashboard/newsletters")}
>📝 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.
| <Link | |
| href="newsletters" | |
| className={getSidebarLinkClassName(pathname, "/newsletters")} | |
| > | |
| <SidebarItem | |
| itemName="Newsletters" | |
| icon={<NewspaperIcon className="size-5" />} | |
| collapsed={isCollapsed} | |
| /> | |
| </Link> | |
| <Link | |
| href="/dashboard/newsletters" | |
| className={getSidebarLinkClassName(pathname, "/dashboard/newsletters")} | |
| > | |
| <SidebarItem | |
| itemName="Newsletters" | |
| icon={<NewspaperIcon className="size-5" />} | |
| collapsed={isCollapsed} | |
| /> | |
| </Link> |
🤖 Prompt for AI Agents
In apps/web/src/components/dashboard/Sidebar.tsx around lines 142 to 151, the
Newsletters link uses a relative href ("newsletters") and passes "/newsletters"
to getSidebarLinkClassName, breaking the absolute path pattern used elsewhere;
change the href to the absolute route "/dashboard/newsletters" and update the
className call to use getSidebarLinkClassName(pathname,
"/dashboard/newsletters") so the link and active-class computation follow the
same full-path convention.
| const CodeBlock = ({ children, className }: { children: React.ReactNode; className?: string }) => { | ||
| const [copied, setCopied] = useState(false); | ||
|
|
||
| const handleCopy = async () => { | ||
| const text = String(children).replace(/\n$/, ''); | ||
| await navigator.clipboard.writeText(text); | ||
| setCopied(true); | ||
| setTimeout(() => setCopied(false), 2000); | ||
| }; |
There was a problem hiding this comment.
Handle clipboard failures gracefully in CodeBlock
navigator.clipboard.writeText can fail (permissions, unsupported environment) and currently the error would be unhandled and the UI still shows the copy button as if everything is fine.
Consider wrapping this in a try/catch and only setting copied when the write succeeds, e.g.:
const handleCopy = async () => {
const text = String(children).replace(/\n$/, "");
try {
await navigator.clipboard.writeText(text);
setCopied(true);
setTimeout(() => setCopied(false), 2000);
} catch {
// Optionally log or show a subtle failure state
}
};This makes the button more robust across browsers and contexts.
🤖 Prompt for AI Agents
In apps/web/src/components/newsletters/NewsletterContent.tsx around lines 11 to
19, wrap the navigator.clipboard.writeText call in a try/catch (or
feature-detect navigator.clipboard) so clipboard failures don't go unhandled;
only call setCopied(true) and start the timeout when the writeText promise
resolves successfully, and in the catch branch either silently ignore or
log/report the error so the UI doesn't falsely show a successful copy.
| import Link from "next/link"; | ||
| import Image from "next/image"; | ||
| import { Newsletter } from "@/data/newsletters"; | ||
| import { CalendarIcon } from "@heroicons/react/24/outline"; | ||
|
|
||
| export default function NewsletterCard({ n }: { n: Newsletter }) { | ||
| return ( | ||
| <article className="group"> | ||
| <Link href={`/dashboard/newsletters/${n.slug}`} className="block"> | ||
| {/* Hero Image */} | ||
| {n.hero && ( | ||
| <div className="relative w-full aspect-[16/10] overflow-hidden bg-[#15161a] mb-6"> | ||
| <Image | ||
| src={n.hero} | ||
| alt={n.title} | ||
| fill | ||
| className="object-cover group-hover:scale-[1.02] transition-transform duration-700 ease-out" | ||
| /> | ||
| <div className="absolute inset-0 bg-gradient-to-t from-black/60 via-transparent to-transparent" /> | ||
| </div> | ||
| )} | ||
|
|
||
| {/* Content */} | ||
| <div className={n.hero ? "" : "pt-2"}> | ||
| {n.featured && ( | ||
| <div className="mb-4"> | ||
| <span className="text-xs tracking-[0.2em] uppercase text-zinc-600 font-light"> | ||
| Featured | ||
| </span> | ||
| </div> | ||
| )} | ||
|
|
||
| <h3 className="text-xl lg:text-2xl font-light text-white mb-3 group-hover:text-zinc-400 transition-colors duration-300 leading-tight"> | ||
| {n.title} | ||
| </h3> | ||
|
|
||
| <p className="text-zinc-500 text-sm leading-relaxed mb-6 line-clamp-2 font-light"> | ||
| {n.excerpt} | ||
| </p> | ||
|
|
||
| <div className="flex items-center gap-2 text-zinc-600 text-xs font-light"> | ||
| <CalendarIcon className="w-3 h-3" /> | ||
| <span> | ||
| {new Date(n.date).toLocaleDateString("en-US", { | ||
| year: "numeric", | ||
| month: "short", | ||
| day: "numeric", | ||
| })} | ||
| </span> | ||
| </div> | ||
| </div> | ||
| </Link> | ||
| </article> | ||
| ); | ||
| } No newline at end of file |
There was a problem hiding this comment.
Fix the critical filename typo.
The filename NewsletterCrad.tsx contains a typo and should be NewsletterCard.tsx (note "Card" not "Crad"). This typo will cause issues:
- Inconsistent naming makes the codebase harder to navigate
- Potential import errors if other files expect "NewsletterCard.tsx"
- Confusing for future developers
Please rename the file from NewsletterCrad.tsx to NewsletterCard.tsx. Verify that all imports referencing this component are updated accordingly.
#!/bin/bash
# Verify imports of this component across the codebase
rg -n --type=ts --type=tsx "from.*NewsletterCrad" -C2
rg -n --type=ts --type=tsx "import.*NewsletterCrad" -C2🤖 Prompt for AI Agents
In apps/web/src/components/newsletters/NewsletterCrad.tsx (lines 1-55) the
filename contains a typo ("Crad" vs "Card"); rename the file to
NewsletterCard.tsx and update all import paths/usages to reference
NewsletterCard (search for NewsletterCrad in the repo and replace
imports/exports/relative paths), ensure any barrel exports or index files are
updated, run TypeScript build or IDE refactor to catch broken references, and
commit the rename so imports are consistent.
| const filtered = NEWSLETTERS | ||
| .sort((a, b) => +new Date(b.date) - +new Date(a.date)) | ||
| .filter((newsletter) => { | ||
| const matchesSearch = query | ||
| ? newsletter.title.toLowerCase().includes(query.toLowerCase()) || | ||
| newsletter.excerpt.toLowerCase().includes(query.toLowerCase()) | ||
| : true; | ||
|
|
||
| const matchesMonth = | ||
| monthFilter === "all" ? true : newsletter.date.startsWith(monthFilter); | ||
|
|
||
| return matchesSearch && matchesMonth; | ||
| }); |
There was a problem hiding this comment.
Avoid mutating NEWSLETTERS with in-place sort
.sort mutates the NEWSLETTERS array in place:
const filtered = NEWSLETTERS
.sort(...)
.filter(...);Since NEWSLETTERS is a shared constant, this introduces hidden global mutation and repeats an O(n log n) sort on every render.
Safer pattern:
- const filtered = NEWSLETTERS
- .sort((a, b) => +new Date(b.date) - +new Date(a.date))
+ const filtered = [...NEWSLETTERS]
+ .sort((a, b) => +new Date(b.date) - +new Date(a.date))
.filter((newsletter) => {
// ...
});This keeps the data immutable and avoids side effects across consumers.
🤖 Prompt for AI Agents
In apps/web/src/components/newsletters/NewsletterList.tsx around lines 12 to 24,
the code calls .sort directly on the shared NEWSLETTERS constant which mutates
it in place; create a non-mutating copy before sorting (e.g., use
NEWSLETTERS.slice() or [...NEWSLETTERS]) and then sort that copy, and to avoid
re-sorting on every render wrap the copy+sort logic in React.useMemo keyed on
NEWSLETTERS (and on query/monthFilter if sorting depends on them) or perform the
sort once outside the component so the original constant remains immutable and
renders are more efficient.
| ".next/types/**/*.ts", | ||
| "types/**/*" | ||
| ], | ||
| , "src/app/(main)/dashboard/newsletters/[slug]" ], |
There was a problem hiding this comment.
Remove or correct the malformed include path.
The include entry "src/app/(main)/dashboard/newsletters/[slug]" is malformed. TypeScript's include expects file paths with extensions or glob patterns, not directory paths. The existing patterns "**/*.ts" and "**/*.tsx" (line 40-41) already cover all TypeScript files in the newsletters directory.
Apply this diff to remove the redundant entry:
"types/**/*"
-, "src/app/(main)/dashboard/newsletters/[slug]" ],
+ ],📝 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.
| , "src/app/(main)/dashboard/newsletters/[slug]" ], | |
| , "types/**/*" ], |
🤖 Prompt for AI Agents
In apps/web/tsconfig.json around line 44, the include entry
"src/app/(main)/dashboard/newsletters/[slug]" is malformed and redundant; remove
this array element so the include list contains only valid glob patterns (e.g.
"**/*.ts", "**/*.tsx") and ensure the JSON array commas are adjusted accordingly
(no trailing comma errors).
1 participant
Fixes
[bounty-to-hire]: create ui for newsletter #155
approach
i focused on creating the best newsletter experience for pro users by implementing a clean, minimal ui that matches the opensox design system. the solution prioritizes readability, easy content management, and optimal performance.
screen recording
opensox.mp4
What in my PR
✅ UI
✅ Optimal Code
✅ Minimal, Quick and Simple
📖 documentation: how to add a blog
step 1: add to data file
edit
data/newsletters.tsand add new object:step 2: content guidelines
Summary by CodeRabbit
New Features
Chores