Feature/blog redesign#384
Open
MistakenPirate wants to merge 14 commits into
Open
Conversation
|
@MistakenPirate 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. |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR migrates the blog from a hardcoded data file to MDX content: adds MDX loader utilities, a ChangesBlog System Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Author
TLDR of the blog redesign so far: done (10/13):
not done yet (3):
haven't added these yet cause the threads are lengthy and had questions questions before i continue:
|
Author
|
#381 referencing this |
Author
|
Demo: output.mp4 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
apps/web/src/lib/blog.ts (1)
23-39: ⚡ Quick winvalidate frontmatter before casting it
both loaders trust
gray-matteroutput withas blogmeta/as blogfrontmatter, so a missingdate, typoedtag, or emptytitlewill only fail later during sort or render. a small runtime schema here would fail the bad file at load time and keep content mistakes from becoming page bugs.Also applies to: 47-57
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/blog.ts` around lines 23 - 39, The getAllPosts mapping trusts gray-matter output and casts to BlogMeta without validation, so add a small runtime schema check in getAllPosts (and the analogous loader around lines 47-57) that verifies required frontmatter fields (e.g., title: non-empty string, date: valid date/string, description: string, author: string, tag/tags present) and their types before casting to BlogMeta; if validation fails, either skip the file and log a clear error (including filename/slug) or throw a descriptive error to fail fast. Locate the frontmatter access in getAllPosts (the { data } = matter(raw) block) and the other loader (the function around lines 47-57 that reads and casts frontmatter) and insert the checks there so only validated objects are returned as BlogMeta.apps/web/src/app/(main)/blog/blog-list.tsx (3)
7-7: ⚡ Quick winrename tags constant to upper snake case
tagsshould follow the constant naming convention.proposed diff
-const tags: BlogTag[] = ["engineering", "startup", "distribution", "misc"]; +const TAGS: BlogTag[] = ["engineering", "startup", "distribution", "misc"]; ... - {tags.map((tag) => ( + {TAGS.map((tag) => (As per coding guidelines "Use UPPER_SNAKE_CASE for constants".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/`(main)/blog/blog-list.tsx at line 7, The constant named tags should be renamed to follow UPPER_SNAKE_CASE: change the declaration const tags: BlogTag[] = ["engineering", "startup", "distribution", "misc"]; to const TAGS: BlogTag[] = ["engineering", "startup", "distribution", "misc"]; then update every usage/reference of tags in this module (e.g., in the blog-list component rendering or imports) to use TAGS so typings (BlogTag[]) and behavior remain unchanged.
9-18: ⚡ Quick winswitch function declarations to const arrow functions
both
formatDateandBlogListshould follow the repo’s function style rule.As per coding guidelines "Use const with arrow functions instead of function declarations (e.g., 'const toggle = () =>')".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/`(main)/blog/blog-list.tsx around lines 9 - 18, The file uses function declarations for formatDate and BlogList; change them to const arrow functions to comply with the repo style: replace "function formatDate(dateStr: string)" with "const formatDate = (dateStr: string) =>" and replace "export default function BlogList({ posts }: { posts: BlogMeta[] })" with "const BlogList = ({ posts }: { posts: BlogMeta[] }) =>" and export default BlogList at the end (or export default const-style), preserving types and the implementation inside each function.
43-43: ⚡ Quick winremove unnecessary jsx comments
these comments describe what the code already makes clear, and they also violate the lowercase-comment rule.
As per coding guidelines "Always use lowercase when writing comments", "Avoid unnecessary comments; code should be self-documenting when possible", and "Use comments to explain 'why', not 'what'".
Also applies to: 70-70
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/`(main)/blog/blog-list.tsx at line 43, Remove the unnecessary JSX comments like {/* Tag filters */} (and the similar one around line ~70) from the BlogList component JSX; these comments state what the markup already shows and violate the lowercase-comment rule—either delete them outright or, if context is needed, replace with a single lowercase, why-focused comment (not a what) adjacent to the relevant JSX in the BlogList component.apps/web/src/app/(main)/blog/page.tsx (2)
10-10: ⚡ Quick winprefer const arrow component declaration
use a const arrow function for consistency with repo rules.
proposed diff
-export default function BlogPage() { +const BlogPage = () => { const posts = getAllPosts(); return <BlogList posts={posts} />; -} +}; + +export default BlogPage;As per coding guidelines "Use const with arrow functions instead of function declarations (e.g., 'const toggle = () =>')".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/`(main)/blog/page.tsx at line 10, Replace the default function declaration "export default function BlogPage()" with a const arrow component declaration named BlogPage (e.g., "const BlogPage = () =>") and export it as the default; locate the BlogPage symbol in the current module and update its declaration to follow the repo rule of using const arrow functions for React components while preserving the component body and existing exports.
1-3: ⚡ Quick winreorder imports to match project import order
this file currently imports local modules before third-party modules.
proposed diff
-import { getAllPosts } from "`@/lib/blog`"; import type { Metadata } from "next"; +import { getAllPosts } from "`@/lib/blog`"; import BlogList from "./blog-list";As per coding guidelines "Organize imports in order: React → third-party → local components → utils → types".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/`(main)/blog/page.tsx around lines 1 - 3, Reorder the imports in page.tsx so they follow the project's import order: React (if you add/use it) → third-party → local components → utils → types; specifically move the third-party import of Metadata (from "next") before the local imports, ensure BlogList (local component) stays in the components group and getAllPosts (from "`@/lib/blog`") is in the utils group, and keep any type imports grouped with other types.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/app/`(main)/blog/[slug]/page.tsx:
- Around line 34-38: The formatted post date in page.tsx uses new
Date(post.frontmatter.date).toLocaleDateString(...) which can shift by timezone
for date-only strings; update the options for the toLocaleDateString call (the
const date formatting) to include timeZone: "UTC" so dates are pinned to UTC (or
alternatively parse the yyyy-mm-dd parts and build a UTC Date via Date.UTC
before formatting) to ensure stable blog post dates across server timezones.
In `@apps/web/src/app/`(main)/blog/blog-list.tsx:
- Around line 45-63: The tag filter buttons (the "All" button and the mapped tag
buttons in blog-list.tsx using activeTag, setActiveTag, and tags) need ARIA
pressed semantics; add aria-pressed to each button and set its boolean value to
true when that button is the active selection (i.e., aria-pressed={activeTag ===
null} for the "All" button and aria-pressed={activeTag === tag} for each tag
button) so assistive tech can detect the toggle state while keeping the existing
onClick behavior and classes.
In `@apps/web/src/content/blog/how-to-build-an-online-presence.mdx`:
- Around line 1-10: Add a draft flag to the frontmatter of this MDX (e.g.,
draft: true) and update the blog loading/filtering logic so drafts are excluded:
modify BlogFrontmatter to include an optional draft boolean, change
getAllPosts(), getAllSlugs(), and getPostBySlug() consumers to filter out items
where draft === true, and ensure route generation and rendering use
generateStaticParams() to omit drafts and add a notFound() check in the [slug]
page to return 404 for draft posts.
In `@apps/web/src/content/blog/how-to-make-your-website-design-consistent.mdx`:
- Around line 34-42: Replace the incorrect singular filename "design-token.ts"
with the correct plural "design-tokens.ts" in the MDX article content (update
every occurrence in the snippet including the sentence about the design system
and the rule in `.cursorrules`), and ensure the quoted coding-guideline
reference matches the repo path "apps/web/src/lib/design-tokens.ts" and
"apps/web/tailwind.config.ts".
In `@apps/web/src/content/blog/why-should-you-open-source-your-project.mdx`:
- Line 10: This MDX post contains only the placeholder text "Coming soon." and
should not be published; either remove the file or mark it as a draft by adding
frontmatter (e.g., draft: true) to why-should-you-open-source-your-project.mdx,
and update the content loader that collects posts to exclude drafts (filter out
items where frontmatter.draft === true) so placeholder posts are not served
publicly.
In `@apps/web/src/lib/blog.ts`:
- Around line 52-55: The code reads MDX into raw, extracts data/content, then
uses marked.parse(content) to produce html which is later rendered with
dangerouslySetInnerHTML; replace this by compiling MDX to sanitized HTML instead
of raw markdown parsing: use an MDX compiler pipeline (e.g., `@mdx-js/mdx` or
remark+rehype with `@mdx-js/mdx`) to compile content to HTML/React and include a
sanitization step (e.g., rehype-sanitize or DOMPurify for server output) before
assigning to html, and ensure files with .mdx are passed to the MDX compiler
(not marked.parse); update the code paths that produce/return html to use the
compiled+sanitized output (replace usage of marked.parse and the html variable
assignment).
---
Nitpick comments:
In `@apps/web/src/app/`(main)/blog/blog-list.tsx:
- Line 7: The constant named tags should be renamed to follow UPPER_SNAKE_CASE:
change the declaration const tags: BlogTag[] = ["engineering", "startup",
"distribution", "misc"]; to const TAGS: BlogTag[] = ["engineering", "startup",
"distribution", "misc"]; then update every usage/reference of tags in this
module (e.g., in the blog-list component rendering or imports) to use TAGS so
typings (BlogTag[]) and behavior remain unchanged.
- Around line 9-18: The file uses function declarations for formatDate and
BlogList; change them to const arrow functions to comply with the repo style:
replace "function formatDate(dateStr: string)" with "const formatDate =
(dateStr: string) =>" and replace "export default function BlogList({ posts }: {
posts: BlogMeta[] })" with "const BlogList = ({ posts }: { posts: BlogMeta[] })
=>" and export default BlogList at the end (or export default const-style),
preserving types and the implementation inside each function.
- Line 43: Remove the unnecessary JSX comments like {/* Tag filters */} (and the
similar one around line ~70) from the BlogList component JSX; these comments
state what the markup already shows and violate the lowercase-comment
rule—either delete them outright or, if context is needed, replace with a single
lowercase, why-focused comment (not a what) adjacent to the relevant JSX in the
BlogList component.
In `@apps/web/src/app/`(main)/blog/page.tsx:
- Line 10: Replace the default function declaration "export default function
BlogPage()" with a const arrow component declaration named BlogPage (e.g.,
"const BlogPage = () =>") and export it as the default; locate the BlogPage
symbol in the current module and update its declaration to follow the repo rule
of using const arrow functions for React components while preserving the
component body and existing exports.
- Around line 1-3: Reorder the imports in page.tsx so they follow the project's
import order: React (if you add/use it) → third-party → local components → utils
→ types; specifically move the third-party import of Metadata (from "next")
before the local imports, ensure BlogList (local component) stays in the
components group and getAllPosts (from "`@/lib/blog`") is in the utils group, and
keep any type imports grouped with other types.
In `@apps/web/src/lib/blog.ts`:
- Around line 23-39: The getAllPosts mapping trusts gray-matter output and casts
to BlogMeta without validation, so add a small runtime schema check in
getAllPosts (and the analogous loader around lines 47-57) that verifies required
frontmatter fields (e.g., title: non-empty string, date: valid date/string,
description: string, author: string, tag/tags present) and their types before
casting to BlogMeta; if validation fails, either skip the file and log a clear
error (including filename/slug) or throw a descriptive error to fail fast.
Locate the frontmatter access in getAllPosts (the { data } = matter(raw) block)
and the other loader (the function around lines 47-57 that reads and casts
frontmatter) and insert the checks there so only validated objects are returned
as BlogMeta.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 687b278b-6f32-42e2-84a0-633343f62f58
📒 Files selected for processing (24)
apps/web/next.config.jsapps/web/src/app/(main)/blog/[slug]/page.tsxapps/web/src/app/(main)/blog/blog-list.tsxapps/web/src/app/(main)/blog/page.tsxapps/web/src/app/(main)/blogs/page.tsxapps/web/src/app/globals.cssapps/web/src/components/blogs/BlogHeader.tsxapps/web/src/components/landing-sections/footer.tsxapps/web/src/content/blog/how-to-be-layoff-proof.mdxapps/web/src/content/blog/how-to-build-an-online-presence.mdxapps/web/src/content/blog/how-to-get-into-gsoc-part-1.mdxapps/web/src/content/blog/how-to-get-into-gsoc-part-2.mdxapps/web/src/content/blog/how-to-get-into-gsoc-part-3.mdxapps/web/src/content/blog/how-to-make-your-website-design-consistent.mdxapps/web/src/content/blog/snapshot-of-my-life-so-far.mdxapps/web/src/content/blog/tiny-habits-that-changed-my-life.mdxapps/web/src/content/blog/ugly-execution-wins.mdxapps/web/src/content/blog/why-opensox-ai-doesnt-do-follow-ups.mdxapps/web/src/content/blog/why-should-you-open-source-your-project.mdxapps/web/src/content/blog/why-you-should-do-open-source.mdxapps/web/src/content/blog/why-you-shouldnt-register-a-company.mdxapps/web/src/data/blogs.tsapps/web/src/lib/blog.tsapps/web/tailwind.config.ts
💤 Files with no reviewable changes (3)
- apps/web/src/components/blogs/BlogHeader.tsx
- apps/web/src/app/(main)/blogs/page.tsx
- apps/web/src/data/blogs.ts
Owner
|
@MistakenPirate hey man! nice changes. can you pls fix the reviews suggested by coderabbit so that i can take a look at the approach then? |
Owner
|
@MistakenPirate also lemme ans ur questions : questions before i continue:
we don't wanna store images from tweets. but we should have a feature to add images by providing a link of it. like most markdown files do.
in case of text, yes. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/src/lib/blog.ts (1)
61-66: ⚡ Quick winDrop the
as anycasts onsanitizeHtml.defaults.
@types/sanitize-htmltypesdefaults.allowedTags(string[]) anddefaults.allowedAttributes, so theanycasts are unnecessary and defeat type checking here.♻️ Proposed refactor
- const html = sanitizeHtml(rawHtml, { - allowedTags: (sanitizeHtml as any).defaults.allowedTags.concat(["img"]), - allowedAttributes: { - ...(sanitizeHtml as any).defaults.allowedAttributes, - img: ["src", "alt", "title", "width", "height", "loading"], - }, - }); + const html = sanitizeHtml(rawHtml, { + allowedTags: sanitizeHtml.defaults.allowedTags.concat(["img"]), + allowedAttributes: { + ...sanitizeHtml.defaults.allowedAttributes, + img: ["src", "alt", "title", "width", "height", "loading"], + }, + });As per coding guidelines: "use proper TypeScript types, avoid
any".Does the `@types/sanitize-html` package (sanitize-html 2.11.0) export a typed `defaults` with allowedTags as string[]?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/lib/blog.ts` around lines 61 - 66, The code is using unnecessary `as any` casts on `sanitizeHtml.defaults` which defeats TypeScript checks; remove the casts and reference the typed properties directly (e.g. use sanitizeHtml.defaults.allowedTags and sanitizeHtml.defaults.allowedAttributes) in the sanitizeHtml options passed when building `html` from `rawHtml`; if the compiler still complains, import or declare the proper type from the package (e.g. the Defaults/Options type from `sanitize-html`) and use it to type any intermediate variables, but do not use `as any` on `sanitizeHtml`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/web/src/lib/blog.ts`:
- Around line 61-66: The code is using unnecessary `as any` casts on
`sanitizeHtml.defaults` which defeats TypeScript checks; remove the casts and
reference the typed properties directly (e.g. use
sanitizeHtml.defaults.allowedTags and sanitizeHtml.defaults.allowedAttributes)
in the sanitizeHtml options passed when building `html` from `rawHtml`; if the
compiler still complains, import or declare the proper type from the package
(e.g. the Defaults/Options type from `sanitize-html`) and use it to type any
intermediate variables, but do not use `as any` on `sanitizeHtml`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 01e00171-89db-49c0-a41a-ad7da764f33c
📒 Files selected for processing (6)
apps/web/src/app/(main)/blog/[slug]/page.tsxapps/web/src/app/(main)/blog/blog-list.tsxapps/web/src/content/blog/how-to-build-an-online-presence.mdxapps/web/src/content/blog/how-to-make-your-website-design-consistent.mdxapps/web/src/content/blog/why-should-you-open-source-your-project.mdxapps/web/src/lib/blog.ts
✅ Files skipped from review due to trivial changes (2)
- apps/web/src/content/blog/why-should-you-open-source-your-project.mdx
- apps/web/src/content/blog/how-to-make-your-website-design-consistent.mdx
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/web/src/content/blog/how-to-build-an-online-presence.mdx
- apps/web/src/app/(main)/blog/[slug]/page.tsx
- apps/web/src/app/(main)/blog/blog-list.tsx
Author
|
@apsinghdev i resolved the coderabbit issues, will be adding the rest of the 3 blogs and then we can merge |
Author
|
@apsinghdev this should do it, added every blog with relevant images |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/content/blog/why-should-you-open-source-your-project.mdx`:
- Line 18: Replace the typo "come one man!" with "come on man!" in the sentence
"do you really think you have a unique idea? come one man! ideas are cheap." —
locate that exact sentence in the blog content and update the phrase "come one"
to "come on" so the line reads "come on man!" preserving original punctuation
and casing.
- Line 22: The sentence contains the unclear fragment "products' on!"—replace it
with a clear phrase like "product's code" (e.g., change 'but they'll copy the
code of my products awesome features? products' on! code can always be
copied...' to 'but they'll copy the code of my product's awesome features?
Product's code can always be copied...'), fix punctuation and capitalization
around "Microsoft" and "Google", and ensure the sentence reads smoothly and
grammatically as part of the paragraph in the file
(apps/web/src/content/blog/why-should-you-open-source-your-project.mdx) by
updating the phrase referenced ("products' on!") to the intended wording.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 19db7885-992f-4110-9c4c-c5f7a60afc19
📒 Files selected for processing (6)
apps/web/src/content/blog/how-to-build-an-online-presence.mdxapps/web/src/content/blog/how-to-get-into-gsoc-part-1.mdxapps/web/src/content/blog/how-to-get-into-gsoc-part-2.mdxapps/web/src/content/blog/how-to-get-into-gsoc-part-3.mdxapps/web/src/content/blog/snapshot-of-my-life-so-far.mdxapps/web/src/content/blog/why-should-you-open-source-your-project.mdx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/content/blog/how-to-get-into-gsoc-part-1.mdx
- apps/web/src/content/blog/how-to-get-into-gsoc-part-2.mdx
|
|
||
| 2. execution is all that matters. and if i have to be specific - iterations (experiments, execution and validated learning) is the key. | ||
|
|
||
| 3. "but they'll copy the code of my products awesome features?" products' on! code can always be copied, even of proprietary software. microsoft created msexcel so did google. |
Contributor
There was a problem hiding this comment.
Clarify unclear phrasing.
The phrase "products' on!" is unclear in this context. Consider revising to improve readability.
💡 Suggested clarification
This might have been intended as "product's code" or similar. Consider:
-3. "but they'll copy the code of my products awesome features?" products' on! code can always be copied, even of proprietary software. microsoft created msexcel so did google.
+3. "but they'll copy the code of my products awesome features?" product's code! code can always be copied, even of proprietary software. microsoft created msexcel so did google.Or adjust as appropriate for the intended meaning.
📝 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.
Suggested change
| 3. "but they'll copy the code of my products awesome features?" products' on! code can always be copied, even of proprietary software. microsoft created msexcel so did google. | |
| 3. "but they'll copy the code of my products awesome features?" product's code! code can always be copied, even of proprietary software. microsoft created msexcel so did google. |
🧰 Tools
🪛 LanguageTool
[grammar] ~22-~22: Ensure spelling is correct
Context: ...proprietary software. microsoft created msexcel so did google. 4. the real differentia...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/content/blog/why-should-you-open-source-your-project.mdx` at
line 22, The sentence contains the unclear fragment "products' on!"—replace it
with a clear phrase like "product's code" (e.g., change 'but they'll copy the
code of my products awesome features? products' on! code can always be
copied...' to 'but they'll copy the code of my product's awesome features?
Product's code can always be copied...'), fix punctuation and capitalization
around "Microsoft" and "Google", and ensure the sentence reads smoothly and
grammatically as part of the paragraph in the file
(apps/web/src/content/blog/why-should-you-open-source-your-project.mdx) by
updating the phrase referenced ("products' on!") to the intended wording.
Author
|
something i thought would be cool: video: output.mp4 |
Owner
|
@MistakenPirate damn! really nice changes! lemme test locally and it all good to go! |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/app/`(main)/blog/blog-theme.tsx:
- Around line 16-23: getSavedTheme and the useState lazy initializer are reading
localStorage during hydration, causing a mismatch between server-rendered "dark"
and client-first render when a saved theme exists; change BlogThemeSelector to
initialize theme state to a stable default (e.g., "dark") synchronously and move
the localStorage read into a useEffect that runs after mount: in the effect,
read localStorage.getItem("blog-theme"), validate against validThemes, and call
setTheme(...) only if valid; keep getSavedTheme (or remove it) but avoid calling
it during initial render so the server never reads window/localStorage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d9454463-1e52-490a-96ae-f5763fb0b8d3
📒 Files selected for processing (5)
apps/web/src/app/(main)/blog/[slug]/page.tsxapps/web/src/app/(main)/blog/blog-list.tsxapps/web/src/app/(main)/blog/blog-theme.tsxapps/web/src/app/globals.cssapps/web/src/content/blog/why-should-you-open-source-your-project.mdx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/content/blog/why-should-you-open-source-your-project.mdx
- apps/web/src/app/(main)/blog/[slug]/page.tsx
Comment on lines
+16
to
+23
| function getSavedTheme(): ThemeId { | ||
| if (typeof window === "undefined") return "dark"; | ||
| const saved = localStorage.getItem("blog-theme"); | ||
| return saved && validThemes.has(saved as ThemeId) ? (saved as ThemeId) : "dark"; | ||
| } | ||
|
|
||
| export default function BlogThemeSelector() { | ||
| const [theme, setTheme] = useState<ThemeId>(getSavedTheme); |
Contributor
There was a problem hiding this comment.
Hydration mismatch: reading localStorage in the useState initializer.
BlogThemeSelector is a client component, so it is also server-rendered. On the server getSavedTheme() returns "dark" (window undefined), but during client hydration the lazy initializer reruns and reads localStorage, so a saved theme like "sepia" makes the first client render produce <select value="sepia"> while the server HTML has value="dark". This triggers a hydration mismatch on the controlled <select>.
Initialize to a stable default and resolve the saved theme after mount inside an effect.
🐛 Proposed fix to defer storage read to the client
export default function BlogThemeSelector() {
- const [theme, setTheme] = useState<ThemeId>(getSavedTheme);
+ const [theme, setTheme] = useState<ThemeId>("dark");
+ const [mounted, setMounted] = useState(false);
+
+ // resolve persisted theme only on the client to avoid hydration mismatch
+ useEffect(() => {
+ setTheme(getSavedTheme());
+ setMounted(true);
+ }, []);
useEffect(() => {
+ if (!mounted) return;
document.documentElement.setAttribute("data-blog-theme", theme);
localStorage.setItem("blog-theme", theme);
return () => {
document.documentElement.removeAttribute("data-blog-theme");
};
- }, [theme]);
+ }, [theme, mounted]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/app/`(main)/blog/blog-theme.tsx around lines 16 - 23,
getSavedTheme and the useState lazy initializer are reading localStorage during
hydration, causing a mismatch between server-rendered "dark" and client-first
render when a saved theme exists; change BlogThemeSelector to initialize theme
state to a stable default (e.g., "dark") synchronously and move the localStorage
read into a useEffect that runs after mount: in the effect, read
localStorage.getItem("blog-theme"), validate against validThemes, and call
setTheme(...) only if valid; keep getSavedTheme (or remove it) but avoid calling
it during initial render so the server never reads window/localStorage.
Owner
|
@MistakenPirate a few change requests
|
Author
|
Nope, no limits. Write as much as you want, the blog reads the full file and renders everything. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/components/ui/Client-tweet.tsx (1)
208-245:⚠️ Potential issue | 🟠 Major | ⚡ Quick winmake the card-thumbnail fallback reachable.
line 211 returns before the
tweet.card.binding_values.thumbnail_image_largebranch below can run, so card-only tweets now render no media at all. include the thumbnail in the guard, or compute it first and only return when all three sources are absent.proposed fix
export const TweetMedia = ({ tweet }: { tweet: EnrichedTweet }) => { const photos = tweet.photos ?? []; const videoVariant = tweet.video?.variants?.[0]; + // `@ts-expect-error` expected + const cardThumbnail = tweet?.card?.binding_values?.thumbnail_image_large?.image_value?.url; - if (!videoVariant && photos.length === 0) return null; + if (!videoVariant && photos.length === 0 && !cardThumbnail) return null; return ( <div className="flex flex-1 items-center justify-center"> {videoVariant && ( <video poster={tweet.video?.poster} @@ - {!videoVariant && - photos.length === 0 && - // `@ts-expect-error` expected - tweet?.card?.binding_values?.thumbnail_image_large?.image_value.url && ( + {!videoVariant && photos.length === 0 && cardThumbnail && ( <img - src={ - // `@ts-expect-error` expected - tweet.card.binding_values.thumbnail_image_large.image_value.url - } + src={cardThumbnail} className="h-64 rounded-xl border object-cover shadow-sm" alt={tweet.text} /> )} </div> ); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/ui/Client-tweet.tsx` around lines 208 - 245, The early return prevents the card-thumbnail fallback from ever rendering; compute a thumbnailUrl first (e.g. const thumbnailUrl = tweet?.card?.binding_values?.thumbnail_image_large?.image_value?.url) and replace the current guard if (!videoVariant && photos.length === 0) return null with a check that returns only when videoVariant, photos.length, and thumbnailUrl are all absent; then use thumbnailUrl in the JSX branch instead of the long optional chain and remove the `@ts-expect-error`.
🧹 Nitpick comments (5)
apps/web/src/app/(main)/(landing)/pitch/page.tsx (3)
291-301: ⚡ Quick winTypeScript won't narrow
premiumPlanIdthrough the intermediate boolean.
planIdOkvalidates thatpremiumPlanIdis a non-empty string, but TypeScript doesn't track this through a separate boolean variable. IfPaymentFlowexpectsplanId: string, this could cause a type error sincepremiumPlanIdremains typed asstring | undefined.♻️ Inline the check or use a type assertion
Option 1 - Use non-null assertion (if confident in the check):
{planIdOk ? ( <div className="w-full max-w-[180px]"> <PaymentFlow - planId={premiumPlanId} + planId={premiumPlanId!} planName="Opensox Pro"Option 2 - Inline the check for proper narrowing:
-const planIdOk = - typeof premiumPlanId === "string" && premiumPlanId.trim().length > 0; ... -{planIdOk ? ( +{typeof premiumPlanId === "string" && premiumPlanId.trim().length > 0 ? ( <div className="w-full max-w-[180px]"> <PaymentFlow planId={premiumPlanId}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/`(main)/(landing)/pitch/page.tsx around lines 291 - 301, The component currently uses planIdOk to guard rendering but TypeScript doesn't narrow premiumPlanId's type for the PaymentFlow prop; update the render to ensure PaymentFlow receives a string by either inlining the check (e.g., use premiumPlanId && <PaymentFlow planId={premiumPlanId} ... />) or applying a non-null assertion/explicit cast to planId when passing to PaymentFlow (planId={premiumPlanId!} or planId={String(premiumPlanId)}), referencing planIdOk, premiumPlanId and the PaymentFlow prop to locate the change.
25-27: ⚡ Quick winUse design token for heading font instead of hardcoded font-family.
Multiple headings use the arbitrary value
[font-family:Helvetica,Arial,sans-serif]directly. Per the design system guidelines and PR discussion requesting Helvetica-Semibold be added to design tokens, these should reference a token class (e.g.,font-heading) once defined indesign-tokens.tsandtailwind.config.ts.This will ensure consistency across pages and make future font changes easier to manage.
♻️ Once the heading font token is defined, update all headings
-<span className="font-semibold [font-family:Helvetica,Arial,sans-serif]"> +<span className="font-semibold font-heading"> opensox manifesto </span>-<h2 className="text-2xl lg:text-3xl font-bold text-brand-purple-light tracking-tight leading-tight [font-family:Helvetica,Arial,sans-serif]"> +<h2 className="text-2xl lg:text-3xl font-bold text-brand-purple-light tracking-tight leading-tight font-heading">Apply similar changes to all
<h2>elements on lines 41, 59, 84, 119, 161, 225, 252, and 287.As per coding guidelines: "Always follow the design system defined in
apps/web/src/lib/design-tokens.tsandapps/web/tailwind.config.ts"Also applies to: 41-43, 59-61, 84-86, 119-121, 161-164, 225-228, 252-254, 287-289
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/`(main)/(landing)/pitch/page.tsx around lines 25 - 27, Replace the hardcoded arbitrary font-family class used on the heading span and all H2 headings (currently using className "font-semibold [font-family:Helvetica,Arial,sans-serif]") with the new design-token class (for example "font-heading font-semibold") so they reference the centralized token instead of an inline font-family; update the span and every <h2> instance indicated in the review (the same pattern appears on the other headings referenced) to use the token class name (e.g., font-heading) and keep existing weight classes like font-semibold intact so styling remains consistent.
12-12: 💤 Low valueAdd type annotation to component function.
Per guidelines, const arrow function components should have explicit type definitions.
♻️ Proposed fix
-const Pitch = () => { +const Pitch: React.FC = () => {As per coding guidelines: "Define a type when defining const functions"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/`(main)/(landing)/pitch/page.tsx at line 12, The Pitch component is missing an explicit type annotation; update the declaration to add a React function-component type (e.g. declare it as const Pitch: React.FC = () => { ... } or const Pitch: FC = () => { ... }) and add the corresponding import (import React from 'react' or import type { FC } from 'react') so the component has an explicit type definition.apps/web/src/app/(main)/blog/blog-list.tsx (1)
30-30: ⚡ Quick winalign the page padding with the design system.
px-6 py-20breaks the spacing contract used elsewhere, so this page will sit off-rhythm against the rest of the app.proposed diff
- <div className="max-w-4xl mx-auto px-6 py-20"> + <div className="max-w-4xl mx-auto p-4 lg:p-[60px]">As per coding guidelines "Follow Tailwind's spacing scale (0.25rem increments); for section padding use mobile
p-4(1rem) and desktopp-[60px]".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/`(main)/blog/blog-list.tsx at line 30, The container div currently uses "px-6 py-20" which violates the spacing contract; replace those classes with the design-system responsive padding (mobile p-4 and desktop p-[60px]) on the same element (the top-level container div in this file) so the className uses p-4 and a desktop breakpoint like md:p-[60px] (keep existing max-w-4xl mx-auto intact).apps/web/src/app/(main)/blog/[slug]/page.tsx (1)
45-45: ⚡ Quick winalign the post container padding with the design system.
the article wrapper uses custom spacing here too, so the post page will drift from the shared section rhythm.
proposed diff
- <article className="max-w-4xl mx-auto px-6 py-20"> + <article className="max-w-4xl mx-auto p-4 lg:p-[60px]">As per coding guidelines "Follow Tailwind's spacing scale (0.25rem increments); for section padding use mobile
p-4(1rem) and desktopp-[60px]".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/`(main)/blog/[slug]/page.tsx at line 45, The article wrapper in page.tsx currently uses custom spacing ("px-6 py-20") that drifts from the design system; replace those with the shared section padding classes so it follows Tailwind's spacing scale—update the <article> element's className (the one currently "max-w-4xl mx-auto px-6 py-20") to keep "max-w-4xl mx-auto" but use "p-4 md:p-[60px]" (mobile p-4, desktop p-[60px]) instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/app/`(main)/blog/blog-socials.tsx:
- Around line 46-55: Update the icon-only Link so screen readers get clear
context and the decorative icon is hidden: change the Link's aria-label to
include the new-tab hint (e.g. aria-label={`${s.label} (opens in a new tab)`})
and add a title attribute for sighted users; ensure the icon element (s.icon) is
rendered with aria-hidden="true" and, if it's an SVG, focusable="false" so
assistive tech ignores it. Keep the existing href/rel/target behavior and do not
remove native keyboard focus handling on the Link.
---
Outside diff comments:
In `@apps/web/src/components/ui/Client-tweet.tsx`:
- Around line 208-245: The early return prevents the card-thumbnail fallback
from ever rendering; compute a thumbnailUrl first (e.g. const thumbnailUrl =
tweet?.card?.binding_values?.thumbnail_image_large?.image_value?.url) and
replace the current guard if (!videoVariant && photos.length === 0) return null
with a check that returns only when videoVariant, photos.length, and
thumbnailUrl are all absent; then use thumbnailUrl in the JSX branch instead of
the long optional chain and remove the `@ts-expect-error`.
---
Nitpick comments:
In `@apps/web/src/app/`(main)/(landing)/pitch/page.tsx:
- Around line 291-301: The component currently uses planIdOk to guard rendering
but TypeScript doesn't narrow premiumPlanId's type for the PaymentFlow prop;
update the render to ensure PaymentFlow receives a string by either inlining the
check (e.g., use premiumPlanId && <PaymentFlow planId={premiumPlanId} ... />) or
applying a non-null assertion/explicit cast to planId when passing to
PaymentFlow (planId={premiumPlanId!} or planId={String(premiumPlanId)}),
referencing planIdOk, premiumPlanId and the PaymentFlow prop to locate the
change.
- Around line 25-27: Replace the hardcoded arbitrary font-family class used on
the heading span and all H2 headings (currently using className "font-semibold
[font-family:Helvetica,Arial,sans-serif]") with the new design-token class (for
example "font-heading font-semibold") so they reference the centralized token
instead of an inline font-family; update the span and every <h2> instance
indicated in the review (the same pattern appears on the other headings
referenced) to use the token class name (e.g., font-heading) and keep existing
weight classes like font-semibold intact so styling remains consistent.
- Line 12: The Pitch component is missing an explicit type annotation; update
the declaration to add a React function-component type (e.g. declare it as const
Pitch: React.FC = () => { ... } or const Pitch: FC = () => { ... }) and add the
corresponding import (import React from 'react' or import type { FC } from
'react') so the component has an explicit type definition.
In `@apps/web/src/app/`(main)/blog/[slug]/page.tsx:
- Line 45: The article wrapper in page.tsx currently uses custom spacing ("px-6
py-20") that drifts from the design system; replace those with the shared
section padding classes so it follows Tailwind's spacing scale—update the
<article> element's className (the one currently "max-w-4xl mx-auto px-6 py-20")
to keep "max-w-4xl mx-auto" but use "p-4 md:p-[60px]" (mobile p-4, desktop
p-[60px]) instead.
In `@apps/web/src/app/`(main)/blog/blog-list.tsx:
- Line 30: The container div currently uses "px-6 py-20" which violates the
spacing contract; replace those classes with the design-system responsive
padding (mobile p-4 and desktop p-[60px]) on the same element (the top-level
container div in this file) so the className uses p-4 and a desktop breakpoint
like md:p-[60px] (keep existing max-w-4xl mx-auto intact).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 29ab2fea-385e-406e-8f85-15e641c72d23
📒 Files selected for processing (12)
apps/web/package.jsonapps/web/src/app/(main)/(landing)/pitch/page.tsxapps/web/src/app/(main)/blog/[slug]/page.tsxapps/web/src/app/(main)/blog/blog-list.tsxapps/web/src/app/(main)/blog/blog-socials.tsxapps/web/src/app/globals.cssapps/web/src/components/payment/PaymentFlow.tsxapps/web/src/components/ui/Client-tweet.tsxapps/web/src/components/ui/client-side-tweet.tsxapps/web/src/components/ui/new-tweet-grid.tsxapps/web/src/components/ui/tweet-grid.tsxapps/web/src/lib/design-tokens.ts
✅ Files skipped from review due to trivial changes (1)
- apps/web/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/app/globals.css
Author
|
All done and good to go from my end. |
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Style
Refactor