refactor: multi-author support with stacked avatars and linkable names #2669
Conversation
WalkthroughThis PR refactors blog author handling to support multiple authors. It adds src/lib/utils/blog-authors.ts with AuthorInfo and getPostAuthors(postAuthor, allAuthors) to normalize slugs and return postAuthors, authorAvatars, and primaryAuthor. Article components (two Article.svelte variants) now accept authors: AuthorInfo[] and avatars: string[] instead of single author/avatar, and render stacked avatars and linked author names. Markdoc layouts and the blog page (Author.svelte, Category.svelte, Post.svelte, src/routes/blog/[[page]]/+page.svelte) were updated to call getPostAuthors and pass the new props. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
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 |
![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 (6)
src/lib/utils/blog-authors.ts (2)
3-6: Consider exportingAuthorInfofor reuse across components.The
AuthorInfointerface is duplicated in bothsrc/lib/components/Article.svelteandsrc/lib/components/blog/article.svelte. Exporting it from this utility module would establish a single source of truth.-export interface AuthorInfo { +export interface AuthorInfo { name: string; href: string; }Then in the Article components:
import { type AuthorInfo } from '$lib/utils/blog-authors';
22-33: Optional: Pre-index authors by slug for O(1) lookup.The current implementation performs linear searches through
allAuthorsfor each slug (lines 23-24, 27, 32). For typical blog sizes this is fine, but aMapwould be more efficient if author counts grow.+ const authorsBySlug = new Map(allAuthors.map((a) => [a.slug, a])); + const primaryAuthor = - allAuthors.find((a) => a.slug === primarySlug) || - allAuthors.find((a) => postAuthorSlugs.includes(a.slug)); + authorsBySlug.get(primarySlug) || + postAuthorSlugs.map((s) => authorsBySlug.get(s)).find(Boolean); const postAuthors = postAuthorSlugs - .map((slug) => allAuthors.find((a) => a.slug === slug)) + .map((slug) => authorsBySlug.get(slug)) .filter((a): a is AuthorData => !!a) .map((a) => ({ name: a.name, href: a.href })); const authorAvatars = postAuthorSlugs - .map((slug) => allAuthors.find((a) => a.slug === slug)?.avatar) + .map((slug) => authorsBySlug.get(slug)?.avatar) .filter((a): a is string => !!a);src/lib/components/blog/article.svelte (1)
42-42: Hardcoded ring color may not adapt to theme changes.The ring color
#19191cappears to be a dark background. If the site supports light mode or theme customization, this could clash.Consider using a CSS variable or Tailwind theme class instead:
class="size-6 rounded-full ring-2 ring-background"src/lib/components/Article.svelte (2)
5-8: DuplicateAuthorInfointerface across components.This interface is defined identically in
src/lib/components/blog/article.svelteandsrc/lib/utils/blog-authors.ts. Importing from the utility would ensure consistency.+import { type AuthorInfo } from '$lib/utils/blog-authors'; + - interface AuthorInfo { - name: string; - href: string; - }
43-43: Hardcoded ring color duplicated here as well.Same concern as
blog/article.svelte—the hardcoded#19191cmay not adapt to theme changes.src/routes/blog/[[page]]/+page.svelte (1)
272-278: Consider UX consistency between avatar count and author names.The code displays up to 3 stacked avatars (line 255) but renders all author names with links. If a post has more than 3 authors, users will see only 3 avatars but all names listed. This may be intentional design, but please verify this is the desired UX.
If limiting both is preferred, apply this diff:
<span class="text-sub-body"> - {#each featuredAuthors as featuredAuthor, i} + {#each featuredAuthors.slice(0, 3) as featuredAuthor, i} <a href={featuredAuthor.href} class="web-link" >{featuredAuthor.name}</a - >{#if i < featuredAuthors.length - 1},{' '}{/if} + >{#if i < Math.min(3, featuredAuthors.length) - 1},{' '}{/if} {/each} + {#if featuredAuthors.length > 3} + {' '}and {featuredAuthors.length - 3} more + {/if} </span>
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/lib/components/Article.svelte(1 hunks)src/lib/components/blog/article.svelte(2 hunks)src/lib/utils/blog-authors.ts(1 hunks)src/markdoc/layouts/Author.svelte(2 hunks)src/markdoc/layouts/Category.svelte(2 hunks)src/markdoc/layouts/Post.svelte(2 hunks)src/routes/blog/[[page]]/+page.svelte(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/lib/utils/blog-authors.ts (1)
src/routes/blog/content.ts (1)
AuthorData(8-18)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: tests
- GitHub Check: assets
- GitHub Check: build
🔇 Additional comments (9)
src/markdoc/layouts/Post.svelte (1)
195-209: Clean integration of centralized author resolution.The use of
getPostAuthorscentralizes author data extraction, and the conditional render onprimaryAuthorproperly guards against posts with unresolved authors. This approach is consistent with the other layout files.src/markdoc/layouts/Author.svelte (1)
174-193: Fallback logic differs from other layouts.Unlike
Post.svelteandCategory.sveltewhich conditionally renderArticleonly whenprimaryAuthorexists, this file always renders but uses fallback values. This is likely intentional since we're on an author's page and can use the page-levelavatar/nameprops as defaults. The approach ensures posts are always displayed even ifgetPostAuthorsreturns incomplete data.Please verify this is the intended behavior—that posts on an author's page should always render using the page author's info as fallback, rather than being hidden when author resolution fails.
src/markdoc/layouts/Category.svelte (1)
62-76: Consistent multi-author integration.The implementation mirrors
Post.sveltewith proper destructuring and conditional rendering based onprimaryAuthor. This ensures consistency across category listing pages.src/lib/components/blog/article.svelte (1)
40-52: Potential index mismatch betweenavatarsandauthorsarrays.The alt text uses
authors[i]?.nameto match each avatar. If these arrays have different lengths (e.g., an author exists but has no avatar), the indexing may produce incorrect alt text. The optional chaining (?.) prevents crashes, but semantics could be off.Verify that
getPostAuthorsalways returns aligned arrays, or consider iterating over a combined structure:// In getPostAuthors, return combined data: const authorsWithAvatars = postAuthorSlugs .map(slug => allAuthors.find(a => a.slug === slug)) .filter((a): a is AuthorData => !!a) .map(a => ({ name: a.name, href: a.href, avatar: a.avatar }));src/lib/components/Article.svelte (1)
41-51: Same array alignment concern asblog/article.svelte.This component uses the same pattern of indexing
authors[i]while iterating overavatars. The same potential mismatch risk applies here.src/routes/blog/[[page]]/+page.svelte (4)
9-9: LGTM!The import is correctly added to support multi-author functionality.
403-404: Verify Article component API changes across the codebase.The props passed to
Articlehave changed from single values to arrays. Ensure theArticlecomponent has been updated to acceptavatars: string[]andauthors: AuthorInfo[], and verify all other call sites are updated.Run the following script to verify the Article component implementation and find all usages:
#!/bin/bash # Description: Verify Article component accepts new array props and find all usages echo "=== Article component implementation ===" # Find Article component definition and show prop types fd -e svelte 'Article.svelte' src/lib/components --exec cat {} \; | head -50 echo -e "\n=== All Article component usages ===" # Find all files using Article component rg -n --type=svelte '<Article' -A 5 -B 1
216-220: Verify edge case handling for missing or undefined authors.The destructuring assumes
getPostAuthorswill always return valid data structures. Iffeatured.authoris undefined or the function returns empty arrays, the rendering logic should handle it gracefully.Run the following script to verify the
getPostAuthorsimplementation handles edge cases:#!/bin/bash # Description: Check getPostAuthors implementation for null/undefined handling and return value structure # Find and display the getPostAuthors function implementation ast-grep --pattern $'export function getPostAuthors($$$) { $$$ }'Additionally, please search the web to confirm best practices for handling undefined author data in Svelte components:
How to handle undefined props in Svelte components with optional chaining and default valuesAlso applies to: 255-269
392-396: LGTM! Code safely handles author data extraction.The logic correctly extracts author data for each post via getPostAuthors(), which gracefully handles edge cases by filtering out undefined authors and returning a typed primaryAuthor. The conditional
{#if primaryAuthor && !post.draft}ensures only posts with valid authors are rendered. The author field is required in the PostsData interface, and all posts include this field.
src/lib/utils/blog-authors.ts
Outdated
| const primaryAuthor = | ||
| allAuthors.find((a) => a.slug === primarySlug) || | ||
| allAuthors.find((a) => postAuthorSlugs.includes(a.slug)); | ||
|
|
||
| const postAuthors = postAuthorSlugs | ||
| .map((slug) => allAuthors.find((a) => a.slug === slug)) | ||
| .filter((a): a is AuthorData => !!a) | ||
| .map((a) => ({ name: a.name, href: a.href })); | ||
|
|
||
| const authorAvatars = postAuthorSlugs | ||
| .map((slug) => allAuthors.find((a) => a.slug === slug)?.avatar) | ||
| .filter((a): a is string => !!a); |
There was a problem hiding this comment.
we can do some nice optimizations here, multiple iterations are not needed.
What does this PR do?
Before:
After
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)
Summary by CodeRabbit
New Features
UI Updates
✏️ Tip: You can customize this high-level summary in your review settings.