[FE-Feat] Avatar 컴포넌트 기본 레이아웃 작성 #107
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request introduces several new Avatar-related features. A Storybook configuration is added to showcase the Avatar component with metadata, controls, and a default story. New components—Avatar, AvatarItem, and AvatarCount—are created to render avatars with a maximum display limit, showing additional count information when necessary. A CSS module is also provided to style the avatar components with responsive size variations. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as User Interface
participant A as Avatar Component
participant AI as AvatarItem Component
participant AC as AvatarCount Component
UI->>A: Render Avatar(imageUrls, size)
loop For each imageUrl in first four images
A->>AI: Render AvatarItem(imageUrl, size)
end
alt imageUrls.length > MAX_IMAGE_COUNT
A->>AC: Render AvatarCount(count = remaining images)
end
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (7)
frontend/src/components/Avatar/AvatarItem.tsx (1)
4-8: Props interface looks good, but consider adding optional onError handler.The interface is well-defined with required props. Consider adding an optional onError handler for image load failures.
interface AvatarItemProps { src: string; alt: string; size: Size; + onError?: (event: React.SyntheticEvent<HTMLImageElement, Event>) => void; }frontend/src/components/Avatar/Avatar.stories.tsx (2)
5-21: Enhance Storybook configuration with documentation and controls.Consider adding:
- Component description and usage examples
- Documentation for each prop
- Control for imageUrls with realistic placeholder images
const meta: Meta = { title: 'Components/Avatar', component: Avatar, + parameters: { + docs: { + description: { + component: 'Avatar component displays a group of user avatars with overflow count.', + }, + }, + backgrounds: { + default: 'dark', + }, + }, argTypes: { size: { + description: 'Size of the avatar (sm: 24px, lg: 32px)', control: { type: 'radio', options: ['sm', 'lg'] }, }, imageUrls: { - control: false, + description: 'Array of image URLs to display', + control: { type: 'array' }, }, }, } satisfies Meta<typeof Avatar>;
27-32: Add realistic placeholder images and more story variants.The current story uses dummy values. Consider:
- Using realistic placeholder images
- Adding more variants (e.g., different sizes, overflow cases)
export const Default: Story = { args: { size: 'sm', - imageUrls: ['1', '2', '3', '4', '5', '6'], + imageUrls: [ + 'https://i.pravatar.cc/150?img=1', + 'https://i.pravatar.cc/150?img=2', + 'https://i.pravatar.cc/150?img=3', + 'https://i.pravatar.cc/150?img=4', + 'https://i.pravatar.cc/150?img=5', + 'https://i.pravatar.cc/150?img=6', + ], }, }; + +export const Large: Story = { + args: { + size: 'lg', + imageUrls: [ + 'https://i.pravatar.cc/150?img=1', + 'https://i.pravatar.cc/150?img=2', + ], + }, +};frontend/src/components/Avatar/AvatarCount.tsx (1)
24-25: Consider memoizing getTypo function.Since getTypo is a pure function, it could be memoized for performance optimization.
-const getTypo = (size: Size): Typo => - size === 'sm' ? 'caption' : 't3'; +const getTypo = React.useMemo( + (size: Size): Typo => size === 'sm' ? 'caption' : 't3', + [] +);frontend/src/components/Avatar/index.tsx (1)
14-19: Optimize performance with useMemo for limitedUrls.The URL limitation logic could be memoized to prevent unnecessary recalculations.
const Avatar = ({ size, imageUrls: prevUrls }: AvatarProps) => { - let limitedUrls = prevUrls; - if (prevUrls.length > MAX_IMAGE_COUNT) { - limitedUrls = prevUrls.slice(0, MAX_IMAGE_COUNT); - } + const limitedUrls = React.useMemo( + () => prevUrls.length > MAX_IMAGE_COUNT + ? prevUrls.slice(0, MAX_IMAGE_COUNT) + : prevUrls, + [prevUrls] + );frontend/src/components/Avatar/index.css.ts (2)
6-9: Consider adding gap control for better spacing flexibility.While the current flex setup works, consider adding a
gapproperty or allowing for customizable spacing between items when they're not overlapping.export const avatarContainerStyle = style({ display: 'flex', alignItems: 'center', + gap: vars.space[2], // Add default gap });
11-44: Consider enhancing the avatar style recipe.A few suggestions to improve the recipe:
- Move hard-coded values to theme variables for better consistency
- Add a medium size variant for more flexibility
- Use theme variables for border width
export const avatarItemStyle = recipe({ base: { backgroundColor: vars.color.Ref.Neutral['White'], borderRadius: vars.radius['Max'], - border: `2px solid ${vars.color.Ref.Netural['White']}`, + border: `${vars.borderWidth.md} solid ${vars.color.Ref.Neutral['White']}`, selectors: { '&:last-child': { - borderColor: vars.color.Ref.Netural[100], + borderColor: vars.color.Ref.Neutral[100], }, }, }, variants: { size: { sm: { - width: '28px', - height: '28px', + width: vars.space[7], // Assuming theme has appropriate spacing scale + height: vars.space[7], selectors: { '&:not(:first-child)': { - marginLeft: '-12px', + marginLeft: `calc(-1 * ${vars.space[3]})`, }, }, }, + md: { + width: vars.space[8], + height: vars.space[8], + selectors: { + '&:not(:first-child)': { + marginLeft: `calc(-1 * ${vars.space[4]})`, + }, + }, + }, lg: { - width: '42px', - height: '42px', + width: vars.space[10], + height: vars.space[10], selectors: { '&:not(:first-child)': { - marginLeft: '-14px', + marginLeft: `calc(-1 * ${vars.space[4]})`, }, }, }, }, }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
frontend/src/components/Avatar/Avatar.stories.tsx(1 hunks)frontend/src/components/Avatar/AvatarCount.tsx(1 hunks)frontend/src/components/Avatar/AvatarItem.tsx(1 hunks)frontend/src/components/Avatar/index.css.ts(1 hunks)frontend/src/components/Avatar/index.tsx(1 hunks)
🔇 Additional comments (1)
frontend/src/components/Avatar/index.css.ts (1)
1-4: Well-structured imports using vanilla-extract!Good choice using vanilla-extract for type-safe CSS and importing theme variables for consistent styling.
| const AvatarItem = ({ src, size, alt }: AvatarItemProps) => ( | ||
| <img | ||
| alt={alt} | ||
| className={avatarItemStyle({ size })} | ||
| src={src} | ||
| /> | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add image loading optimization and error handling.
Consider the following improvements:
- Add loading="lazy" for performance
- Add onError handler for fallback behavior
- Ensure meaningful alt text for accessibility
const AvatarItem = ({ src, size, alt }: AvatarItemProps) => (
<img
- alt={alt}
+ alt={alt || `Avatar image ${src}`}
className={avatarItemStyle({ size })}
+ loading="lazy"
+ onError={onError}
src={src}
/>
);Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
@hamo-o Thanks for confirming! I'm glad you find the suggested improvements helpful. Let me know if you need any assistance implementing these changes.
(\ (
(^.^)
c(")(")
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| return ( | ||
| <div className={avatarContainerStyle}> | ||
| {limitedUrls.map((url, index) => ( | ||
| <AvatarItem | ||
| alt='' | ||
| key={`${index}-${url}`} | ||
| size={size} | ||
| src={url} | ||
| /> | ||
| ))} | ||
| {prevUrls.length > MAX_IMAGE_COUNT && ( | ||
| <AvatarCount | ||
| count={prevUrls.length} | ||
| size={size} | ||
| /> | ||
| )} | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve accessibility and key prop.
Add aria-label to the container and use more reliable key prop.
return (
- <div className={avatarContainerStyle}>
+ <div
+ aria-label={`Avatar group with ${prevUrls.length} images`}
+ className={avatarContainerStyle}
+ role="group"
+ >
{limitedUrls.map((url, index) => (
<AvatarItem
alt=''
- key={`${index}-${url}`}
+ key={url}
size={size}
src={url}
/>📝 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.
| return ( | |
| <div className={avatarContainerStyle}> | |
| {limitedUrls.map((url, index) => ( | |
| <AvatarItem | |
| alt='' | |
| key={`${index}-${url}`} | |
| size={size} | |
| src={url} | |
| /> | |
| ))} | |
| {prevUrls.length > MAX_IMAGE_COUNT && ( | |
| <AvatarCount | |
| count={prevUrls.length} | |
| size={size} | |
| /> | |
| )} | |
| </div> | |
| ); | |
| return ( | |
| <div | |
| aria-label={`Avatar group with ${prevUrls.length} images`} | |
| className={avatarContainerStyle} | |
| role="group" | |
| > | |
| {limitedUrls.map((url, index) => ( | |
| <AvatarItem | |
| alt='' | |
| key={url} | |
| size={size} | |
| src={url} | |
| /> | |
| ))} | |
| {prevUrls.length > MAX_IMAGE_COUNT && ( | |
| <AvatarCount | |
| count={prevUrls.length} | |
| size={size} | |
| /> | |
| )} | |
| </div> | |
| ); |
| interface AvatarProps { | ||
| size: Size; | ||
| imageUrls: string[]; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding prop validation for imageUrls.
Add validation to ensure imageUrls is not empty and contains valid URLs.
interface AvatarProps {
size: Size;
- imageUrls: string[];
+ imageUrls: string[] & { length: number };
}Committable suggestion skipped: line range outside the PR's diff.
| export const Default: Story = { | ||
| args: { | ||
| size: 'sm', | ||
| imageUrls: ['1', '2', '3', '4', '5', '6'], |
There was a problem hiding this comment.
p3;
https://picsum.photos/
요런 mock url이라도 넣어주시면 확인이 편할 것 같습니다!!
| let limitedUrls = prevUrls; | ||
| if (prevUrls.length > MAX_IMAGE_COUNT) { | ||
| limitedUrls = prevUrls.slice(0, MAX_IMAGE_COUNT); | ||
| } |
There was a problem hiding this comment.
| let limitedUrls = prevUrls; | |
| if (prevUrls.length > MAX_IMAGE_COUNT) { | |
| limitedUrls = prevUrls.slice(0, MAX_IMAGE_COUNT); | |
| } | |
| const ENTIRE_LENGTH = imageUrls.length; | |
| const limitedUrls = imageUrls.slice(0, MAX_IMAGE_COUNT); |
p2;
불필요한 let 사용인 것 같습니다. 위와 같이 수정하면 어떨까요? 배열보다 큰 범위여도 원본 배열의 내용이 리턴될 것 같아요.
이후에 리턴문의 조건문에서는 ENTIRE_LENGTH > MAX_IMAGE_COUNT으로 쓸 수도 있고요!
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
frontend/src/components/Avatar/AvatarItem.tsx (1)
15-15: Consider making the fallback image URL configurable.The hardcoded fallback URL could be:
- Moved to a constants file or configuration
- Made more reliable by using a local asset or a stable external URL
- Passed as a prop to allow customization
Example implementation:
- const fallbackSrc = 'https://picsum.photos/id/200/200/200'; + const fallbackSrc = props.fallbackSrc || DEFAULT_AVATAR_FALLBACK;And in a constants file:
export const DEFAULT_AVATAR_FALLBACK = '/assets/default-avatar.png';frontend/src/components/Avatar/Avatar.stories.tsx (2)
5-21: Consider enhancing the component documentation and size options.The meta configuration is well-structured, but could benefit from:
- Adding a description field to document the component's purpose and usage
- Consider adding a 'md' (medium) size option for more flexibility
const meta: Meta = { title: 'Components/Avatar', component: Avatar, + parameters: { + docs: { + description: { + component: 'Avatar component displays user profile images with overflow handling.', + }, + }, + backgrounds: { default: 'dark', }, }, argTypes: { size: { - control: { type: 'radio', options: ['sm', 'lg'] }, + control: { type: 'radio', options: ['sm', 'md', 'lg'] }, }, imageUrls: { control: false, }, }, } satisfies Meta<typeof Avatar>;
41-51: Improve error state testing.The current approach using
hi.comis not ideal for testing fetch errors. Consider using:
- A real but non-existent image URL
- Different error scenarios (404, 500, etc.)
export const FetchError: Story = { args: { size: 'sm', imageUrls: [ - 'https://hi.com/', - 'https://hi.com/', - 'https://hi.com/', - 'https://hi.com/', + 'https://picsum.photos/404', // 404 Not Found + 'https://picsum.photos/500', // 500 Server Error + 'https://picsum.photos/invalid.jpg', // Invalid image + 'https://picsum.photos/broken.jpg', // Another invalid image ], }, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/components/Avatar/Avatar.stories.tsx(1 hunks)frontend/src/components/Avatar/AvatarItem.tsx(1 hunks)
🔇 Additional comments (5)
frontend/src/components/Avatar/AvatarItem.tsx (3)
1-4: LGTM! Imports are well-organized.The imports are correctly structured with React hooks and local imports properly separated.
6-10: LGTM! Props interface is well-defined.The interface is clean, properly typed, and follows TypeScript best practices.
24-30: Great job implementing the previous review suggestions!The component now includes all the recommended optimizations:
- Lazy loading for performance
- Error handling with fallback
- Meaningful alt text
frontend/src/components/Avatar/Avatar.stories.tsx (2)
25-25: LGTM!The Story type definition is correct and follows Storybook best practices.
27-39: Well implemented with mock image URLs!Good job implementing the suggestion to use picsum.photos with different IDs for unique mock images. This helps in testing both the regular display and overflow scenarios.
#️⃣ 연관된 이슈>
📝 작업 내용> 이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)
Avatar 컴포넌트의 기본적인 레이아웃을 구현했습니다.
🙏 여기는 꼭 봐주세요! > 리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요
Summary by CodeRabbit
New Features
Styling Enhancements