refactor: multi-author support with stacked avatars and linkable names #2669
Conversation
WalkthroughThe PR adds multi-author support by introducing src/lib/utils/blog-authors.ts which exports AuthorInfo and getPostAuthors(postAuthor, allAuthors) returning postAuthors, authorAvatars, and primaryAuthor. The Article component (src/lib/components/Article.svelte and src/lib/components/blog/article.svelte) now accepts authors: AuthorInfo[] and avatars: string[] (replacing single author/avatar), and its markup was updated to render stacked avatars, linked author names, a date/time line, and adjusted cover/title anchors. Markdoc layouts and blog pages (src/markdoc/layouts/Author.svelte, Category.svelte, Post.svelte, and src/routes/blog/[[page]]/+page.svelte) were updated to call getPostAuthors and pass the new props. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 |
![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.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/lib/components/blog/article.svelte (1)
5-8: Extract the duplicated AuthorInfo interface to a shared location.This is a duplicate of the same issue in
Article.svelte. TheAuthorInfointerface should be imported fromsrc/lib/utils/blog-authors.tsrather than redefined here.🔎 Proposed fix
+import type { AuthorInfo } from '$lib/utils/blog-authors'; + -interface AuthorInfo { - name: string; - href: string; -} -
🧹 Nitpick comments (1)
src/lib/components/blog/article.svelte (1)
46-48: Note: Different avatar overlap margin compared to Article.svelte.This file uses a
-8pxmargin for avatar overlap, whileArticle.svelteuses-4px. Similarly, this file usessize-6avatars while the other usessize-5. This might be intentional for different layout contexts, but ensure the visual design is consistent where appropriate.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lib/components/Article.sveltesrc/lib/components/blog/article.svelte
⏰ 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: build
- GitHub Check: tests
- GitHub Check: assets
🔇 Additional comments (5)
src/lib/components/Article.svelte (2)
24-41: Nice UX improvements with linkable cover and title.The refactor successfully makes both the cover image and title clickable, improving the user experience. The hover underline on the title provides good visual feedback.
43-55: Well-implemented stacked avatar effect.The avatar stacking with negative margins (
-4px) and z-index ordering creates a nice visual effect. Theloading="lazy"attribute is good for performance.src/lib/components/blog/article.svelte (3)
42-65: Consider validating array alignment between authors and avatars.Similar to
Article.svelte, this component assumes index alignment between theauthorsandavatarsarrays (line 53). Misaligned arrays could result in incorrect alt text or author associations.
44-54: Good addition of explicit width and height attributes.The explicit
width="24"andheight="24"attributes on the avatar images help prevent Cumulative Layout Shift (CLS) and improve Core Web Vitals scores.
23-39: Well-structured multi-author layout.The refactored layout successfully separates the cover, date/time, and title into distinct, linkable sections. The transition and hover effects maintain good UX.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/lib/components/Article.svelte (1)
41-51: Array misalignment concern remains applicable.The optional chaining on
authors[i]?.nameprevents crashes, but the root cause—avatarsandauthorsarrays potentially having different lengths—should be addressed ingetPostAuthorswithinblog-authors.ts, as noted in the previous review.
🧹 Nitpick comments (1)
src/lib/components/Article.svelte (1)
43-43: Consider using a theme-aware color for the avatar ring.The hardcoded
ring-[#19191c]may not adapt well if the design system supports light/dark themes or if the background color changes. Consider using a CSS variable or semantic color class if available.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lib/components/Article.svelte
⏰ 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: build
- GitHub Check: tests
- GitHub Check: assets
🔇 Additional comments (4)
src/lib/components/Article.svelte (4)
5-6: Good: AuthorInfo is now imported from the shared module.This addresses the previous review feedback about the duplicated interface definition. The type is now sourced from
$lib/utils/blog-authors, maintaining a single source of truth.
7-17: Props interface correctly updated for multi-author support.The transition from single
author/avatarstrings toauthors/avatarsarrays aligns with the broader refactor. Based on the AI summary, calling components have been updated accordingly.
21-29: LGTM!The cover image is now clickable with a clean hover scale effect, and accessible alt text is properly set.
30-38: LGTM!The date/time metadata and title link are cleanly separated with appropriate styling and hover affordance.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/lib/components/blog/article.svelte (1)
43-62: Consider the UX impact of rendering all author names when some authors lack avatars.The avatar rendering (lines 43-55) uses the filtered
authorAvatarPairs, while the author names (lines 58-62) iterate over the fullauthorsarray. This means if one author lacks an avatar, users will see, for example, 2 stacked avatars but 3 comma-separated names.This may be intentional design to credit all authors regardless of avatar availability, but it could cause confusion about which avatar corresponds to which name. Consider one of these approaches:
- Add a visual indicator (e.g., initials badge) for authors without avatars
- Display only authors with avatars in both sections
- Add a design note explaining the intentional mismatch
src/lib/components/Article.svelte (1)
19-21: Consider extracting the shared multi-author rendering logic.The
authorAvatarPairsderivation (lines 19-21) and the avatar/name rendering logic (lines 45-62) are nearly identical tosrc/lib/components/blog/article.svelte, differing only in styling details (avatar size, overlap margin, and some missing attributes likewidth/height).Extracting this logic into a shared component or helper would improve maintainability and ensure consistent behavior across both files when bugs are fixed or enhancements are made.
💡 Example: Shared AuthorList component
<!-- src/lib/components/AuthorList.svelte --> <script lang="ts"> import type { AuthorInfo } from '$lib/utils/blog-authors'; interface Props { authors: AuthorInfo[]; avatars: string[]; avatarSize?: 'sm' | 'md'; // 'sm' = size-5/-4px, 'md' = size-6/-8px } const { authors, avatars, avatarSize = 'md' }: Props = $props(); const sizeClasses = avatarSize === 'sm' ? 'size-5' : 'size-6'; const overlapMargin = avatarSize === 'sm' ? '-4px' : '-8px'; const authorAvatarPairs = $derived( avatars.map((avatar, i) => ({ avatar, author: authors[i] })).filter(({ avatar }) => avatar) ); </script> <!-- avatar and name rendering here -->Then import and use in both Article files.
Also applies to: 45-62
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/lib/components/Article.sveltesrc/lib/components/blog/article.sveltesrc/lib/utils/blog-authors.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/utils/blog-authors.ts
⏰ 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: assets
- GitHub Check: tests
- GitHub Check: build
🔇 Additional comments (1)
src/lib/components/blog/article.svelte (1)
18-20: The array length guarantee fromgetPostAuthorsis already in place.Looking at the implementation in
src/lib/utils/blog-authors.ts(lines 39-47), bothpostAuthorsandauthorAvatarsare populated in the same conditional loop. This guarantees they always have matching lengths. All call sites (src/routes/blog/[[page]]/+page.svelte,src/markdoc/layouts/Post.svelte, etc.) source these arrays directly fromgetPostAuthors, so the concern about mismatched lengths is unfounded.However, the code does have a subtle UX issue: avatars are filtered by emptiness (line 19), but author names are always displayed unfiltered (line 59-62). This means if an author lacks an avatar, their name appears in the author list but their avatar doesn't appear in the stacked avatar section—creating a visual mismatch between avatar count and name count.
Likely an incorrect or invalid review comment.
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.