feat: add tags to project metadata #69#164
Conversation
WalkthroughAdded a Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant PI as Portfolio Index
participant C as Content Collections
participant S as Slugify
participant UI as UI Components
U->>PI: Request /portfolio
PI->>C: Load projects (with `tags`)
C-->>PI: Projects metadata
PI->>S: slugify(tag labels)
S-->>PI: tag slugs
PI->>UI: Render uniqueTags (filters) & project cards (tag badges)
UI-->>U: Page with filters and projects
sequenceDiagram
autonumber
participant Build as Static Build
participant TP as Tag Page Generator
participant C as Content Collections
participant P as Paginator
Note over Build,TP: Build-time getStaticPaths
Build->>TP: Invoke getStaticPaths
TP->>C: Fetch all projects (with tags)
C-->>TP: Projects list
TP->>TP: Aggregate uniqueTags (use slugify)
loop For each tag
TP->>P: Paginate filtered projects (pageSize=6)
P-->>TP: Paginated pages with props (currentTag, uniqueTags)
end
TP-->>Build: Return tag-specific pages
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
✅ Deploy Preview for accessible-astro-starter-incluud ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
src/content/projects/project-04.mdx (1)
5-5: Consider more specific tags for the e-learning platform.While "machine-learning" aligns with the AI-driven personalization mentioned, "voice-ui" isn't clearly described. Consider adding more relevant tags like "education", "accessibility", "adaptive-learning", or "dyslexia-support" to improve filtering accuracy.
src/content/projects/project-06.mdx (1)
5-5: Consider adding healthcare-specific tags."voice-ui" is appropriate (voice commands mentioned), but "machine-learning" isn't clearly described. Consider adding more relevant tags like "healthcare", "accessibility", "hipaa", or "medical-records" to improve categorization.
src/content/projects/project-07.mdx (1)
5-5: Consider adding tags for additional input methods."voice-ui" is appropriate (voice control mentioned), but the project also features eye tracking and adaptive switches which aren't represented. Consider adding tags like "eye-tracking", "adaptive-controls", "smart-home", or "mobility" for better categorization.
src/pages/portfolio/[...page].astro (2)
19-20: Consider renamingtagstoallTagsfor consistency.The variable is renamed to
allTagsin the tag-filtered page (src/pages/portfolio/tag/[tag]/[...page].astroline 19), which is more descriptive. Consider using the same naming here for consistency across the codebase.Apply this diff:
- const tags = projects.map((project) => project.data.tags).flat() - const uniqueTags = [...new Set(tags)] + const allTags = projects.map((project) => project.data.tags).flat() + const uniqueTags = [...new Set(allTags)]
57-70: Use template literals for URL construction.While the logic is correct, prefer template literals over string concatenation for better readability and consistency with modern JavaScript practices.
Apply this diff:
uniqueTags.map((tag: string) => ( - <Link isButton size="sm" href={'/portfolio/tag/' + tag} class="mr-2"> + <Link isButton size="sm" href={`/portfolio/tag/${tag}`} class="mr-2"> {tag} </Link> ))Note: The explicit type annotation
(tag: string)is also redundant since TypeScript can infer the type fromuniqueTags: string[], but this is a minor style preference.src/pages/portfolio/tag/[tag]/[...page].astro (1)
117-122: Consider using semantic classes or theme variables for badge styling.The current tag badge uses inline Tailwind classes
"bg-blue-600 text-white". For better maintainability and theming consistency, consider defining a semantic CSS class or using theme variables that can be managed centrally.Example approaches:
- Define a semantic class in your CSS:
.badge-active { background-color: var(--theme-primary); color: var(--theme-primary-text); }
- Or extract to a reusable constant:
const ACTIVE_BADGE_CLASSES = 'bg-blue-600 text-white'This improves maintainability and makes theme updates easier.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/content.config.ts(1 hunks)src/content/projects/project-01.mdx(1 hunks)src/content/projects/project-02.mdx(1 hunks)src/content/projects/project-03.mdx(1 hunks)src/content/projects/project-04.mdx(1 hunks)src/content/projects/project-05.mdx(1 hunks)src/content/projects/project-06.mdx(1 hunks)src/content/projects/project-07.mdx(1 hunks)src/pages/portfolio/[...page].astro(5 hunks)src/pages/portfolio/tag/[tag]/[...page].astro(1 hunks)
🔇 Additional comments (9)
src/content.config.ts (1)
14-14: Schema addition is correct, but verify tag usage consistency across projects.The schema correctly adds an optional
tagsfield with a sensible default. However, the actual tag values in the project MDX files show concerning patterns (see comments on individual project files).src/content/projects/project-01.mdx (1)
5-5: LGTM!Tags are semantically appropriate and align well with the project description. This project features accessibility for visual impairments, smart home automation, voice-first interface, machine learning for speech recognition, and IoT devices.
src/pages/portfolio/[...page].astro (4)
4-4: LGTM!The additional imports for
BadgeandLinkcomponents are necessary for the tag filtering UI.
31-34: LGTM!Correctly passes
uniqueTagsthrough pagination props to make it available on all pages.
39-39: LGTM!The
uniqueTagsproperty is correctly added to the Props interface.
84-88: Verify spacing between tag badges.The tag badges are rendered without explicit spacing between them. Ensure that the
Badgecomponent fromaccessible-astro-componentsincludes default spacing, or add spacing classes to prevent badges from appearing crowded.Consider adding spacing if needed:
<span slot="meta"> - {project.data.tags.map((tag) => ( - <Badge>{tag}</Badge> + {project.data.tags.map((tag, index) => ( + <Badge class={index > 0 ? 'ml-2' : ''}>{tag}</Badge> ))} </span>src/pages/portfolio/tag/[tag]/[...page].astro (3)
1-16: LGTM!The imports are correct and necessary for the tag-filtered portfolio page. The duplication with the main portfolio page is acceptable for separate route files.
17-50: LGTM!The
getStaticPathsimplementation correctly generates paginated pages for each tag. The approach of iterating throughuniqueTagsand creating filtered, paginated pages is appropriate for this use case.
86-98: Verify Link’s disabled support. accessible-astro-components’Linkis used with both adisabledprop andclass="disabled". ConfirmLinksupportsdisabled; if so, remove the redundant class, otherwise implement disabling per its API.
| title: FinanceForAll | ||
| author: Viktor Nakamoto | ||
| description: A fully accessible financial management platform that makes banking and investment tools available to users with various disabilities | ||
| tags: ["voice-ui", "machine-learning"] |
There was a problem hiding this comment.
Tags don't reflect the described features.
FinanceForAll is described with screen-reader optimization and keyboard navigation, but neither "voice-ui" nor "machine-learning" are mentioned in the description or technical challenges.
Consider tags that match the actual features: "finance", "banking", "accessibility", "screen-reader", "keyboard-navigation".
🤖 Prompt for AI Agents
In src/content/projects/project-05.mdx around line 5, the tag line currently
lists ["voice-ui", "machine-learning"] which do not match the project's
description; replace those tags with ones that reflect the content (e.g.,
"finance", "banking", "accessibility", "screen-reader", "keyboard-navigation")
so the metadata matches the described features and accessibility focus.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
src/pages/portfolio/[...page].astro (1)
63-67: Slugify tags before using them in URLs (duplicate concern).This issue was previously flagged: tags containing spaces, slashes, or special characters (e.g.,
UI/UX,Design Systems) will break the URL or create invalid route segments. The previous review provided a detailed solution involving a{ label, slug }structure to keep routing stable.src/pages/portfolio/tag/[tag]/[...page].astro (2)
87-101: Keep routing slugs separate from display labels (duplicate concern).This issue was previously flagged: using raw tag strings in both
params: { tag }and<Link>URLs will break for tags with spaces, punctuation, or special characters. The previous review provided a solution using{ label, slug }structure.
132-140: Usepage.sizeinstead of hard-coded page size (duplicate concern).Lines 132, 138, and 140 all hard-code the page size as
6. This was previously flagged: these calculations should usepage.sizefor consistency and maintainability. The previous review provided specific diffs for each line.
🧹 Nitpick comments (2)
src/pages/portfolio/[...page].astro (1)
84-88: Consider adding spacing between tag badges.The badges are rendered without spacing, which may appear cramped when multiple tags are present. Adding a small margin or gap would improve visual clarity.
Apply this diff to add spacing:
<span slot="meta"> - {project.data.tags.map((tag) => ( + {project.data.tags.map((tag, index) => ( - <Badge>{tag}</Badge> + <Badge class={index > 0 ? 'ml-1' : ''}>{tag}</Badge> ))} </span>Or use a flex gap on the parent span:
- <span slot="meta"> + <span slot="meta" class="flex flex-wrap gap-1"> {project.data.tags.map((tag) => ( <Badge>{tag}</Badge> ))} </span>src/pages/portfolio/tag/[tag]/[...page].astro (1)
17-29: Consider extracting shared project loading logic.Lines 17-29 are nearly identical to the corresponding lines in
src/pages/portfolio/[...page].astro. The project loading, tag extraction, and image mapping logic could be extracted into a shared helper function to reduce duplication and improve maintainability.Example helper structure:
// src/utils/portfolio.ts export async function loadProjectsWithMetadata() { const projects = await getCollection('projects') const allTags = projects.map((project) => project.data.tags ?? []).flat().filter(Boolean) const uniqueTags = [...new Set(allTags)].sort((a, b) => a.localeCompare(b)) const projectImages = [projectImage1, projectImage2, /* ... */] const projectsWithImages = projects.map((project, index) => ({ ...project, featuredImage: projectImages[index % projectImages.length], })) return { projectsWithImages, uniqueTags } }Then import and use in both files:
import { loadProjectsWithMetadata } from '@utils/portfolio' export const getStaticPaths = (async ({ paginate }) => { const { projectsWithImages, uniqueTags } = await loadProjectsWithMetadata() // ... rest of logic }) satisfies GetStaticPaths
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/content/projects/project-01.mdx(1 hunks)src/content/projects/project-02.mdx(1 hunks)src/content/projects/project-03.mdx(1 hunks)src/content/projects/project-04.mdx(1 hunks)src/content/projects/project-05.mdx(1 hunks)src/content/projects/project-06.mdx(1 hunks)src/content/projects/project-07.mdx(1 hunks)src/pages/portfolio/[...page].astro(5 hunks)src/pages/portfolio/tag/[tag]/[...page].astro(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- src/content/projects/project-02.mdx
- src/content/projects/project-01.mdx
- src/content/projects/project-03.mdx
- src/content/projects/project-06.mdx
- src/content/projects/project-05.mdx
- src/content/projects/project-04.mdx
- src/content/projects/project-07.mdx
🔇 Additional comments (5)
src/pages/portfolio/[...page].astro (2)
31-34: LGTM!Passing
uniqueTagsthrough thepaginateprops is the correct approach for making tag data available across all paginated pages.
39-39: LGTM!The Props interface correctly extends the type to include
uniqueTags.src/pages/portfolio/tag/[tag]/[...page].astro (3)
34-46: Ensure tag URL params use slugified values (related to other comments).Line 39 sets
params: { tag }using the raw tag string. This must be updated to use the slugified version when implementing the slugification fix suggested in other comments (lines 87-101). The[tag]route parameter will receive the URL-safe slug, so the filtering logic (line 35) must also compare against the slug or maintain a mapping.After implementing the slugification approach suggested in previous comments, ensure:
params: { tag: slug }passes the URL-safe slug- Store a mapping or use the slug for filtering
- Pass both
slugandlabelin props for display purposes
52-58: LGTM!The Props interface is well-typed and includes all necessary data for rendering the tag-filtered portfolio page.
117-122: LGTM!The conditional styling for the current tag badge (blue background) provides clear visual feedback about which tag filter is active.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/pages/portfolio/tag/[tag]/[...page].astro (2)
132-132: Use page.size instead of hard-coded 6 for pagination mathKeeps UI consistent with getStaticPaths and future config changes.
Apply:
- page && Math.ceil(page.total / 6) > 1 && ( + page && Math.ceil(page.total / page.size) > 1 && ( @@ - lastPage={page.url.next ? `/portfolio/tag/${currentTag}/${Math.ceil(page.total / 6)}` : null} + lastPage={page.url.next ? `/portfolio/tag/${currentTag}/${Math.ceil(page.total / page.size)}` : null} @@ - totalPages={`${Math.ceil(page.total / 6)}`} + totalPages={`${Math.ceil(page.total / page.size)}`}Also applies to: 138-140
34-44: Slugify tags for routing; keep human-readable labels for displayUsing raw tags in routes breaks for spaces, punctuation, or “/”. Use { label, slug }:
- Generate slugs in getStaticPaths
- Set params: { tag: slug }
- Pass both currentTag (slug) and currentTagLabel (label)
- Render links with slug, display label
- Compare currentTag against slugs; compare badges against currentTagLabel
Suggested changes:
@@ -export const getStaticPaths = (async ({ paginate }) => { +export const getStaticPaths = (async ({ paginate }) => { const projects = await getCollection('projects') - const allTags = projects.map((project) => project.data.tags).flat() - const uniqueTags = [...new Set(allTags)].sort((a, b) => a.localeCompare(b)) + const allTags = projects.map((project) => project.data.tags).flat() + const uniqueTagLabels = [...new Set(allTags)].sort((a, b) => a.localeCompare(b)) + const uniqueTags = uniqueTagLabels.map((label) => ({ + label, + slug: label + .toLowerCase() + .trim() + .replace(/[^a-z0-9]+/g, '-') + .replace(/^-+|-+$/g, ''), + })) @@ - for (const tag of uniqueTags) { - const filteredProjects = projectsWithImages.filter((project) => project.data.tags.includes(tag)) + for (const { slug, label } of uniqueTags) { + const filteredProjects = projectsWithImages.filter((project) => project.data.tags.includes(label)) @@ - const paginatedPages = paginate(filteredProjects, { + const paginatedPages = paginate(filteredProjects, { pageSize: 6, - params: { tag }, + params: { tag: slug }, props: { - currentTag: tag, - uniqueTags, + currentTag: slug, + currentTagLabel: label, + uniqueTags, }, })-interface Props { - page: Page<CollectionEntry<'projects'> & { featuredImage: any }> - currentTag: string - uniqueTags: string[] -} +interface Props { + page: Page<CollectionEntry<'projects'> & { featuredImage: any }> + currentTag: string // slug + currentTagLabel: string + uniqueTags: Array<{ label: string; slug: string }> +} -const { page, currentTag, uniqueTags } = Astro.props +const { page, currentTag, currentTagLabel, uniqueTags } = Astro.props- <PageHeader - title={`Portfolio: ${currentTag}`} - subtitle={`Showing ${page.total} project${page.total !== 1 ? 's' : ''} tagged with "${currentTag}". <a href="/portfolio">View all projects</a> or filter by other tags below.`} + <PageHeader + title={`Portfolio: ${currentTagLabel}`} + subtitle={`Showing ${page.total} project${page.total !== 1 ? 's' : ''} tagged with "${currentTagLabel}". <a href="/portfolio">View all projects</a> or filter by other tags below.`}- { - uniqueTags.map((tag: string) => { - const isCurrentTag = tag === currentTag + { + uniqueTags.map(({ slug, label }) => { + const isCurrentTag = slug === currentTag return isCurrentTag ? ( - <Link isButton size="sm" href={'/portfolio/tag/' + tag} disabled class="disabled"> - {tag} + <Link isButton size="sm" href={'/portfolio/tag/' + slug} disabled class="disabled"> + {label} </Link> ) : ( - <Link isButton size="sm" href={'/portfolio/tag/' + tag}> - {tag} + <Link isButton size="sm" href={'/portfolio/tag/' + slug}> + {label} </Link> ) }) }- <PageHeader - title={`Portfolio: ${currentTag}`} + <PageHeader + title={`Portfolio: ${currentTagLabel}`}- {project.data.tags.map((tag) => { - const isCurrentTag = tag === currentTag - return isCurrentTag ? <Badge class="bg-blue-600 text-white">{tag}</Badge> : <Badge>{tag}</Badge> - })} + {project.data.tags.map((tag) => { + const isCurrentTag = tag === currentTagLabel + return isCurrentTag ? <Badge class="bg-blue-600 text-white">{tag}</Badge> : <Badge>{tag}</Badge> + })}Also applies to: 80-91, 116-119, 62-69, 52-56, 58-58, 17-21
src/pages/portfolio/[...page].astro (1)
19-20: Slugify tags for links; pass { label, slug } via propsAvoid broken routes for tags with spaces/slashes; keep labels for display.
Apply:
- const tags = projects.map((project) => project.data.tags).flat() - const uniqueTags = [...new Set(tags)].sort((a, b) => a.localeCompare(b)) + const tags = projects.map((project) => project.data.tags).flat() + const uniqueTagLabels = [...new Set(tags)].sort((a, b) => a.localeCompare(b)) + const uniqueTags = uniqueTagLabels.map((label) => ({ + label, + slug: label + .toLowerCase() + .trim() + .replace(/[^a-z0-9]+/g, '-') + .replace(/^-+|-+$/g, ''), + })) @@ - return paginate(projectsWithImages, { - pageSize: 6, - props: { uniqueTags }, - }) + return paginate(projectsWithImages, { + pageSize: 6, + props: { uniqueTags }, + }) @@ -interface Props { +interface Props { page: Page<CollectionEntry<'projects'> & { featuredImage: any }> - uniqueTags: string[] + uniqueTags: Array<{ label: string; slug: string }> } @@ -const { page, uniqueTags } = Astro.props +const { page, uniqueTags } = Astro.props @@ - uniqueTags.map((tag: string) => ( - <Link isButton size="sm" href={'/portfolio/tag/' + tag}> - {tag} + uniqueTags.map(({ slug, label }) => ( + <Link isButton size="sm" href={`/portfolio/tag/${slug}`}> + {label} </Link> ))Also applies to: 31-34, 39-39, 42-42, 61-66
🧹 Nitpick comments (1)
astro.config.mjs (1)
13-13: Run astro-compress last in the integrations listSafer to compress after other integrations generate assets.
Apply:
- integrations: [compress(), icon(), mdx(), sitemap()], + integrations: [icon(), mdx(), sitemap(), compress()],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
LICENSE(1 hunks)astro.config.mjs(2 hunks)src/layouts/DefaultLayout.astro(1 hunks)src/pages/portfolio/[...page].astro(4 hunks)src/pages/portfolio/tag/[tag]/[...page].astro(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- LICENSE
🔇 Additional comments (3)
src/layouts/DefaultLayout.astro (1)
4-6: Path alias switch looks goodAliases match astro.config.mjs; no behavior change.
astro.config.mjs (1)
2-2: New imports are correctfileURLToPath for aliases and mdx/sitemap integrations look good.
Also applies to: 5-6
src/pages/portfolio/tag/[tag]/[...page].astro (1)
19-21: Tag collection/sort looks safe given schema defaultscontent.config.ts defines tags: z.array(z.string()).default([]), so this pipeline is fine.
…s for improved navigation
…tro-starter into feature/tag-badges
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/utils/slugify.ts (1)
6-12: Consider input validation and unicode normalization.The slugification logic is sound for basic ASCII text, but consider these edge cases:
- Empty strings or strings with only special characters will produce empty slugs.
- Unicode characters (e.g., "café") are removed rather than transliterated (café → caf instead of cafe).
- No protection against null/undefined input.
Consider this enhanced implementation:
export function slugify(text: string): string { + if (!text) return '' + return text + .normalize('NFD') + .replace(/[\u0300-\u036f]/g, '') .toLowerCase() .trim() .replace(/[^a-z0-9]+/g, '-') .replace(/^-+|-+$/g, '') }This adds:
- Early return for empty input
- Unicode normalization to convert accented characters (é → e)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/components/PageHeader.astro(2 hunks)src/content/projects/project-01.mdx(1 hunks)src/content/projects/project-02.mdx(1 hunks)src/content/projects/project-03.mdx(1 hunks)src/content/projects/project-04.mdx(1 hunks)src/content/projects/project-05.mdx(1 hunks)src/content/projects/project-06.mdx(1 hunks)src/content/projects/project-07.mdx(1 hunks)src/pages/blog/[...page].astro(1 hunks)src/pages/portfolio/[...page].astro(4 hunks)src/pages/portfolio/tag/[tag]/[...page].astro(1 hunks)src/utils/slugify.ts(1 hunks)tsconfig.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- src/content/projects/project-06.mdx
- src/content/projects/project-05.mdx
- src/content/projects/project-07.mdx
- src/content/projects/project-04.mdx
- src/content/projects/project-02.mdx
- src/content/projects/project-03.mdx
🔇 Additional comments (10)
tsconfig.json (1)
6-17: LGTM! Clean addition of the utils path alias.The reformatting to multi-line arrays improves readability, and the new
@/utils/*alias enables cleaner imports throughout the codebase.src/content/projects/project-01.mdx (1)
5-5: LGTM! Tags metadata added correctly.The tags field aligns with the content schema and will be properly consumed by the portfolio listing and filtering pages.
src/pages/blog/[...page].astro (1)
87-101: LGTM! Improved pagination logic usingpage.lastPage.The refactor correctly uses the built-in
page.lastPageproperty instead of manually computing total pages, improving maintainability and eliminating the hard-coded page size.src/components/PageHeader.astro (1)
47-51: LGTM! Custom breadcrumb labels implemented correctly.The new
customBreadcrumbLabelsprop enables clean override of URL-based breadcrumb text, with proper fallback to the existing formatting logic. The optional chaining is used correctly.Also applies to: 62-62, 96-102
src/pages/portfolio/tag/[tag]/[...page].astro (3)
18-57: LGTM! Static path generation correctly implements slugification.The implementation properly separates routing slugs from display labels:
- Tags are slugified for URL params (
params: { tag: tag.slug })- Projects are filtered using slugified tags
- Human-readable labels are preserved for display
This addresses the past review concerns about URL-unsafe tag names.
88-103: LGTM! Tag navigation uses slugs consistently.The tag filter UI correctly:
- Uses
tag.slugin hrefs for routing- Compares
tag.slug === currentTag.slugfor current state- Displays
tag.labelfor human-readable textThis addresses the past review concerns about keeping routing separate from display.
142-154: LGTM! Pagination usespage.lastPageconsistently.The pagination logic correctly uses
page.lastPagethroughout instead of hard-coded page sizes, addressing the past review concerns.src/pages/portfolio/[...page].astro (3)
20-25: LGTM! Tag collection properly implements slugification.The code correctly creates
uniqueTagswith bothlabelandslugproperties, addressing the past review concerns about URL-safe routing.
64-72: LGTM! Tag navigation uses slugs for URLs.The tag filter links correctly use
tag.slugfor routing while displayingtag.labelfor readability, addressing the past review concerns.
102-116: LGTM! Pagination usespage.lastPageconsistently.The pagination implementation correctly uses
page.lastPageinstead of hard-coded calculations, matching the pattern from the blog page refactor.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/pages/portfolio/[...page].astro (1)
20-25: LGTM! Tag processing logic is correctly implemented.The tag extraction, deduplication, slugification, and prop passing are all correct. The implementation mirrors the logic in the tag-specific page, which ensures consistency.
Consider extracting shared tag processing logic (optional).
Both this file and
src/pages/portfolio/tag/[tag]/[...page].astrocontain identical tag processing logic (lines 20-25 here, lines 19-25 there). While the duplication is minimal and acceptable for separate pages, you could optionally extract this to a shared utility function likegetUniqueTags(projects)to ensure both pages stay in sync if the logic evolves.Also applies to: 36-39, 44-44, 47-47
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/pages/portfolio/[...page].astro(5 hunks)src/pages/portfolio/tag/[tag]/[...page].astro(1 hunks)
🔇 Additional comments (6)
src/pages/portfolio/tag/[tag]/[...page].astro (3)
18-57: LGTM! Well-structured static path generation.The
getStaticPathsimplementation correctly:
- Extracts and deduplicates tags from all projects
- Slugifies tags for URL-safe routing while preserving display labels
- Filters projects by comparing slugified tags (line 41)
- Generates paginated pages for each tag with proper props
The round-robin image assignment (line 32) is a practical approach for demo content.
59-66: LGTM! Clear type definitions.The Props interface correctly defines all required props with proper types, including the Page type for Astro pagination and the tag shape with label and slug.
68-150: LGTM! Solid template implementation with good UX touches.The template correctly implements:
- Custom breadcrumb labels for improved navigation (lines 76-78)
- Tag filter panel with proper disabled state for the current tag (lines 88-96)
- Tag badge highlighting that compares display labels (line 121)
- Pagination using
page.lastPageproperty (lines 134, 140, 142) instead of hard-coded calculationsAll past concerns have been addressed.
src/pages/portfolio/[...page].astro (3)
61-68: LGTM! Clean tag navigation UI.The tag filter panel correctly generates links using slugified URLs while displaying human-readable labels. The layout uses flexbox with wrapping for responsive behavior.
84-88: LGTM! Tag badges enhance project cards.The meta slot correctly displays all project tags as badges. Unlike the tag-specific page, there's no highlighting here, which is appropriate since this is the main portfolio view showing all projects.
98-112: LGTM! Pagination correctly uses page properties.The pagination component properly uses
page.lastPagefor all calculations and conditionally renders only when multiple pages exist. This avoids the hard-coded page size issues flagged in earlier reviews.
…tro-starter into feature/tag-badges
| if (joinLastBreadcrumb && index === segments.length - 1) { | ||
| return null | ||
| } | ||
|
|
||
| const url = `/${segments.slice(0, index + 1).join('/')}` | ||
| const isLast = index === segments.length - 1 | ||
| const formattedName = segment | ||
| .split('-') | ||
| .map((word) => word.charAt(0).toUpperCase() + word.slice(1)) | ||
| .join(' ') | ||
| const isLast = index === segments.length - 1 || (joinLastBreadcrumb && index === segments.length - 2) | ||
|
|
||
| const formattedName = (() => { | ||
| if (joinLastBreadcrumb && isLast) { | ||
| const prevSegment = segments[segments.length - 2] | ||
| const lastSegment = segments[segments.length - 1] | ||
| return `${formatSegment(prevSegment)}: ${formatSegment(lastSegment)}` | ||
| } | ||
|
|
||
| return formatSegment(segment) | ||
| })() |
There was a problem hiding this comment.
Guard joinLastBreadcrumb for single-segment paths
With joinLastBreadcrumb enabled on a single-segment route (e.g. /portfolio), the lone breadcrumb item is suppressed because we null out the final segment even though there is nothing to join. That regresses the breadcrumb trail to just “Home”. Please gate the join-only logic behind a segments.length > 1 check so single-level pages keep their label.
- if (joinLastBreadcrumb && index === segments.length - 1) {
+ if (joinLastBreadcrumb && segments.length > 1 && index === segments.length - 1) {
return null
}
const url = `/${segments.slice(0, index + 1).join('/')}`
- const isLast =
- index === segments.length - 1 ||
- (joinLastBreadcrumb && index === segments.length - 2)
+ const isLast =
+ index === segments.length - 1 ||
+ (joinLastBreadcrumb && segments.length > 1 && index === segments.length - 2)
const formattedName = (() => {
- if (joinLastBreadcrumb && isLast) {
+ if (joinLastBreadcrumb && segments.length > 1 && index === segments.length - 2) {
const prevSegment = segments[segments.length - 2]
const lastSegment = segments[segments.length - 1]
return `${formatSegment(prevSegment)}: ${formatSegment(lastSegment)}`
}📝 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.
| if (joinLastBreadcrumb && index === segments.length - 1) { | |
| return null | |
| } | |
| const url = `/${segments.slice(0, index + 1).join('/')}` | |
| const isLast = index === segments.length - 1 | |
| const formattedName = segment | |
| .split('-') | |
| .map((word) => word.charAt(0).toUpperCase() + word.slice(1)) | |
| .join(' ') | |
| const isLast = index === segments.length - 1 || (joinLastBreadcrumb && index === segments.length - 2) | |
| const formattedName = (() => { | |
| if (joinLastBreadcrumb && isLast) { | |
| const prevSegment = segments[segments.length - 2] | |
| const lastSegment = segments[segments.length - 1] | |
| return `${formatSegment(prevSegment)}: ${formatSegment(lastSegment)}` | |
| } | |
| return formatSegment(segment) | |
| })() | |
| if (joinLastBreadcrumb && segments.length > 1 && index === segments.length - 1) { | |
| return null | |
| } | |
| const url = `/${segments.slice(0, index + 1).join('/')}` | |
| const isLast = | |
| index === segments.length - 1 || | |
| (joinLastBreadcrumb && segments.length > 1 && index === segments.length - 2) | |
| const formattedName = (() => { | |
| if (joinLastBreadcrumb && segments.length > 1 && index === segments.length - 2) { | |
| const prevSegment = segments[segments.length - 2] | |
| const lastSegment = segments[segments.length - 1] | |
| return `${formatSegment(prevSegment)}: ${formatSegment(lastSegment)}` | |
| } | |
| return formatSegment(segment) | |
| })() |
🤖 Prompt for AI Agents
In src/components/PageHeader.astro around lines 110–125, the joinLastBreadcrumb
logic can suppress the only breadcrumb on single-segment pages; guard the
join-only behavior with a segments.length > 1 check so single-level routes still
render their label. Specifically, require segments.length > 1 before returning
null for the last segment, include the same segments.length > 1 condition when
computing isLast, and only run the "join prev and last segment" branch of
formattedName when segments.length > 1.
…lement in portfolio tag page
| title={`Portfolio: ${currentTag.label}`} | ||
| subtitle={`Showing ${page.total} project${page.total !== 1 ? 's' : ''} tagged with "${currentTag.label}". <a href="/portfolio">View all projects</a> or filter by other tags below.`} | ||
| bgType="bordered" |
There was a problem hiding this comment.
Correct the subtitle project count.
page.total is the total number of items across all paginated pages, so on page 1 we only display 6 projects yet tell the user we're showing (say) 12. Swap to the per-page count to keep the UI truthful.
- subtitle={`Showing ${page.total} project${page.total !== 1 ? 's' : ''} tagged with "${currentTag.label}". <a href="/portfolio">View all projects</a> or filter by other tags below.`}
+ subtitle={`Showing ${page.end - page.start + 1} project${page.end - page.start + 1 !== 1 ? 's' : ''} tagged with "${currentTag.label}". <a href="/portfolio">View all projects</a> or filter by other tags below.`}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/pages/portfolio/tag/[tag]/[...page].astro around lines 73-75, the
subtitle uses page.total (total across all pages) but should show the number of
projects on the current page; replace page.total with the current-page count
(e.g. the length of the array that holds projects for this page, such as
page.items.length or page.data.length depending on your pagination shape) so the
subtitle reads "Showing X project(s) tagged with…" where X is the per-page
count.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/pages/portfolio/[...page].astro (1)
63-70: Tag filtering UI looks good.The implementation correctly uses slugified URLs for tag links and displays human-readable labels. The tag navigation will work properly now that slugs are being used for routing.
For consistency with line 104, consider using a template literal on line 67:
- <Link href={'/portfolio/tag/' + tag.slug}>{tag.label}</Link> + <Link href={`/portfolio/tag/${tag.slug}`}>{tag.label}</Link>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/pages/portfolio/[...page].astro(5 hunks)
🔇 Additional comments (2)
src/pages/portfolio/[...page].astro (2)
86-90: LGTM! Tag badges enhance project cards nicely.The meta slot implementation cleanly displays project tags using Badge components, providing good visual hierarchy and metadata organization.
97-111: Pagination implementation is solid.The conditional rendering (
page.lastPage > 1) correctly hides pagination when unnecessary, and the component receives all required props for proper navigation. The URL construction properly handles first, previous, next, and last page links.
| const { page } = Astro.props | ||
| const { page, uniqueTags } = Astro.props | ||
|
|
||
| console.log(page.lastPage) |
There was a problem hiding this comment.
Remove debug console.log statement.
This debug logging statement should be removed before merging to production.
Apply this diff:
-console.log(page.lastPage)📝 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.
| console.log(page.lastPage) |
🤖 Prompt for AI Agents
In src/pages/portfolio/[...page].astro around line 49, remove the debug
console.log(page.lastPage) statement; simply delete that line (or replace it
with a proper logger call if intentional debug info is required behind a feature
flag) so no debug console output remains in production.
2 participants
Add tags to project and add filter buttons to portfolio page
Summary by CodeRabbit
New Features
Bug Fixes / UX
Chores