Remove New Moon theme and implement dynamic tag system#4
Conversation
- Add tooltip to dark mode toggle button in Nav - Make blog post tags clickable, linking to /tags/[tag] - Create dynamic tag pages showing all posts with that tag - Remove original-author field from package.json code written with the help of Claude Code
code written with the help of Claude Code
WalkthroughRemoved the New Moon theme and its import, added tag navigation and a dynamic tag page, added a tooltip to the dark-mode button, and removed a non-functional package.json metadata field. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
coderabbitai
Bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/pages/tags/[tag].astro (2)
5-39: Refactor to avoid duplicate data fetching.Hey friend! I'm seeing a performance opportunity here. You're fetching posts and guides twice - once in
getStaticPaths(lines 6-7) and again in the component body (lines 27-28).While Astro handles this at build time (not runtime), it still means you're re-fetching and re-filtering the entire collection for every tag page. If you have 20 tags, that's 20 unnecessary collection fetches!
The more elegant approach: do the filtering once in
getStaticPathsand pass the filtered content through props. This is exactly what props are for!🔎 Refactored approach with filtered content in props
export async function getStaticPaths() { const posts = await getCollection('posts', ({ data }) => !data.hidden); const guides = await getCollection('guides'); + const allContent = [...posts, ...guides]; // Collect all unique tags const allTags = new Set<string>(); - [...posts, ...guides].forEach(entry => { + allContent.forEach(entry => { if (entry.data.tags) { entry.data.tags.forEach((tag: string) => allTags.add(tag.toLowerCase())); } }); // Generate paths for each tag return Array.from(allTags).map(tag => ({ params: { tag }, - props: { tag } + props: { + tag, + content: allContent + .filter(entry => entry.data.tags?.some((t: string) => t.toLowerCase() === tag)) + .sort((a, b) => { + const dateA = a.data.date ? new Date(a.data.date).getTime() : 0; + const dateB = b.data.date ? new Date(b.data.date).getTime() : 0; + return dateB - dateA; + }) + } })); } -const { tag } = Astro.props; +const { tag, content: sortedContent } = Astro.props; -// Get all posts and guides with this tag -const posts = await getCollection('posts', ({ data }) => !data.hidden); -const guides = await getCollection('guides'); - -const allContent = [...posts, ...guides].filter(entry => - entry.data.tags?.some((t: string) => t.toLowerCase() === tag) -); - -// Sort by date (newest first) -const sortedContent = allContent.sort((a, b) => { - const dateA = a.data.date ? new Date(a.data.date).getTime() : 0; - const dateB = b.data.date ? new Date(b.data.date).getTime() : 0; - return dateB - dateA; -}); const displayTag = tag.charAt(0).toUpperCase() + tag.slice(1);
41-41: Consider more sophisticated capitalization for multi-word tags.Quick heads up! The current capitalization logic (
charAt(0).toUpperCase() + tag.slice(1)) works great for single-word tags, but might not handle multi-word tags or acronyms the way you'd want:
"machine-learning"→"Machine-learning"(probably want "Machine-Learning")"api"→"Api"(probably want "API"`)Totally works as-is, but if you find yourself with hyphenated tags or acronyms, you might want to revisit this. Could be a future enhancement!
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
package.jsonsrc/components/astro/Nav.astrosrc/layouts/BaseLayout.astrosrc/new-moon.csssrc/pages/[...slug].astrosrc/pages/tags/[tag].astro
💤 Files with no reviewable changes (3)
- src/layouts/BaseLayout.astro
- package.json
- src/new-moon.css
🔇 Additional comments (4)
src/components/astro/Nav.astro (1)
17-17: Awesome accessibility improvement! 🎉Hey there! Love seeing the
titleattribute added here. While thearia-labelhandles screen readers beautifully, thistitlegives sighted users that nice tooltip on hover. It's a small touch that makes the UX more discoverable. Well done!src/pages/[...slug].astro (2)
44-44: Nice work making tags navigable! 🚀This transformation from static spans to clickable links is solid. The
toLowerCase()normalization ensures consistency with your tag page routes. Users can now explore content by tag - that's exactly what we want!
85-91: Clean styling with a nice hover effect! 👌The link styling here is on point. Removing the underline keeps tags looking like tags rather than traditional links, and the hover state gives users that immediate feedback. The fallback for
--lighter-backgroundis a nice defensive touch too!src/pages/tags/[tag].astro (1)
66-131: Solid, semantic styling! 💪The CSS here is clean and well-organized. You're using:
- Semantic HTML structure (ul/li)
- Flexbox for layout
- CSS custom properties for theming
- Responsive design with a media query
The spacing, typography, and hover states all look good. Nice work keeping the styles scoped and maintainable!
Move filtering and sorting into getStaticPaths, pass through props. code written with the help of Claude Code
1 participant
Summary by CodeRabbit
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.