-
Notifications
You must be signed in to change notification settings - Fork 0
gamecard 컴포넌트 생성 #56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
gamecard 컴포넌트 생성 #56
Conversation
Walkthrough새로운 React 컴포넌트 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GameCard
participant DropdownMenu
User->>GameCard: 카드 렌더링 (props 전달)
GameCard->>GameCard: 변형별 UI 렌더링
alt myGame 변형
User->>GameCard: 옵션 버튼 클릭
GameCard->>DropdownMenu: 메뉴 표시
User->>DropdownMenu: 편집/공유/삭제 선택
DropdownMenu->>GameCard: 콜백 함수 실행 (onEdit/onShare/onDelete)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes Suggested labels
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (3)
shared/design/src/components/gameCard/index.tsx (2)
43-58: imageVariants에서 중복된 스타일 정의가 있습니다.
thumbnailVariants와imageVariants에서 동일한 스타일이 중복 정의되어 있습니다.스타일 중복을 줄이고 일관성을 개선할 수 있습니다:
const imageVariants = cva( - "shrink-0 rounded-[10px] bg-cover bg-center bg-no-repeat", + "shrink-0", { variants: { type: { libraryGame: "size-[178px]", myGame: "size-[178px]", gamePreview: "h-[260px] w-[178px]", onlyTitleGamePreview: "hidden", }, }, defaultVariants: { type: "libraryGame", }, }, )
170-205: renderTitle 함수에서 스타일 중복이 있습니다.
myGame타입에서 인라인 스타일과titleVariants에서 중복된 스타일이 정의되어 있습니다.일관성을 위해
titleVariants를 사용하는 것을 고려해보세요:{type === 'myGame' ? ( <> - <div className="line-clamp-2 w-[130px] shrink-0 overflow-hidden text-[19px] font-bold leading-[120%] text-text-primary"> + <div className={cn(titleVariants({ type }), "line-clamp-2")}> {title} </div> <DropdownMenuRoot>shared/design/src/stories/gameCard.stories.tsx (1)
212-212: 동일한 이미지 URL 사용에 대한 개선 제안입니다.모든 스토리에서 동일한 이미지를 사용하고 있어 시각적 다양성이 부족할 수 있습니다.
각 스토리별로 다른 이미지를 사용하여 시각적 다양성을 높이는 것을 고려해보세요:
export const LibraryGame: Story = { args: { type: "libraryGame", title: "수학 퀴즈 게임", questionCount: 10, - imageUrl: "https://picsum.photos/id/237/536/354", + imageUrl: "https://picsum.photos/id/1/536/354", }, } export const MyGame: Story = { args: { type: "myGame", title: "내가 만든 게임", questionCount: 15, - imageUrl: "https://picsum.photos/id/237/536/354", + imageUrl: "https://picsum.photos/id/2/536/354", optionView: false, }, }Also applies to: 221-221, 232-232, 248-248
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
shared/design/src/components/gameCard/index.tsx(1 hunks)shared/design/src/stories/gameCard.stories.tsx(1 hunks)
🔇 Additional comments (9)
shared/design/src/components/gameCard/index.tsx (5)
1-8: Import 구조가 잘 정리되어 있습니다.모든 필요한 의존성이 적절히 import되어 있고, 그룹핑도 논리적으로 잘 되어 있습니다.
26-41: thumbnailVariants가 적절하게 정의되었습니다.각 타입별로 적절한 크기와 레이아웃이 설정되어 있습니다.
onlyTitleGamePreview에서 썸네일을 숨기는 것도 타당합니다.
113-150: TypeScript 타입 정의가 훌륭합니다.discriminated union을 사용하여 각 변형별로 적절한 props를 강제하는 타입 안전성이 구현되어 있습니다.
never타입을 사용한 제약도 적절합니다.
177-197: 드롭다운 메뉴 구현이 잘 되어 있습니다.접근성을 위한
aria-label도 적절히 설정되어 있고, 각 메뉴 항목의 아이콘과 텍스트도 적절합니다.
207-217: 컴포넌트 전체 구조가 깔끔합니다.
forwardRef사용과 전체 레이아웃 구조가 적절하며, 고정된 높이 설정도 일관성 있게 처리되어 있습니다.shared/design/src/stories/gameCard.stories.tsx (4)
5-15: StorybookGameCardProps 타입 정의가 적절합니다.GameCard의 모든 props를 포함하여 Storybook에서 테스트할 수 있도록 잘 설계되어 있습니다.
17-181: Storybook 문서화가 매우 상세하고 유용합니다.한국어로 작성된 상세한 문서가 개발자들에게 매우 도움이 될 것입니다. 각 카드 타입별 특징, 사용법, 주의사항이 모두 잘 설명되어 있습니다.
183-202: argTypes 설정이 훌륭합니다.특히
optionView컨트롤을myGame타입에서만 표시하도록 하는 조건부 설정이 사용자 경험을 크게 개선합니다.
207-258: 다양한 시나리오를 포함한 포괄적인 스토리 세트입니다.모든 카드 타입, 긴 제목 처리, 이미지 없는 경우 등 실제 사용 시나리오를 잘 반영한 스토리들이 정의되어 있습니다.
| const gameCardVariants = cva( | ||
| "relative", | ||
| { | ||
| variants: { | ||
| type: { | ||
| libraryGame: "", | ||
| myGame: "", | ||
| gamePreview: "", | ||
| onlyTitleGamePreview: "", | ||
| }, | ||
| }, | ||
| defaultVariants: { | ||
| type: "libraryGame", | ||
| }, | ||
| }, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
gameCardVariants에서 실제 스타일이 누락되었습니다.
현재 모든 type 변형에 빈 문자열이 설정되어 있어 실제 차별화된 스타일이 적용되지 않습니다.
각 타입별로 필요한 스타일을 추가하는 것을 고려해보세요:
const gameCardVariants = cva(
"relative",
{
variants: {
type: {
- libraryGame: "",
- myGame: "",
- gamePreview: "",
- onlyTitleGamePreview: "",
+ libraryGame: "cursor-pointer hover:shadow-md transition-shadow",
+ myGame: "cursor-pointer hover:shadow-md transition-shadow",
+ gamePreview: "cursor-pointer hover:shadow-lg transition-shadow",
+ onlyTitleGamePreview: "cursor-pointer hover:shadow-sm transition-shadow",
},
},
defaultVariants: {
type: "libraryGame",
},
},
)📝 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.
| const gameCardVariants = cva( | |
| "relative", | |
| { | |
| variants: { | |
| type: { | |
| libraryGame: "", | |
| myGame: "", | |
| gamePreview: "", | |
| onlyTitleGamePreview: "", | |
| }, | |
| }, | |
| defaultVariants: { | |
| type: "libraryGame", | |
| }, | |
| }, | |
| ) | |
| const gameCardVariants = cva( | |
| "relative", | |
| { | |
| variants: { | |
| type: { | |
| libraryGame: "cursor-pointer hover:shadow-md transition-shadow", | |
| myGame: "cursor-pointer hover:shadow-md transition-shadow", | |
| gamePreview: "cursor-pointer hover:shadow-lg transition-shadow", | |
| onlyTitleGamePreview: "cursor-pointer hover:shadow-sm transition-shadow", | |
| }, | |
| }, | |
| defaultVariants: { | |
| type: "libraryGame", | |
| }, | |
| }, | |
| ) |
🤖 Prompt for AI Agents
In shared/design/src/components/gameCard/index.tsx between lines 9 and 24, the
gameCardVariants object defines variant types but assigns empty strings to all,
resulting in no actual styles being applied. To fix this, add the appropriate
CSS class strings or style definitions for each variant type (libraryGame,
myGame, gamePreview, onlyTitleGamePreview) so that each variant applies distinct
styles as intended.
| const renderThumbnail = () => ( | ||
| <div className={cn(thumbnailVariants({ type }), className)}> | ||
| <div | ||
| className={cn(imageVariants({ type }))} | ||
| style={{ backgroundImage: imageUrl ? `url(${imageUrl})` : undefined }} | ||
| /> | ||
| <div className={cn(badgeVariants({ type }))}> | ||
| <span className="text-[13px] font-normal leading-[120%] text-text-inverse"> | ||
| {questionCount}문제 | ||
| </span> | ||
| </div> | ||
| </div> | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
renderThumbnail 함수의 구조가 깔끔합니다.
하지만 이미지 로드 실패에 대한 fallback 처리가 누락되어 있습니다.
이미지 로드 실패 시 기본 이미지나 체커 패턴을 표시하는 로직을 추가하는 것을 고려해보세요:
const renderThumbnail = () => (
<div className={cn(thumbnailVariants({ type }), className)}>
<div
className={cn(imageVariants({ type }))}
- style={{ backgroundImage: imageUrl ? `url(${imageUrl})` : undefined }}
+ style={{
+ backgroundImage: imageUrl ? `url(${imageUrl})` : 'url(/assets/checker.svg)'
+ }}
/>
<div className={cn(badgeVariants({ type }))}>
<span className="text-[13px] font-normal leading-[120%] text-text-inverse">
{questionCount}문제
</span>
</div>
</div>
)📝 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.
| const renderThumbnail = () => ( | |
| <div className={cn(thumbnailVariants({ type }), className)}> | |
| <div | |
| className={cn(imageVariants({ type }))} | |
| style={{ backgroundImage: imageUrl ? `url(${imageUrl})` : undefined }} | |
| /> | |
| <div className={cn(badgeVariants({ type }))}> | |
| <span className="text-[13px] font-normal leading-[120%] text-text-inverse"> | |
| {questionCount}문제 | |
| </span> | |
| </div> | |
| </div> | |
| ) | |
| const renderThumbnail = () => ( | |
| <div className={cn(thumbnailVariants({ type }), className)}> | |
| <div | |
| className={cn(imageVariants({ type }))} | |
| style={{ | |
| backgroundImage: imageUrl ? `url(${imageUrl})` : 'url(/assets/checker.svg)' | |
| }} | |
| /> | |
| <div className={cn(badgeVariants({ type }))}> | |
| <span className="text-[13px] font-normal leading-[120%] text-text-inverse"> | |
| {questionCount}문제 | |
| </span> | |
| </div> | |
| </div> | |
| ) |
🤖 Prompt for AI Agents
In shared/design/src/components/gameCard/index.tsx around lines 156 to 168, the
renderThumbnail function lacks fallback handling for image load failures. Modify
the component to detect if the image fails to load and display a default image
or checker pattern instead. This can be done by adding an onError event handler
to the image element or managing a state flag to conditionally render the
fallback background when the imageUrl is invalid or fails to load.
whdgur5717
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
썸네일 이미지부분을 아예 next/image로 대체할지, 아니면 합성컴포넌트 방식으로 이미지부분만 Slot 을 사용할수도 있을것같은데 저는 next/image쓰는게 좋아보입니다
|
question 구현 때 디자이너 분께서 넘겨주신 asset을 next/image로 넣어뒀는데, 같은 방법으로 진행하도록 하겠습니다 |
📝 설명
GameCard 컴포넌트 개발 및 Storybook 문서화 작업을 완료했습니다.
🛠️ 주요 변경 사항
리뷰 시 고려해야 할 사항
Summary by CodeRabbit