Open
Conversation
Contributor
WalkthroughMultiple UI and behavior updates across the blog app and shared UI package: the blog page Suspense fallback now renders real tag links and post previews; layout navigation URLs and ThemeProvider storage key were adjusted; newsletter, theme-provider, navigation, BlogGrid, PostCard, and a CSS gradient were modified. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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: 1
🧹 Nitpick comments (1)
apps/blog/src/app/(blog)/page.tsx (1)
98-141: Consider sharing the fallback preview markup with the loaded grid.
page.tsxnow owns a second tag-pill/post-preview rendering path besideBlogGrid. That will be easy to drift the next time the blog card markup or styling changes. Extracting the preview pieces into shared helpers/components would keep the streamed and settled states aligned.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/blog/src/app/`(blog)/page.tsx around lines 98 - 141, The fallback markup duplicates BlogGrid's tag-pill and post-preview rendering (see staticFallbackTags, staticFallbackItems, formatTag, formatDate and BlogGrid) — extract shared components/helpers (e.g., TagPill and PostPreview) that encapsulate the pill link and article card markup + props (tag, post) and replace the inline fallback mapping with those components, then update BlogGrid to use the same TagPill and PostPreview so both streamed (fallback) and settled render paths share identical markup and styling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/blog/src/app/`(blog)/page.tsx:
- Around line 85-86: The fallback currently uses staticFallbackItems and
staticFallbackTags and ignores the active tag/searchParams, causing unrelated
posts to render; update BlogHome (and its fallback logic around
staticFallbackItems/staticFallbackTags) to accept the active tag (from
searchParams.tag) as a prop, filter items and uniqueTags by that tag before
slicing (so staticFallbackItems = items filtered by tag then slice(0,12),
staticFallbackTags = uniqueTags filtered/ordered relative to the active tag then
slice(0,10)), and pass the active tag into BlogGrid so the fallback can also
apply the selected pill styling for the active tag.
---
Nitpick comments:
In `@apps/blog/src/app/`(blog)/page.tsx:
- Around line 98-141: The fallback markup duplicates BlogGrid's tag-pill and
post-preview rendering (see staticFallbackTags, staticFallbackItems, formatTag,
formatDate and BlogGrid) — extract shared components/helpers (e.g., TagPill and
PostPreview) that encapsulate the pill link and article card markup + props
(tag, post) and replace the inline fallback mapping with those components, then
update BlogGrid to use the same TagPill and PostPreview so both streamed
(fallback) and settled render paths share identical markup and styling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c576ea9e-b105-45fd-ab95-700db0530435
📒 Files selected for processing (1)
apps/blog/src/app/(blog)/page.tsx
Comment on lines
+85
to
+86
| const staticFallbackItems = items.slice(0, 12); | ||
| const staticFallbackTags = uniqueTags.slice(0, 10); |
Contributor
There was a problem hiding this comment.
Make the fallback respect the active tag filter.
This fallback always renders the first 12 unfiltered posts, and BlogHome never reads searchParams. On /blog?tag=..., users will briefly see unrelated content until BlogGrid resolves, which makes the new SSR state misleading.
Suggested direction
+ // derive `activeTag` from the page's search params before building fallback data
+ const fallbackSourceItems = activeTag
+ ? items.filter((item) => item.tags?.includes(activeTag))
+ : items;
- const staticFallbackItems = items.slice(0, 12);
+ const staticFallbackItems = fallbackSourceItems.slice(0, 12);If you thread the active tag into this component, you can also reflect the selected pill styling in the fallback.
Also applies to: 97-142
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/blog/src/app/`(blog)/page.tsx around lines 85 - 86, The fallback
currently uses staticFallbackItems and staticFallbackTags and ignores the active
tag/searchParams, causing unrelated posts to render; update BlogHome (and its
fallback logic around staticFallbackItems/staticFallbackTags) to accept the
active tag (from searchParams.tag) as a prop, filter items and uniqueTags by
that tag before slicing (so staticFallbackItems = items filtered by tag then
slice(0,12), staticFallbackTags = uniqueTags filtered/ordered relative to the
active tag then slice(0,10)), and pass the active tag into BlogGrid so the
fallback can also apply the selected pill styling for the active tag.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/ui/src/components/newsletter.tsx (1)
103-107: Use assertive live region for errors.Error feedback should be announced immediately;
role="status"with polite live mode can delay it.Proposed accessibility tweak
- <p - className={cn("text-sm self-start", statusMessage.className)} - role="status" - aria-live="polite" - > + <p + className={cn("text-sm self-start", statusMessage.className)} + role={error ? "alert" : "status"} + aria-live={error ? "assertive" : "polite"} + > {statusMessage.text} </p>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/newsletter.tsx` around lines 103 - 107, The error message paragraph in newsletter.tsx currently uses role="status" and aria-live="polite", which can delay announcing errors; update the component that renders the <p> (the element referencing statusMessage and statusMessage.className) so that when the message represents an error it uses an assertive live region (e.g., role="alert" or aria-live="assertive") instead of polite; implement a conditional around statusMessage (or a statusMessage.type/severity check) to switch the role/aria-live only for errors while keeping polite for non-error notices.
🤖 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/theme-provider.tsx`:
- Around line 66-72: getInitialTheme currently falls back to the hardcoded
string "system" which ignores the configured defaultTheme; change the fallback
to use the provided defaultTheme instead. Update the return in getInitialTheme
(and the undefined-window early return) so it returns defaultTheme when no
stored value exists (e.g., return stored ?? defaultTheme) and keep existing
system-handling elsewhere if defaultTheme === "system"; reference
getInitialTheme, defaultTheme, storageKey, and toTheme to locate the code to
change.
---
Nitpick comments:
In `@packages/ui/src/components/newsletter.tsx`:
- Around line 103-107: The error message paragraph in newsletter.tsx currently
uses role="status" and aria-live="polite", which can delay announcing errors;
update the component that renders the <p> (the element referencing statusMessage
and statusMessage.className) so that when the message represents an error it
uses an assertive live region (e.g., role="alert" or aria-live="assertive")
instead of polite; implement a conditional around statusMessage (or a
statusMessage.type/severity check) to switch the role/aria-live only for errors
while keeping polite for non-error notices.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 830ef817-ce6f-4211-80ad-10e038d9f5fb
📒 Files selected for processing (7)
apps/blog/src/app/(blog)/layout.tsxapps/blog/src/app/global.cssapps/blog/src/components/BlogGrid.tsxapps/blog/src/components/PostCard.tsxpackages/ui/src/components/newsletter.tsxpackages/ui/src/components/theme-provider.tsxpackages/ui/src/components/web-navigation.tsx
Comment on lines
+66
to
+72
| const getInitialTheme = (): Theme => { | ||
| if (typeof window === "undefined") return defaultTheme; | ||
|
|
||
| const stored = toTheme(localStorage.getItem(storageKey)); | ||
| // Requested behavior: prefer stored value, otherwise system preference. | ||
| return stored ?? "system"; | ||
| }; |
Contributor
There was a problem hiding this comment.
defaultTheme is ignored when storage is empty.
On Line 71, fallback is hardcoded to "system", so callers passing defaultTheme="light" or "dark" won’t get their configured default on first load.
Proposed fix
- // Requested behavior: prefer stored value, otherwise system preference.
- return stored ?? "system";
+ // Prefer stored value; otherwise respect configured default.
+ return stored ?? defaultTheme;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/components/theme-provider.tsx` around lines 66 - 72,
getInitialTheme currently falls back to the hardcoded string "system" which
ignores the configured defaultTheme; change the fallback to use the provided
defaultTheme instead. Update the return in getInitialTheme (and the
undefined-window early return) so it returns defaultTheme when no stored value
exists (e.g., return stored ?? defaultTheme) and keep existing system-handling
elsewhere if defaultTheme === "system"; reference getInitialTheme, defaultTheme,
storageKey, and toTheme to locate the code to change.
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
UI/UX
Behavior