Merged
Conversation
Contributor
WalkthroughAdds a typed, React/TypeScript navigation menu suite (desktop + mobile), exports a public Link type with icon fields, enhances StarCount typing/formatting, and small blog and config tweaks (unoptimized images, tag links, whitespace reformats). Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
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 |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
![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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/blog/src/components/PostCard.tsx (1)
100-107:⚠️ Potential issue | 🟠 MajorKeep non-featured card images lazy.
Line 106 makes every
PostCardimage eager.BlogGridrenders multiple cards per page, and the non-featured image wrapper ishidden sm:block, so this eagerly fetches offscreen or mobile-hidden assets unnecessarily. You already gatepriorityon Line 107 for the featured card, so this blanket eager load is inconsistent. Combined withimages.unoptimizedenabled inapps/blog/next.config.mjs(line 229), unoptimized images load at full resolution regardless, making premature eager loading especially costly.Suggested fix
- loading="eager" + loading={isFeatured ? "eager" : undefined}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/blog/src/components/PostCard.tsx` around lines 100 - 107, The Image component currently forces eager loading for all PostCard images; change it so only featured cards load eagerly by making the loading prop conditional on isFeatured (e.g., loading = "eager" when isFeatured, otherwise "lazy") while retaining the existing priority={isFeatured} logic; update the Image usage (the JSX with Image, src via withBlogBasePathForImageSrc, alt, fill, sizes, className/imageClassName) to use this conditional loading to avoid fetching offscreen or hidden non-featured images.
🧹 Nitpick comments (1)
\ (1)
282-286: Use the shared navigation link type through the mobile menu flow.
packages/ui/src/components/web-navigation.tsx:21-41already defines theLinkshape passed intoNavigationMobileMenu, but this file widens it back toanyandMenuNavigationItemrequiresicon/descfields that that shared type does not guarantee. Reusing the shared type here will let TypeScript catch missing menu metadata before it renders blank icons or descriptions.Also applies to: 310-339
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @\ around lines 282 - 286, The MenuNavigationItem component is widening the link prop and requiring icon/desc fields that the shared navigation Link type does not guarantee; import and use the shared Link type (the exported type from the web-navigation component used by NavigationMobileMenu) as the prop type for MenuNavigationItem (and the other similar component at the 310-339 range), remove the forced icon/desc requirements, and adjust render logic to treat icon and desc as optional (guard before rendering) so TypeScript will enforce the correct shape and prevent blank icons/descriptions at runtime.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @\:
- Around line 245-252: The Discord link renders two icons because the old Font
Awesome element is still present; remove the <i className="fa-brands
fa-discord"> element inside the NavigationMenuLink so only the inline SVG
remains; locate the NavigationMenuItem/NavigationMenuLink block and delete that
<i> tag (or replace it with the SVG if you intended to switch), ensuring the SVG
at line with viewBox="0 0 24 24" is the sole icon.
- Around line 354-367: The mobile CTA buttons ("Login" and "Get started")
currently render inert because neither NavigationMenuItem nor Button include
navigation handlers; wire them to their destinations by adding proper navigation
props to the relevant components (e.g., pass an href or onClick to the Button or
NavigationMenuItem that triggers routing to the login route for the "Login"
Button and to the signup/get-started route for the "Get started" Button), using
the Button and NavigationMenuItem instances in this block so the CTAs perform
navigation through your app router (or call a navigation helper like
navigate()/router.push()).
In `@apps/blog/src/app/`(blog)/[slug]/page.tsx:
- Around line 188-205: In page.tsx where page.data?.tags.map renders Link with
href={`/?tag=${tag}`}, stop interpolating the raw tag into the query string and
instead pass the tag as a query parameter (use an object href with pathname '/'
and query { tag } or otherwise encode via URLSearchParams/encodeURIComponent) so
tags containing &, #, = or spaces are correctly encoded and round-trippable;
update the Link invocation that wraps Badge (and remove the redundant key on
Badge if present) to use the object/query approach referencing the Link,
page.data?.tags, and formatTag locations.
In `@apps/blog/src/app/api/search/route.ts`:
- Around line 29-37: The transform function is replacing the incoming relevance
ranking by sorting solely on generated_metadata.date; instead, preserve the
original relevance order and apply date only as a secondary tiebreaker (or
expose a separate "sortByDate" mode). Update transform (the results parameter
and how you sort) to first consult the incoming relevance (e.g., a hit.score or
the original results order) and only use generated_metadata.date (from
generated_metadata cast to GeneratedMetadata) when relevance is equal or within
a chosen tie threshold; alternatively implement an explicit mode flag that
bypasses relevance and sorts strictly by date if requested.
In `@packages/ui/src/components/star-count.tsx`:
- Around line 21-29: The component currently hides itself when not loading and
starCount <= 0, which causes it to disappear on fetch errors from
useStarCount(); update the hook to expose an error flag (e.g., isError) or
return null for starCount on error, then change the isHidden logic in StarCount
to only hide when not loading AND starCount is a valid positive number (e.g.,
const isHidden = !isLoading && !isError && starCount <= 0 or const isHidden =
!isLoading && (starCount == null || starCount <= 0) depending on your hook
change) so the fallback UI remains visible on error; adjust
getStarParts/formatting to handle null/undefined if you choose that route.
---
Outside diff comments:
In `@apps/blog/src/components/PostCard.tsx`:
- Around line 100-107: The Image component currently forces eager loading for
all PostCard images; change it so only featured cards load eagerly by making the
loading prop conditional on isFeatured (e.g., loading = "eager" when isFeatured,
otherwise "lazy") while retaining the existing priority={isFeatured} logic;
update the Image usage (the JSX with Image, src via withBlogBasePathForImageSrc,
alt, fill, sizes, className/imageClassName) to use this conditional loading to
avoid fetching offscreen or hidden non-featured images.
---
Nitpick comments:
In @\:
- Around line 282-286: The MenuNavigationItem component is widening the link
prop and requiring icon/desc fields that the shared navigation Link type does
not guarantee; import and use the shared Link type (the exported type from the
web-navigation component used by NavigationMobileMenu) as the prop type for
MenuNavigationItem (and the other similar component at the 310-339 range),
remove the forced icon/desc requirements, and adjust render logic to treat icon
and desc as optional (guard before rendering) so TypeScript will enforce the
correct shape and prevent blank icons/descriptions at runtime.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5334e208-7484-4b1e-82a3-93dc2b78ecf6
📒 Files selected for processing (8)
\apps/blog/next.config.mjsapps/blog/src/app/(blog)/[slug]/page.tsxapps/blog/src/app/api/search/route.tsapps/blog/src/components/PostCard.tsxpackages/ui/src/components/navigation-menu.tsxpackages/ui/src/components/star-count.tsxpackages/ui/src/styles/globals.css
Resolve the actionable review findings by restoring conditional image eager loading, preserving search relevance ordering, encoding tag query links, and tightening mobile navigation behavior and shared link typing. Made-with: Cursor
Member
Author
|
Addressed the CodeRabbit actionable feedback in commit Updated:
The Discord double-icon finding was already resolved in the current branch state (only the SVG remains). |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ui/src/components/navigation-menu.tsx`:
- Around line 239-241: The GitHub SVG in navigation-menu.tsx is hardcoded with
width="200" height="200" which breaks layout; replace the fixed dimensions on
the SVG element (the inline <svg> in the social row) by removing width/height
attributes and instead rely on the viewBox plus CSS sizing — e.g. add a
className (e.g. "h-5 w-5" or similar tailwind/stylesheet class) or set
width/height to relative units like "1em" so the icon inherits font-size and
fits the nav; ensure fill="currentColor" remains so it respects text color.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 90a1b517-3a5a-4785-9982-74d589674278
📒 Files selected for processing (5)
apps/blog/src/app/(blog)/[slug]/page.tsxapps/blog/src/app/api/search/route.tspackages/ui/src/components/navigation-menu.tsxpackages/ui/src/components/star-count.tsxpackages/ui/src/components/web-navigation.tsx
💤 Files with no reviewable changes (1)
- apps/blog/src/app/api/search/route.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/blog/src/app/(blog)/[slug]/page.tsx
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/ui/src/components/navigation-menu.tsx (1)
236-249: Accessibility inconsistency between social icons.The Discord SVG includes a
<title>Discord</title>element for screen readers, but the GitHub SVG lacks equivalent accessible labeling. For consistency and accessibility, consider adding either a<title>element oraria-labelto the GitHub icon.Also, the icon sizes differ (
size-4for GitHub vssize-5for Discord). If intentional for visual balance, that's fine—but worth confirming this was deliberate.♿ Suggested accessibility fix for GitHub icon
- <svg xmlns="http://www.w3.org/2000/svg" className="size-4" viewBox="0 0 32 32" fill="currentColor"><path fill="currentColor" d="M16 .396c-8.839 0-16 7.167-16 16c0 7.073 4.584 13.068 10.937 15.183c.803.151 1.093-.344 1.093-.772c0-.38-.009-1.385-.015-2.719c-4.453.964-5.391-2.151-5.391-2.151c-.729-1.844-1.781-2.339-1.781-2.339c-1.448-.989.115-.968.115-.968c1.604.109 2.448 1.645 2.448 1.645c1.427 2.448 3.744 1.74 4.661 1.328c.14-1.031.557-1.74 1.011-2.135c-3.552-.401-7.287-1.776-7.287-7.907c0-1.751.62-3.177 1.645-4.297c-.177-.401-.719-2.031.141-4.235c0 0 1.339-.427 4.4 1.641a15.436 15.436 0 0 1 4-.541c1.36.009 2.719.187 4 .541c3.043-2.068 4.381-1.641 4.381-1.641c.859 2.204.317 3.833.161 4.235c1.015 1.12 1.635 2.547 1.635 4.297c0 6.145-3.74 7.5-7.296 7.891c.556.479 1.077 1.464 1.077 2.959c0 2.14-.02 3.864-.02 4.385c0 .416.28.916 1.104.755c6.4-2.093 10.979-8.093 10.979-15.156c0-8.833-7.161-16-16-16z" /></svg> + <svg xmlns="http://www.w3.org/2000/svg" className="size-4" viewBox="0 0 32 32" fill="currentColor" aria-label="GitHub"><path fill="currentColor" d="M16 .396c-8.839 0-16 7.167-16 16c0 7.073 4.584 13.068 10.937 15.183c.803.151 1.093-.344 1.093-.772c0-.38-.009-1.385-.015-2.719c-4.453.964-5.391-2.151-5.391-2.151c-.729-1.844-1.781-2.339-1.781-2.339c-1.448-.989.115-.968.115-.968c1.604.109 2.448 1.645 2.448 1.645c1.427 2.448 3.744 1.74 4.661 1.328c.14-1.031.557-1.74 1.011-2.135c-3.552-.401-7.287-1.776-7.287-7.907c0-1.751.62-3.177 1.645-4.297c-.177-.401-.719-2.031.141-4.235c0 0 1.339-.427 4.4 1.641a15.436 15.436 0 0 1 4-.541c1.36.009 2.719.187 4 .541c3.043-2.068 4.381-1.641 4.381-1.641c.859 2.204.317 3.833.161 4.235c1.015 1.12 1.635 2.547 1.635 4.297c0 6.145-3.74 7.5-7.296 7.891c.556.479 1.077 1.464 1.077 2.959c0 2.14-.02 3.864-.02 4.385c0 .416.28.916 1.104.755c6.4-2.093 10.979-8.093 10.979-15.156c0-8.833-7.161-16-16-16z" /></svg>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/navigation-menu.tsx` around lines 236 - 249, The GitHub SVG inside the NavigationMenuItem/NavigationMenuLink lacks accessible labeling and uses a different size class than the Discord icon; add an accessible label (e.g., include a <title>GitHub</title> element inside the GitHub SVG or add aria-label="GitHub" and role="img" on the SVG) so screen readers can announce it, and confirm/standardize the size class (change className="size-4" to match the Discord className="size-5" if the visual size should match) to keep icon sizing consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/ui/src/components/navigation-menu.tsx`:
- Around line 236-249: The GitHub SVG inside the
NavigationMenuItem/NavigationMenuLink lacks accessible labeling and uses a
different size class than the Discord icon; add an accessible label (e.g.,
include a <title>GitHub</title> element inside the GitHub SVG or add
aria-label="GitHub" and role="img" on the SVG) so screen readers can announce
it, and confirm/standardize the size class (change className="size-4" to match
the Discord className="size-5" if the visual size should match) to keep icon
sizing consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 47f05408-f23a-4479-bd6f-496d298ade47
📒 Files selected for processing (1)
packages/ui/src/components/navigation-menu.tsx
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
1 participant
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
Improvements