-
Notifications
You must be signed in to change notification settings - Fork 0
gameCard 컴포넌트 리팩토링 #155
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 컴포넌트 리팩토링 #155
Conversation
… and optional callback
…or GameCard component
WalkthroughGameCard가 props 기반에서 합성(컴포지션) 기반 API로 전면 교체되었고, 이를 소비하는 페이지/위젯/스토리북이 모두 새로운 하위 컴포넌트 구조로 갱신되었습니다. 대시보드/라이브러리 뷰는 MyGameCard와 GameCardOptions를 도입해 편집/공유/삭제 액션을 포함하도록 변경되었으며, 일부 버튼의 포커스 링 스타일이 제거되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as 사용자
participant Grid as GameLibraryGrid
participant Card as MyGameCard/GameCard
participant Menu as GameCardOptions
User->>Grid: 카드 클릭/호버
Grid->>Card: isDashboard 분기 렌더링
alt 대시보드 뷰
Card->>Menu: 옵션 트리거(… 버튼)
User->>Menu: 편집/공유(또는 공유 취소)/삭제 클릭
Menu-->>Grid: onEdit/onShare/onDelete 콜백 호출
else 라이브러리 뷰
Note right of Card: 합성된 Image/Badge/Title/SharedBadge만 표시
end
sequenceDiagram
autonumber
actor User as 사용자
participant Section as GameSection
participant Card as GameCard
participant NextImg as next/image
User->>Section: 게임 목록 보기
loop 각 게임
Section->>Card: GameCard, .Image/.Badge/.SharedBadge/.Title로 구성
alt 썸네일 있음
Card->>NextImg: Image fill + blur placeholder
else 썸네일 없음
Card-->>Card: "이미지 없음" 플레이스홀더 렌더
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ 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). (1)
✨ 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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Deploy preview for re-creation ready! ✅ Preview Built with commit d8f2559. |
|
✅ CI 및 E2E 테스트 성공! 🚀 Preview URL: https://re-creation-dhqe9onlz-whdgur5717s-projects.vercel.app 모든 테스트가 통과했습니다. 코드 리뷰를 진행해주세요! |
|
✅ CI 및 E2E 테스트 성공! 🚀 Preview URL: https://re-creation-r1x95bzth-whdgur5717s-projects.vercel.app 모든 테스트가 통과했습니다. 코드 리뷰를 진행해주세요! |
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: 8
🧹 Nitpick comments (11)
shared/design/src/components/gameCard/gameCardOptions.tsx (1)
36-63: 테스트 용이성과 UX 개선 제안: 항목 data-testid 및 비활성화 처리
- E2E/RTL 테스트 편의를 위해 각 메뉴 항목에 고유 data-testid 추가를 제안합니다.
- 콜백이 전달되지 않은 항목은 disabled 처리(라이브러리가 지원한다면)를 권장합니다. 현재는 클릭 가능해 보이지만 아무 동작을 하지 않습니다.
- <DropdownMenuItem - type="icon" - onClick={stopAnd(onEdit)} - > + <DropdownMenuItem + type="icon" + onClick={stopAnd(onEdit)} + data-testid="game-options-edit" + disabled={!onEdit} + > ... - <DropdownMenuItem - type="icon" - onClick={stopAnd(onShare)} - > + <DropdownMenuItem + type="icon" + onClick={stopAnd(onShare)} + data-testid="game-options-share" + disabled={!onShare} + > ... - <DropdownMenuItem - type="icon" - onClick={stopAnd(onDelete)} - > + <DropdownMenuItem + type="icon" + onClick={stopAnd(onDelete)} + data-testid="game-options-delete" + disabled={!onDelete} + >shared/design/src/components/gameCard/gameCardImage.tsx (1)
76-83: 선택: 스켈레톤 접근성 표시 추가스켈레톤에
aria-busy="true"를 부여하면 보조기기 친화도가 올라갑니다.- <div + <div className={cn(skeletonVariants({ type }), "absolute inset-0")} data-testid="image-skeleton" + aria-busy="true" />shared/design/src/components/gameCard/myGameCard.tsx (5)
77-79: 제목/옵션 정렬이 우측으로 몰릴 수 있습니다 — justify-between으로 안정화 권장현재 titleContainerVariants가 justify-end라 제목과 옵션 버튼이 우측 정렬됩니다. 일반적인 카드 레이아웃(제목 좌측, 옵션 우측)을 위해 justify-between이 더 적합합니다.
-const titleContainerVariants = cva( - "flex h-[46px] w-[178px] items-start justify-end", +const titleContainerVariants = cva( + "flex h-[46px] w-[178px] items-start justify-between",
129-131: className가 썸네일에만 적용됩니다 — root/thumbnail 클래스 분리 제안외부에서 카드 전체 여백/정렬을 조정하려면 className이 루트에 적용되는 것이 일반적입니다. 현재는 썸네일 컨테이너에만 적용되어 사용성이 떨어집니다. rootClassName(=className), thumbnailClassName로 분리하는 리팩터를 제안합니다.
-type MyGameCardProps = { +type MyGameCardProps = { title: string questionCount: number imageUrl?: string - className?: string + className?: string // root용 + thumbnailClassName?: string // 썸네일용 shared?: boolean @@ - <div className={cn(thumbnailVariants({ type: "myGame" }), className)}> + <div className={cn(thumbnailVariants({ type: "myGame" }), thumbnailClassName)}> @@ - <div - ref={ref} - className={cn(myGameCardVariants({ type: "myGame" }))} - data-testid="game-card" - > + <div + ref={ref} + className={cn(myGameCardVariants({ type: "myGame" }), className)} + data-testid="game-card" + >
93-94: 불필요한 타입 export 가능성: MyGameCardVariantProps이 타입은 외부에서 사용되지 않는다면 내보내지 않는 편이 좋습니다. 유지할 경우 사용 예시(테마 확장 등)를 문서에 명시하는 것도 방법입니다.
115-186: forwardRef 디버깅 편의성 — displayName 지정 제안개발자 도구에서 컴포넌트명이 익명으로 표시될 수 있습니다. displayName을 지정해 디버깅 가독성을 높이세요.
export const MyGameCard = forwardRef<HTMLDivElement, MyGameCardProps>( (props, ref) => { @@ }, ) + +MyGameCard.displayName = "MyGameCard"
165-169: GameCardOptionsProps의shared는 Optional Boolean입니다
GameCardOptionsProps에서shared?: boolean으로 정의되어 있어, 현재 호출부에서shared={shared}로 전달해도 타입 오류는 없습니다. 다만, 컴포넌트 내부 로직이 항상 Boolean 값(true/false)을 기대한다면 아래 중 한 가지 방식을 적용해 안정성을 높일 것을 권장드립니다.
- 컴포넌트 정의 시 기본값 할당
export function GameCardOptions({- shared,
- shared = false,
onEdit,
onShare,
onDelete
}: GameCardOptionsProps) {
// …
}- 호출부에서 명시적 Boolean 변환 ```diff - <GameCardOptions shared={shared} onEdit={onEdit} onShare={onShare} onDelete={onDelete} /> + <GameCardOptions shared={!!shared} onEdit={onEdit} onShare={onShare} onDelete={onDelete} />
- 또는 prop 타입을
shared: boolean으로 변경해 필수화위 옵션 중 코드 컨벤션과 의도에 맞는 방식을 선택해 주세요.
shared/design/src/components/gameCard/index.tsx (4)
60-63: 잔존 myGame variant 정리 필요 (badgeVariants)GameCard에서 myGame 타입을 제거했는데 badgeVariants에는 myGame 키가 남아있습니다. 혼란을 줄이기 위해 제거하세요.
type: { libraryGame: "block", - myGame: "block", gamePreview: "hidden", onlyTitleGamePreview: "hidden", },
77-80: 잔존 myGame variant 정리 필요 (sharedBadgeVariants)sharedBadgeVariants에도 myGame 키가 남아있습니다. 일관성을 위해 제거하세요.
type: { libraryGame: "block", - myGame: "block", gamePreview: "hidden", onlyTitleGamePreview: "hidden", },
199-202: 타이틀 wrapper div의 중복 높이 제거renderTitle 내부 titleContainerVariants가 이미 h-[46px]을 부여합니다. 외부 래퍼의 h-[46px]은 중복이므로 제거해 마크업을 간결화하세요.
- <div className="flex flex-col gap-[14px]"> - {renderThumbnail()} - <div className="h-[46px]">{renderTitle()}</div> - </div> + <div className="flex flex-col gap-[14px]"> + {renderThumbnail()} + {renderTitle()} + </div>
121-126: className 적용 위치 개선 제안 — root/thumbnail 분리현재 className이 썸네일 컨테이너에만 적용됩니다. 카드 루트/썸네일 스타일을 분리하면 재사용성이 좋아집니다. (MyGameCard와 동일 제안)
type BaseGameCardProps = { title: string questionCount: number imageUrl?: string - className?: string + className?: string // root용 + thumbnailClassName?: string // 썸네일용 shared?: boolean ImageComponent?: React.ComponentType<{ @@ - <div className={cn(thumbnailVariants({ type }), className)}> + <div className={cn(thumbnailVariants({ type }), thumbnailClassName)}> @@ - className={cn(gameCardVariants({ type }))} + className={cn(gameCardVariants({ type }), className)}Also applies to: 158-159, 196-197
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
service/app/src/entities/game/ui/components/gameLibraryGrid.tsx(2 hunks)shared/design/src/components/gameCard/gameCardImage.tsx(1 hunks)shared/design/src/components/gameCard/gameCardOptions.tsx(1 hunks)shared/design/src/components/gameCard/hooks/useImageState.ts(1 hunks)shared/design/src/components/gameCard/index.tsx(4 hunks)shared/design/src/components/gameCard/myGameCard.tsx(1 hunks)shared/design/src/utils/eventHandlers.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Component names in code must use PascalCase
Files:
shared/design/src/components/gameCard/gameCardOptions.tsxshared/design/src/components/gameCard/gameCardImage.tsxservice/app/src/entities/game/ui/components/gameLibraryGrid.tsxshared/design/src/components/gameCard/myGameCard.tsxshared/design/src/components/gameCard/index.tsx
🧬 Code graph analysis (5)
shared/design/src/components/gameCard/gameCardOptions.tsx (1)
shared/design/src/utils/eventHandlers.ts (1)
stopAnd(1-6)
shared/design/src/components/gameCard/gameCardImage.tsx (2)
shared/design/src/components/gameCard/hooks/useImageState.ts (1)
useImageState(3-22)shared/design/src/utils/cn.ts (1)
cn(4-6)
service/app/src/entities/game/ui/components/gameLibraryGrid.tsx (2)
shared/design/src/components/gameCard/myGameCard.tsx (1)
MyGameCard(115-186)shared/design/src/components/gameCard/index.tsx (1)
GameCard(153-206)
shared/design/src/components/gameCard/myGameCard.tsx (3)
shared/design/src/utils/cn.ts (1)
cn(4-6)shared/design/src/components/gameCard/gameCardImage.tsx (1)
GameCardImage(56-97)shared/design/src/components/gameCard/gameCardOptions.tsx (1)
GameCardOptions(18-66)
shared/design/src/components/gameCard/index.tsx (2)
shared/design/src/utils/cn.ts (1)
cn(4-6)shared/design/src/components/gameCard/gameCardImage.tsx (1)
GameCardImage(56-97)
⏰ 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). (1)
- GitHub Check: ci
🔇 Additional comments (3)
service/app/src/entities/game/ui/components/gameLibraryGrid.tsx (2)
4-4: MyGameCard 분리 도입 LGTM대시보드 전용 카드로의 분리 방향 좋습니다. 의존성도 최소화되어 있습니다.
68-86: 옵션 클릭 시 카드 onClick 전파 차단 동작 확인 필요MyGameCard 내부 옵션(드롭다운) 클릭이 부모 div의 onClick(onGameClick)을 트리거하지 않는지 실제 동작 검증을 권장합니다. stopAnd를 트리거/메뉴 아이템에 부착했으나, 라이브러리의 이벤트 흐름에 따라 캡처 단계에서 예외가 있을 수 있습니다.
재현/확인 가이드:
- 시나리오: 카드 영역 클릭 시 onGameClick 호출되어야 함.
- 옵션 버튼/메뉴 항목(Edit/Share/Delete) 클릭 시 onGameClick 호출되지 않아야 함.
shared/design/src/components/gameCard/gameCardOptions.tsx (1)
18-66: 옵션 컴포넌트 구조/상태 분기 LGTM
- 공유 상태에 따른 라벨/아이콘 토글, stopAnd로 전파 차단 모두 적절합니다.
| ImageComponent?: React.ComponentType<{ | ||
| src: string | ||
| alt: string | ||
| className?: string | ||
| onLoad?: () => void | ||
| onError?: () => void | ||
| }> | ||
| } |
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
React 타입 네임스페이스 직접 참조 지양 — ComponentType를 명시 import
React.ComponentType 사용은 TS 설정에 따라 네임스페이스 해석 오류를 유발할 수 있습니다. 타입을 직접 import하여 의존성을 명확히 하세요.
-import { cva, type VariantProps } from "class-variance-authority"
+import { cva, type VariantProps } from "class-variance-authority"
+import type { ComponentType } from "react"
...
- ImageComponent?: React.ComponentType<{
+ ImageComponent?: ComponentType<{
src: string
alt: string
className?: string
onLoad?: () => void
onError?: () => void
}>📝 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.
| ImageComponent?: React.ComponentType<{ | |
| src: string | |
| alt: string | |
| className?: string | |
| onLoad?: () => void | |
| onError?: () => void | |
| }> | |
| } | |
| // at the top of shared/design/src/components/gameCard/gameCardImage.tsx | |
| import { cva, type VariantProps } from "class-variance-authority" | |
| import type { ComponentType } from "react" | |
| // … | |
| export interface GameCardImageProps { | |
| ImageComponent?: ComponentType<{ | |
| src: string | |
| alt: string | |
| className?: string | |
| onLoad?: () => void | |
| onError?: () => void | |
| }> | |
| } |
🤖 Prompt for AI Agents
In shared/design/src/components/gameCard/gameCardImage.tsx around lines 47 to
54, avoid referencing the React namespace directly for types; replace
React.ComponentType with a direct type import: add an import type {
ComponentType } from 'react' at the top of the file and change the
ImageComponent prop type to use ComponentType< { src: string; alt: string;
className?: string; onLoad?: () => void; onError?: () => void } > so the type is
explicitly imported and namespace resolution issues are prevented.
| export const GameCardImage = ({ imageUrl, title, type, className }: GameCardImageProps) => { | ||
| const { imageLoading, imageError, handleLoad, handleError } = useImageState() | ||
|
|
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
useImageState에 imageUrl 전달 누락
위 훅은 src 변경 시 상태 초기화를 위해 src 인자를 필요로 합니다(제안된 훅 수정안 기준). imageUrl을 인자로 전달하세요.
-export const GameCardImage = ({ imageUrl, title, type, className }: GameCardImageProps) => {
- const { imageLoading, imageError, handleLoad, handleError } = useImageState()
+export const GameCardImage = ({ imageUrl, title, type, className, ImageComponent }: GameCardImageProps) => {
+ const { imageLoading, imageError, handleLoad, handleError } = useImageState(imageUrl)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In shared/design/src/components/gameCard/gameCardImage.tsx around lines 56 to
58, the useImageState hook is called without the imageUrl argument even though
the hook requires the src to reset state on src changes; update the call to pass
imageUrl (e.g., const { imageLoading, imageError, handleLoad, handleError } =
useImageState(imageUrl)) so the hook receives the current image URL and can
reinitialize state when it changes.
| <img | ||
| src={imageUrl} | ||
| alt={title} | ||
| className={cn( | ||
| "size-full rounded-[10px] object-cover transition-opacity duration-500 ease-in-out", | ||
| imageLoading ? "opacity-0" : "opacity-100" | ||
| )} | ||
| onLoad={handleLoad} | ||
| onError={handleError} | ||
| /> |
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.
ImageComponent prop이 무시되고 있습니다 — 커스텀 이미지 컴포넌트를 실제로 렌더링해야 합니다
현재 항상 로 렌더링하고 있어, 상위에서 전달한 ImageComponent가 전혀 사용되지 않습니다. 이는 기능적 버그입니다.
아래와 같이 커스텀 컴포넌트를 우선 사용하고, 기본 img에는 성능 속성(lazy, decoding)도 추가하세요.
- <img
- src={imageUrl}
- alt={title}
- className={cn(
- "size-full rounded-[10px] object-cover transition-opacity duration-500 ease-in-out",
- imageLoading ? "opacity-0" : "opacity-100"
- )}
- onLoad={handleLoad}
- onError={handleError}
- />
+ {ImageComponent ? (
+ <ImageComponent
+ src={imageUrl!}
+ alt={title}
+ className={cn(
+ "size-full rounded-[10px] object-cover transition-opacity duration-500 ease-in-out",
+ imageLoading ? "opacity-0" : "opacity-100"
+ )}
+ onLoad={handleLoad}
+ onError={handleError}
+ />
+ ) : (
+ <img
+ src={imageUrl!}
+ alt={title}
+ loading="lazy"
+ decoding="async"
+ className={cn(
+ "size-full rounded-[10px] object-cover transition-opacity duration-500 ease-in-out",
+ imageLoading ? "opacity-0" : "opacity-100"
+ )}
+ onLoad={handleLoad}
+ onError={handleError}
+ />
+ )}📝 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.
| <img | |
| src={imageUrl} | |
| alt={title} | |
| className={cn( | |
| "size-full rounded-[10px] object-cover transition-opacity duration-500 ease-in-out", | |
| imageLoading ? "opacity-0" : "opacity-100" | |
| )} | |
| onLoad={handleLoad} | |
| onError={handleError} | |
| /> | |
| {ImageComponent ? ( | |
| <ImageComponent | |
| src={imageUrl!} | |
| alt={title} | |
| className={cn( | |
| "size-full rounded-[10px] object-cover transition-opacity duration-500 ease-in-out", | |
| imageLoading ? "opacity-0" : "opacity-100" | |
| )} | |
| onLoad={handleLoad} | |
| onError={handleError} | |
| /> | |
| ) : ( | |
| <img | |
| src={imageUrl!} | |
| alt={title} | |
| loading="lazy" | |
| decoding="async" | |
| className={cn( | |
| "size-full rounded-[10px] object-cover transition-opacity duration-500 ease-in-out", | |
| imageLoading ? "opacity-0" : "opacity-100" | |
| )} | |
| onLoad={handleLoad} | |
| onError={handleError} | |
| /> | |
| )} |
🤖 Prompt for AI Agents
shared/design/src/components/gameCard/gameCardImage.tsx around lines 85 to 94:
the component always renders a plain <img> and ignores the passed ImageComponent
prop; update the render so it uses the provided ImageComponent when present
(passing src, alt, className, onLoad, onError and any other relevant props) and
falls back to a native <img> only when no ImageComponent is supplied; for the
native fallback include performance attrs loading="lazy" and decoding="async"
and preserve the existing className and handlers so imageLoading opacity
behavior still works.
| export const useImageState = () => { | ||
| const [imageLoading, setImageLoading] = useState(true) | ||
| const [imageError, setImageError] = useState(false) | ||
|
|
||
| const handleLoad = useCallback(() => { | ||
| setImageLoading(false) | ||
| }, []) | ||
|
|
||
| const handleError = useCallback(() => { | ||
| setImageLoading(false) | ||
| setImageError(true) | ||
| }, []) |
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
이미지 URL 변경 시 상태 초기화 누락으로 스켈레톤/에러 상태가 갱신되지 않습니다
현재 훅은 최초 렌더 이후 src가 바뀌어도 imageLoading/ imageError를 리셋하지 않아, 새 이미지에 대해 스켈레톤이 다시 보이지 않거나 에러 상태가 지속될 수 있습니다. src를 인자로 받아 변경 시 상태를 초기화하도록 수정하는 것을 권장합니다.
다음 패치를 적용해 주세요:
-import { useCallback, useState } from "react"
+import { useCallback, useEffect, useState } from "react"
-export const useImageState = () => {
- const [imageLoading, setImageLoading] = useState(true)
+export const useImageState = (src?: string) => {
+ const [imageLoading, setImageLoading] = useState<boolean>(!!src)
const [imageError, setImageError] = useState(false)
const handleLoad = useCallback(() => {
setImageLoading(false)
}, [])
const handleError = useCallback(() => {
setImageLoading(false)
setImageError(true)
}, [])
+ useEffect(() => {
+ // 새 이미지가 주어지면 로딩 재시작, 에러 초기화
+ setImageLoading(!!src)
+ setImageError(false)
+ }, [src])
+
return {
imageLoading,
imageError,
handleLoad,
handleError,
}
}Also applies to: 16-22
🤖 Prompt for AI Agents
In shared/design/src/components/gameCard/hooks/useImageState.ts around lines 3
to 14 (also applies to lines 16-22), the hook doesn't reset
imageLoading/imageError when the image src changes; update the hook to accept a
src parameter and add a useEffect that watches src and calls
setImageLoading(true) and setImageError(false) whenever src changes so new
images show the skeleton and clear prior error state; keep existing
handleLoad/handleError callbacks unchanged but ensure they remain stable via
useCallback and that the effect lists src in its dependency array.
| <GameCardImage | ||
| imageUrl={imageUrl} | ||
| title={title} | ||
| type={type} | ||
| ImageComponent={ImageComponent} | ||
| /> |
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.
💡 Verification agent
🧩 Analysis chain
GameCardImage에 ImageComponent 전달 — 구현과 타입 정의 일치 여부 확인 필요
ImageComponent를 전달하지만 gameCardImage.tsx가 해당 prop을 수신/사용하는지 확인이 필요합니다. 미수신 시 타입 오류 또는 커스텀 이미지 컴포넌트가 적용되지 않습니다.
검증 스크립트:
필요 시 조치:
- GameCardImageProps에 ImageComponent 추가
- 이미지 렌더링 시 img 대신 전달된 ImageComponent를 조건부 사용
🏁 Script executed:
#!/bin/bash
# GameCardImage가 ImageComponent를 props로 받는지/사용하는지 확인
rg -n -C3 'type\s+GameCardImageProps' shared/design/src/components/gameCard -g '*.ts*'
rg -n -C3 'export\s+const\s+GameCardImage' shared/design/src/components/gameCard -g '*.ts*'
rg -n 'ImageComponent' shared/design/src/components/gameCard -g '*.ts*'Length of output: 2186
GameCardImage 컴포넌트에 ImageComponent prop 수신 및 사용 로직 추가 필요
GameCardImageProps 타입 정의에는 ImageComponent가 포함되어 있지만, 실제 컴포넌트 시그니처와 렌더링 로직에서는 이 prop을 꺼내거나 사용하지 않고 있습니다. 따라서 외부에서 전달한 커스텀 이미지 컴포넌트가 적용되지 않습니다. 아래 위치를 참고해 수정하세요:
• shared/design/src/components/gameCard/gameCardImage.tsx
type GameCardImageProps블록에ImageComponent?: React.ComponentType<React.ImgHTMLAttributes<HTMLImageElement>>추가export const GameCardImage = ({ … }: GameCardImageProps)시그니처에ImageComponent포함- 이미지 렌더링 부분에서 기본
<img>대신ImageComponent를 조건부로 사용
예시 제안 diff:
--- a/shared/design/src/components/gameCard/gameCardImage.tsx
+++ b/shared/design/src/components/gameCard/gameCardImage.tsx
@@ -42,6 +42,7 @@ type GameCardImageProps = {
imageUrl?: string
title: string
type: GameCardImageVariantProps["type"]
+ ImageComponent?: React.ComponentType<React.ImgHTMLAttributes<HTMLImageElement>>
className?: string
}
@@ -56,7 +57,7 @@ export const GameCardImage = ({ imageUrl, title, type, className }: GameCardI
-export const GameCardImage = ({ imageUrl, title, type, className }: GameCardImageProps) => {
+export const GameCardImage = ({ imageUrl, title, type, className, ImageComponent }: GameCardImageProps) => {
const { imageLoading, imageError, handleLoad, handleError } = useImageState()
if (!imageUrl || imageError) {
@@ -63,7 +64,12 @@ export const GameCardImage = ({ imageUrl, title, type, className, ImageComp
return (
<div className={cx("placeholder", className)} data-loading={imageLoading}>
{/* placeholder */}
- <img src="" alt={`No image for ${title}`} />
+ <img
+ src=""
+ alt={`No image for ${title}`}
+ className="placeholder-img"
+ />
</div>
)
}
@@ -74,7 +80,14 @@ export const GameCardImage = ({ imageUrl, title, type, className, ImageComp
className={cx("image", type, className)}
data-loading={imageLoading}
>
- <img src={imageUrl} alt={title} onLoad={handleLoad} onError={handleError} />
+ {ImageComponent ? (
+ <ImageComponent
+ src={imageUrl}
+ alt={title}
+ onLoad={handleLoad}
+ onError={handleError}
+ className="custom-image"
+ />
+ ) : (
+ <img src={imageUrl} alt={title} onLoad={handleLoad} onError={handleError} />
+ )}
</figure>
)📝 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.
| <GameCardImage | |
| imageUrl={imageUrl} | |
| title={title} | |
| type={type} | |
| ImageComponent={ImageComponent} | |
| /> | |
| // shared/design/src/components/gameCard/gameCardImage.tsx | |
| type GameCardImageProps = { | |
| imageUrl?: string | |
| title: string | |
| type: GameCardImageVariantProps["type"] | |
| ImageComponent?: React.ComponentType<React.ImgHTMLAttributes<HTMLImageElement>> | |
| className?: string | |
| } | |
| export const GameCardImage = ({ | |
| imageUrl, | |
| title, | |
| type, | |
| className, | |
| ImageComponent, | |
| }: GameCardImageProps) => { | |
| const { imageLoading, imageError, handleLoad, handleError } = useImageState() | |
| if (!imageUrl || imageError) { | |
| return ( | |
| <div className={cx("placeholder", className)} data-loading={imageLoading}> | |
| {/* placeholder */} | |
| <img | |
| src="" | |
| alt={`No image for ${title}`} | |
| className="placeholder-img" | |
| /> | |
| </div> | |
| ) | |
| } | |
| return ( | |
| <figure | |
| className={cx("image", type, className)} | |
| data-loading={imageLoading} | |
| > | |
| {ImageComponent ? ( | |
| <ImageComponent | |
| src={imageUrl} | |
| alt={title} | |
| onLoad={handleLoad} | |
| onError={handleError} | |
| className="custom-image" | |
| /> | |
| ) : ( | |
| <img | |
| src={imageUrl} | |
| alt={title} | |
| onLoad={handleLoad} | |
| onError={handleError} | |
| /> | |
| )} | |
| </figure> | |
| ) | |
| } |
🤖 Prompt for AI Agents
In shared/design/src/components/gameCard/index.tsx around lines 159-164 and in
shared/design/src/components/gameCard/gameCardImage.tsx, add handling for the
ImageComponent prop: update the GameCardImageProps type to include
ImageComponent?: React.ComponentType<React.ImgHTMLAttributes<HTMLImageElement>>,
include ImageComponent in the GameCardImage function signature, and change the
render so the image uses the provided ImageComponent when present (render
<ImageComponent {...imageProps} />) and falls back to a native <img
{...imageProps} /> when not.
| <GameCardImage | ||
| imageUrl={imageUrl} | ||
| title={title} | ||
| type="myGame" | ||
| ImageComponent={ImageComponent} | ||
| /> |
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.
💡 Verification agent
🧩 Analysis chain
GameCardImage에 ImageComponent 전달 — 실제로 수신·사용되는지 확인 필요
이 파일과 GameCard(index.tsx)에서 모두 ImageComponent를 전달하지만, gameCardImage.tsx가 해당 prop을 수신/반영하는지 확인이 필요합니다. 수신하지 않으면 TS 오류 혹은 동작 불일치가 발생합니다.
검증 스크립트:
예상 처리:
- GameCardImageProps에 ImageComponent가 없다면, Props에 추가하고 img 대신 전달된 ImageComponent를 조건부로 렌더링하도록 수정 필요.
🏁 Script executed:
#!/bin/bash
# gameCardImage 컴포넌트가 ImageComponent를 props로 받고 사용하는지 확인
rg -n -C3 'export\s+const\s+GameCardImage' shared/design/src/components/gameCard -g '*.ts*'
rg -n -C2 'type\s+GameCardImageProps' shared/design/src/components/gameCard -g '*.ts*'
rg -n 'ImageComponent' shared/design/src/components/gameCard -g '*.ts*'Length of output: 2025
GameCardImage에 ImageComponent prop 반영 필요
GameCardImageProps 타입에는 이미 ImageComponent?: React.ComponentType<…>가 선언되어 있지만, GameCardImage 컴포넌트의 props destructuring 목록에 포함되지 않아 실제로 전달된 커스텀 이미지 컴포넌트가 사용되지 않습니다. 아래 사항을 반영해주세요:
• 수정 위치
- shared/design/src/components/gameCard/gameCardImage.tsx
export const GameCardImage = ({ … }: GameCardImageProps)부분에ImageComponent추가- 렌더링 로직에서
ImageComponent가 전달된 경우 이를, 그렇지 않을 경우 기본<img>를 사용하도록 변경
• 예시 Diff
-export const GameCardImage = ({ imageUrl, title, type, className }: GameCardImageProps) => {
+export const GameCardImage = ({ imageUrl, title, type, className, ImageComponent }: GameCardImageProps) => {
const { imageLoading, imageError, handleLoad, handleError } = useImageState()
if (!imageUrl || imageError) {
// 에러 또는 로딩 처리
}
- return (
- <img
- src={imageUrl}
- alt={title}
- className={cn(imageVariants[type], className)}
- onLoad={handleLoad}
- onError={handleError}
- />
- )
+ const Img = ImageComponent ?? 'img'
+ return (
+ <Img
+ src={imageUrl}
+ alt={title}
+ className={cn(imageVariants[type], className)}
+ onLoad={handleLoad}
+ onError={handleError}
+ />
+ )이렇게 반영하면 myGameCard.tsx(라인 135)와 index.tsx(라인 163)에서 전달된 ImageComponent prop이 정상적으로 동작합니다.
📝 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.
| <GameCardImage | |
| imageUrl={imageUrl} | |
| title={title} | |
| type="myGame" | |
| ImageComponent={ImageComponent} | |
| /> | |
| // shared/design/src/components/gameCard/gameCardImage.tsx | |
| export const GameCardImage = ({ | |
| imageUrl, | |
| title, | |
| type, | |
| className, | |
| ImageComponent, // ← add ImageComponent here | |
| }: GameCardImageProps) => { | |
| const { imageLoading, imageError, handleLoad, handleError } = useImageState(); | |
| if (!imageUrl || imageError) { | |
| // 에러 또는 로딩 처리 | |
| return null; | |
| } | |
| // use the custom component if provided, otherwise fall back to the native <img> | |
| const Img = ImageComponent ?? 'img'; | |
| return ( | |
| <Img | |
| src={imageUrl} | |
| alt={title} | |
| className={cn(imageVariants[type], className)} | |
| onLoad={handleLoad} | |
| onError={handleError} | |
| /> | |
| ); | |
| }; |
🤖 Prompt for AI Agents
In shared/design/src/components/gameCard/gameCardImage.tsx around the export
const GameCardImage = ({ ... }) block, the ImageComponent prop from
GameCardImageProps is not included in the destructured props and thus never
used; update the destructuring to accept ImageComponent, keep the prop type as
declared, and modify the render logic so that if ImageComponent is provided you
render it (passing the same imageUrl/title/alt props as needed), otherwise fall
back to the default <img> element; ensure required props passed to the custom
component match the existing image usage and preserve existing
styles/attributes.
| /> | ||
| )} | ||
|
|
||
| <img |
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를안쓰는이유가 있나요?
lazy loading이나, skeleton같은 처리도 쉬울것같고 최적화도 알아서해줘서 성능면에서는 더 좋을것같습니다.
그리고 사실 카드 컴포넌트라는 로직이 어려울 필요가 없는데 저 variant에 대응하느라 복잡해지는것같은데
저는 그냥 피그마의 variant는 신경쓰지말고 저희가 사용하기 편하게 만드는게 더 좋아보입니다.
next/image로 바꾸면 현재 이미지에서 고려하고 있는것들 대부분을 props로 해결할수 있을것같고,
합성 컴포넌트로 해서 Card.image / Card.title / ?(아이콘있는곳) 으로 나누고, 각자 조합해서 쓰는 방식으로 가는게 더 좋을것같습니다
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가 아직 안 들어갔습니다. 아직 방법을 고민 중이었다보니 이상태로 pr이 올라갔네요... 전에 navigation 구현했던 것처럼 밖에서 next/image 주입하는 방식으로 바꿔 보겠습니다
나머지 부분도 figma 신경 안 쓰고 조금 더 커스텀해보겠습니다
|
✅ CI 및 E2E 테스트 성공! 🚀 Preview URL: https://re-creation-od3ty3umt-whdgur5717s-projects.vercel.app 모든 테스트가 통과했습니다. 코드 리뷰를 진행해주세요! |
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: 1
♻️ Duplicate comments (3)
shared/design/src/utils/eventHandlers.ts (1)
1-8: React 네임스페이스 제거로 TS 오류 가능성 해소 — 깔끔합니다
.ts파일에서React.MouseEvent대신import type { MouseEvent } from "react"를 사용해 네임스페이스 의존을 제거했습니다. 유틸의 목적(버블링 차단 후 콜백 실행)도 명확합니다. LGTM.shared/design/src/components/gameCard/myGameCard.tsx (1)
5-8: 옵션 관련 불필요 props 제거 정합성 확인 — 👍PR 목표대로
optionView/onMoreClick등이 타입에서 제거된 상태로 보입니다. API 표면이 정리되어 사용성이 개선되었습니다.shared/design/src/components/gameCard/index.tsx (1)
29-39: 이전 코멘트(이미지 컴포넌트 주입) 관련 정리과거 PR 논의에서
ImageComponentprop 주입을 제안했으나, 현재 구조는GameCard.Image가 단순 컨테이너를 제공하고 소비자가 Next/Image 등을 직접 주입하는 합성 패턴으로 전환되었습니다. 이 방향이라면 별도의ImageComponentprop은 불필요합니다. 문서/스토리에서 이 의도를 명확히 유지하면 충분합니다.
🧹 Nitpick comments (15)
shared/design/src/utils/eventHandlers.ts (1)
3-8: 콜백에 이벤트 전달 + prevent 버전 유틸 추가를 제안합니다
- 앞으로 콜백에서 이벤트 객체를 필요로 할 수 있으니 이벤트를 전달하도록 타입/호출을 확장하는 것을 권장합니다.
- 클릭뿐 아니라 기본 동작까지 막아야 하는 케이스를 위해
stopAndPrevent유틸을 함께 제공하면 재사용성이 높아집니다.적용 diff:
-import type { MouseEvent } from "react" +import type { MouseEvent } from "react" export function stopAnd(fn?: () => void) { - return (e: MouseEvent) => { - e.stopPropagation(); - fn?.(); - }; + return (e: MouseEvent) => { + e.stopPropagation() + fn?.() + } } + +// 옵션: 이벤트를 콜백에 전달하도록 시그니처 확장 +export function stopAndWithEvent(fn?: (e: MouseEvent) => void) { + return (e: MouseEvent) => { + e.stopPropagation() + fn?.(e) + } +} + +// 옵션: 기본 동작까지 차단하는 유틸 +export function stopAndPrevent(fn?: (e: MouseEvent) => void) { + return (e: MouseEvent) => { + e.stopPropagation() + e.preventDefault() + fn?.(e) + } +}shared/design/src/components/gameCard/myGameCard.tsx (2)
5-8: Props를 GameCard에 패스스루하도록 확장하면 재사용성이 좋아집니다현재는
className만 전달됩니다.onClick,data-*,aria-*등 추가 속성을 그대로 넘길 수 있도록 GameCard의 props를 상속받는 형태를 권장합니다. 또한React.ReactNode대신 타입 임포트를 사용해 네임스페이스 의존을 줄일 수 있습니다.적용 diff:
-import { forwardRef } from "react" +import { forwardRef, type ComponentPropsWithoutRef, type ReactNode } from "react" import { GameCard } from "./index" -type MyGameCardProps = { - children: React.ReactNode - className?: string -} +export type MyGameCardProps = Omit< + ComponentPropsWithoutRef<typeof GameCard>, + "children" +> & { + children: ReactNode +}
10-20: forwardRef 래퍼 내부에서 잔여 props를 그대로 위임하세요잔여 props를
GameCard로 전달하면 API가 더 유연해집니다(예: data-testid, onClick 등).적용 diff:
export const MyGameCard = forwardRef<HTMLDivElement, MyGameCardProps>( (props, ref) => { - const { children, className } = props + const { children, className, ...rest } = props return ( - <GameCard ref={ref} className={className}> + <GameCard ref={ref} className={className} {...rest}> {children} </GameCard> ) }, )service/app/src/entities/game/ui/components/gamePreview.tsx (2)
97-105: 중복된 blurDataURL 상수화 제안여러 파일에서 동일한 base64 blur SVG가 반복됩니다. 상수로 추출해 재사용하면 유지보수가 쉬워집니다.
적용 diff(이 파일 내 치환):
- placeholder="blur" - blurDataURL="data:image/svg+xml;base64,PHN2ZyB3aWR0aD0iNzI3IiBoZWlnaHQ9IjQ1OSIgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnIj48cmVjdCB3aWR0aD0iMTAwJSIgaGVpZ2h0PSIxMDAlIiBmaWxsPSIjZTVlN2ViIi8+PC9zdmc+" + placeholder="blur" + blurDataURL={BLUR_DATA_URL}파일 상단에 다음 상수를 추가하세요(비-diff 참고):
const BLUR_DATA_URL = "data:image/svg+xml;base64,PHN2ZyB3aWR0aD0iNzI3IiBoZWlnaHQ9IjQ1OSIgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnIj48cmVjdCB3aWR0aD0iMTAwJSIgaGVpZ2h0PSIxMDAlIiBmaWxsPSIjZTVlN2ViIi8+PC9zdmc+"
93-94: 사소한 정리: 사용하지 않는 인덱스 파라미터 제거 가능
map((question, _index) => …)에서 인덱스를 사용하지 않으므로 파라미터를 생략해도 됩니다.service/app/src/entities/game/ui/components/gameLibraryGrid.tsx (3)
66-69: 클릭 가능한 div의 접근성 개선(키보드/스크린리더)현재 카드 래퍼가
div+onClick만으로 구성되어 키보드 접근이 어렵습니다. 내부에 옵션 버튼이 있어 래퍼를button으로 바꾸긴 어렵지만, role/keyboard 처리를 추가하면 안전하게 접근성을 향상할 수 있습니다. 드롭다운 버튼과의 이벤트 충돌을 피하기 위해currentTarget === target가드를 둡니다.적용 diff:
- <div - key={game.gameId} - onClick={() => onGameClick?.(game)} - className="cursor-pointer" - > + <div + key={game.gameId} + onClick={() => onGameClick?.(game)} + onKeyDown={(e) => { + if ((e.key === "Enter" || e.key === " ") && e.currentTarget === e.target) { + e.preventDefault() + onGameClick?.(game) + } + }} + role="button" + tabIndex={0} + aria-label={`${game.gameTitle} 카드 열기`} + className="cursor-pointer" + >
70-111: 대시보드 분기: 옵션 분리와 클릭 가드가 의도대로 동작 — 👍 / 핸들러 전달 단순화 제안
GameCardOptions+stopAnd로 부모 클릭 차단이 잘 적용되어 있습니다. LGTM.- 다만
onEdit={() => onEditGame?.(game)}처럼 항상 함수를 생성하기보다, 존재할 때만 핸들러를 전달하면 상태에 따라 비활성화 처리가 쉬워집니다.적용 diff:
- <GameCardOptions - shared={game.isShared} - onEdit={() => onEditGame?.(game)} - onShare={() => onShareGame?.(game)} - onDelete={() => onDeleteGame?.(game)} - /> + <GameCardOptions + shared={game.isShared} + onEdit={onEditGame ? () => onEditGame(game) : undefined} + onShare={onShareGame ? () => onShareGame(game) : undefined} + onDelete={onDeleteGame ? () => onDeleteGame(game) : undefined} + />
70-139: 중복된 썸네일 블록과 blurDataURL 상수 재사용두 분기(대시보드/일반)에서 썸네일 렌더링이 거의 동일합니다. 다음을 권장합니다:
- 공통
BLUR_DATA_URL상수로 치환- 선택: 작은
Thumbnail컴포넌트로 추출해 중복 제거상수 치환 diff(두 곳 모두):
- placeholder="blur" - blurDataURL="data:image/svg+xml;base64,PHN2ZyB3aWR0aD0iNzI3IiBoZWlnaHQ9IjQ1OSIgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnIj48cmVjdCB3aWR0aD0iMTAwJSIgaGVpZ2h0PSIxMDAlIiBmaWxsPSIjZTVlN2ViIi8+PC9zdmc+" + placeholder="blur" + blurDataURL={BLUR_DATA_URL}파일 상단(임포트 아래)에 상수 추가(비-diff 참고):
const BLUR_DATA_URL = "data:image/svg+xml;base64,PHN2ZyB3aWR0aD0iNzI3IiBoZWlnaHQ9IjQ1OSIgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnIj48cmVjdCB3aWR0aD0iMTAwJSIgaGVpZ2h0PSIxMDAlIiBmaWxsPSIjZTVlN2ViIi8+PC9zdmc+"선택적 추출 예시(비-diff 참고):
function Thumbnail({ src, alt }: { src?: string; alt: string }) { return src ? ( <Image src={src} alt={alt} fill className="rounded-[10px] object-cover" sizes="178px" placeholder="blur" blurDataURL={BLUR_DATA_URL} /> ) : ( <div className="flex size-full items-center justify-center rounded-[10px] bg-gray-200"> <span className="text-[14px] font-medium text-gray-500">이미지 없음</span> </div> ) }service/app/src/widgets/GameSection.tsx (2)
83-91: button에 대한 커스텀 키보드 핸들러는 불필요하며 중복 호출 리스크가 있습니다
button은 Enter/Space로 자동 클릭됩니다. 현재 구현은 keydown 시 직접 호출 + 기본 클릭(keyup) 간 중복을 예방하려고preventDefault를 쓰고 있는데, 불필요한 복잡성입니다. 핸들러를 제거하고onClick만으로 처리하세요.적용 diff:
- const handleGameCardKeyDown = ( - event: React.KeyboardEvent, - game: GameListItem, - ) => { - if (event.key === "Enter" || event.key === " ") { - event.preventDefault() - handleGameCardClick(game) - } - } + // 키보드 접근성: button의 기본 동작(Enter/Space)이 click을 트리거하므로 별도 핸들러 불필요버튼 사용처에서
onKeyDown제거:- onClick={() => handleGameCardClick(game)} - onKeyDown={(e) => handleGameCardKeyDown(e, game)} + onClick={() => handleGameCardClick(game)}
185-211: 썸네일 blurDataURL 상수화 및 재사용을 권장합니다여기서도 동일 base64 문자열이 반복됩니다. 상수로 추출해 사용하면 유지보수가 용이합니다. 또한 첫 화면의 퍼스트 뷰 항목에만
priority를 부여한 점은 적절합니다.적용 diff:
- placeholder="blur" - blurDataURL="data:image/svg+xml;base64,PHN2ZyB3aWR0aD0iNzI3IiBoZWlnaHQ9IjQ1OSIgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnIj48cmVjdCB3aWR0aD0iMTAwJSIgaGVpZ2h0PSIxMDAlIiBmaWxsPSIjZTVlN2ViIi8+PC9zdmc+" + placeholder="blur" + blurDataURL={BLUR_DATA_URL}파일 상단에 상수 추가(비-diff 참고):
const BLUR_DATA_URL = "data:image/svg+xml;base64,PHN2ZyB3aWR0aD0iNzI3IiBoZWlnaHQ9IjQ1OSIgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnIj48cmVjdCB3aWR0aD0iMTAwJSIgaGVpZ2h0PSIxMDAlIiBmaWxsPSIjZTVlN2ViIi8+PC9zdmc+"shared/design/src/components/gameCard/index.tsx (4)
29-36: GameCard.Image의 배경 관련 클래스 정리 제안
bg-cover bg-center bg-no-repeat는 실제 이미지가 자식으로 들어오는 구조(Next/Image 등)에서는 효과가 없습니다. 혼동을 줄이기 위해 제거하는 편이 좋습니다.적용 예시:
- className={cn( - "relative size-[178px] rounded-[10px] bg-cover bg-center bg-no-repeat", - className, - )} + className={cn("relative size-[178px] rounded-[10px]", className)}
47-60: 제목 영역의 시맨틱 개선(선택) — 헤딩 태그 도입카드 제목은 시맨틱하게 헤딩 요소로 표현하는 편이 접근성과 내비게이션에 유리합니다. 필요 시 내부 래퍼를
h3로 바꾸는 정도의 변경을 권장합니다.예시:
- <div className="h-[46px] w-[178px]"> - <div + <div className="h-[46px] w-[178px]"> + <h3 className={cn( "line-clamp-2 h-[46px] w-[178px] overflow-hidden text-ellipsis text-[19px] font-bold leading-[120%] text-text-primary", className, )} data-testid="game-title" > {children} - </div> + </h3> </div>참고: 카드 격자에서 헤딩 계층이 혼동된다면
aria-level을 함께 지정하거나, 페이지 전체 헤딩 구조에 맞춰 적절한 레벨을 선택하세요.
68-82: Badge 래퍼의 불필요한 gap 제거 제안자식이
span하나뿐이라gap-[10px]은 의미가 없습니다. 클래스 제거로 스타일 의도를 명확히 하세요.- "absolute left-2 top-2 inline-flex items-center justify-center gap-[10px] rounded-[2px] bg-background-badge-primary px-[5px] py-[2px]", + "absolute left-2 top-2 inline-flex items-center justify-center rounded-[2px] bg-background-badge-primary px-[5px] py-[2px]",
84-98: SharedBadge도 동일하게 gap 제거위 배지와 동일한 이유로
gap-[10px]제거 제안합니다.- "absolute bottom-2 left-2 inline-flex items-center justify-center gap-[10px] rounded-[2px] bg-background-badge-secondary px-[5px] py-[2px]", + "absolute bottom-2 left-2 inline-flex items-center justify-center rounded-[2px] bg-background-badge-secondary px-[5px] py-[2px]",shared/design/src/stories/gameCard.stories.tsx (1)
52-72: MyGameCard 예시에서 제목 시맨틱 및 접근성 보완 제안타이틀이 일반
div로 들어가 있어 스크린리더 탐색성이 떨어질 수 있습니다. 스토리 예시에서도h3로 노출하거나aria-label을 부여해 접근성 의도를 보여주는 것이 좋습니다.예시:
- <div className="line-clamp-2 h-[46px] w-[130px] shrink-0 overflow-hidden text-[19px] font-bold leading-[120%] text-text-primary"> - 내가 만든 게임 - </div> + <h3 className="line-clamp-2 h-[46px] w-[130px] shrink-0 overflow-hidden text-[19px] font-bold leading-[120%] text-text-primary"> + 내가 만든 게임 + </h3>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
service/app/src/entities/game/ui/components/gameLibraryGrid.tsx(2 hunks)service/app/src/entities/game/ui/components/gamePreview.tsx(2 hunks)service/app/src/widgets/GameSection.tsx(5 hunks)shared/design/src/components/gameCard/index.tsx(1 hunks)shared/design/src/components/gameCard/myGameCard.tsx(1 hunks)shared/design/src/stories/gameCard.stories.tsx(1 hunks)shared/design/src/utils/eventHandlers.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Component names in code must use PascalCase
Files:
service/app/src/entities/game/ui/components/gamePreview.tsxshared/design/src/stories/gameCard.stories.tsxservice/app/src/widgets/GameSection.tsxshared/design/src/components/gameCard/myGameCard.tsxshared/design/src/components/gameCard/index.tsxservice/app/src/entities/game/ui/components/gameLibraryGrid.tsx
🧬 Code graph analysis (6)
service/app/src/entities/game/ui/components/gamePreview.tsx (1)
shared/design/src/components/gameCard/index.tsx (1)
GameCard(100-105)
shared/design/src/stories/gameCard.stories.tsx (3)
shared/design/src/components/gameCard/index.tsx (1)
GameCard(100-105)shared/design/src/components/gameCard/myGameCard.tsx (1)
MyGameCard(10-20)shared/design/src/components/gameCard/gameCardOptions.tsx (1)
GameCardOptions(18-66)
service/app/src/widgets/GameSection.tsx (1)
shared/design/src/components/gameCard/index.tsx (1)
GameCard(100-105)
shared/design/src/components/gameCard/myGameCard.tsx (1)
shared/design/src/components/gameCard/index.tsx (1)
GameCard(100-105)
shared/design/src/components/gameCard/index.tsx (1)
shared/design/src/utils/cn.ts (1)
cn(4-6)
service/app/src/entities/game/ui/components/gameLibraryGrid.tsx (3)
shared/design/src/components/gameCard/myGameCard.tsx (1)
MyGameCard(10-20)shared/design/src/components/gameCard/index.tsx (1)
GameCard(100-105)shared/design/src/components/gameCard/gameCardOptions.tsx (1)
GameCardOptions(18-66)
⏰ 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). (1)
- GitHub Check: ci
🔇 Additional comments (8)
service/app/src/entities/game/ui/components/gamePreview.tsx (1)
94-109: GameCard.Image에 이미relative가 적용되어 있습니다
GameCard.Image 컴포넌트의 최상위<div>가 Tailwind 클래스relative를 기본으로 포함하고 있어, Next.js<Image fill>사용 시 상위 컨테이너의 상대 위치 요구를 이미 만족합니다. 추가 수정은 필요 없습니다.service/app/src/widgets/GameSection.tsx (3)
6-6: Next/Image 도입 확인 — OK이미지 썸네일에 Next/Image를 사용하도록 준비되어 있습니다. 도메인 허용 목록(next.config)만 점검하면 됩니다.
112-121: 에러 UI의 ARIA 속성 구성 적절 — LGTM
role="alert"와aria-live="assertive"를 통한 즉시 공지 패턴이 올바르게 적용되어 있습니다.
144-218: 리스트/아이템 역할 및 스켈레톤의 라이브 리전 지정 — 적절합니다
role="list"/listitem적용 및 로딩 상태의aria-live="polite"지정은 접근성 측면에서 좋습니다.shared/design/src/components/gameCard/index.tsx (2)
10-20: 합성 기반 컨테이너로의 단순화, 구조/테스트 아이디 배치 적절루트 컨테이너의 책임 축소와
data-testid="game-card"노출이 명확합니다.gap-[14px]로 내부 간격도 일관되게 유지됩니다. 이 부분은 그대로 가도 좋습니다.
100-105: 정적 서브컴포넌트 부착 방식 적절
Object.assign으로 합성 API를 노출하는 방식이 깔끔하고 타입 추론도 이점이 있습니다. 이 구조는 유지해도 좋습니다.shared/design/src/stories/gameCard.stories.tsx (2)
7-10: Storybook 타입 임포트 경로 확인 필요
Meta<typeof GameCard>사용은 적절하지만, 타입 임포트를@storybook/react-vite에서 가져오면 워크스페이스 설정에 따라 타입 해상도 문제가 있을 수 있습니다(일반적으로@storybook/react권장). 현재 버전에 맞는 올바른 패키지에서 타입을 임포트하는지 확인해주세요.적용 예시(필요한 경우에 한함, 파일 상단 변경):
-import type { Meta, StoryObj } from "@storybook/react-vite" +import type { Meta, StoryObj } from "@storybook/react"
239-251: GamePreview에서 큰 이미지 컨테이너 사용 방식 검증
<GameCard.Image className="h-[260px]">로 높이만 확장하는 패턴은 의도대로 178×260을 만들며,twMerge덕분에h-[178px]가 적절히 치환됩니다. 이 사용성은 문제 없습니다. 그대로 유지해도 좋습니다.
|
✅ CI 및 E2E 테스트 성공! 🚀 Preview URL: https://re-creation-a2cgtnf5w-whdgur5717s-projects.vercel.app 모든 테스트가 통과했습니다. 코드 리뷰를 진행해주세요! |
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
service/app/src/widgets/GameSection.tsx (1)
83-91: button에 onKeyDown 핸들러로 인한 중복 활성화(특히 Space) 가능성네이티브
<button>은 Enter/Space로 기본적으로 클릭을 트리거합니다. 현재 구현은 keydown 시 직접 호출하고, Space의 기본 click은 keyup에 발생하므로 중복 실행될 여지가 있습니다. 가장 안전한 해결은 커스텀 키 핸들러를 제거하고 네이티브 동작에 맡기는 것입니다.적용 diff:
- const handleGameCardKeyDown = ( - event: React.KeyboardEvent, - game: GameListItem, - ) => { - if (event.key === "Enter" || event.key === " ") { - event.preventDefault() - handleGameCardClick(game) - } - }- onKeyDown={(e) => handleGameCardKeyDown(e, game)}Also applies to: 180-180
service/app/src/app/dashboard/page.tsx (2)
199-207: 키보드 포커스 가시성 퇴행: 아바타 버튼에서 focus ring 제거됨
focus:outline-none만 남아 키보드 사용자에게 포커스 위치가 보이지 않습니다. 최소focus-visible기반의 링/아웃라인을 복원해주세요. 또한 메뉴와의 관계를 위해aria-controls를 추가하는 것을 권장합니다.적용 예시(diff):
- <button - className="flex size-[42px] cursor-pointer items-center justify-center rounded-full bg-gray-300 focus:outline-none" + <button + className="flex size-[42px] cursor-pointer items-center justify-center rounded-full bg-gray-300 focus:outline-none focus-visible:ring-2 focus-visible:ring-offset-2 focus-visible:ring-indigo-500" onClick={handleAvatarClick} - aria-label={`사용자 메뉴 ${listButton ? "닫기" : "열기"}`} + aria-label={`사용자 메뉴 ${listButton ? "닫기" : "열기"}`} + aria-controls="user-menu-dashboard" aria-expanded={listButton} aria-haspopup="true" tabIndex={0} >추가로, 이 페이지의 사용자 메뉴는 외부 클릭/ESC로 닫히지 않습니다(아래 HomeNavigation과 불일치). 동일한 접근성/사용성 패턴을 이 페이지에도 적용하는 것을 권장합니다.
217-229: 사용자 메뉴 컨테이너에 접근성 속성(role/id/label) 보강 필요
현재 단순 div로 표시되고 있어 보조기기에서 메뉴 컨텍스트가 명확하지 않습니다.role,aria-label, 그리고 위 버튼의aria-controls와 매칭될id를 부여해주세요.적용 예시(diff):
- {listButton && ( - <div className="absolute right-0 top-full z-10 mt-2"> + {listButton && ( + <div + id="user-menu-dashboard" + className="absolute right-0 top-full z-10 mt-2" + role="menu" + aria-label="사용자 메뉴" + > <SecondaryOutlineBoxButton size="md" onClick={handleLogoutClick} className="whitespace-nowrap" role="menuitem" aria-label="로그아웃" tabIndex={0} > 로그아웃 </SecondaryOutlineBoxButton> </div> )}service/app/src/widgets/HomeNavigation.tsx (2)
98-102: 홈 로고 버튼의 focus ring 제거로 접근성 퇴행
키보드 포커스 가시성 유지를 위해focus-visible스타일을 복원해주세요.적용 예시(diff):
- <button - className="flex h-[60px] w-[268px] cursor-pointer items-center justify-center p-3.5 focus:outline-none" + <button + className="flex h-[60px] w-[268px] cursor-pointer items-center justify-center p-3.5 focus:outline-none focus-visible:ring-2 focus-visible:ring-offset-2 focus-visible:ring-indigo-500" onClick={() => router.push("/")} aria-label="홈으로 이동" tabIndex={0} >
140-147: 아바타 버튼 focus ring 복원 + ARIA 개선(aria-haspopup, aria-controls)
- focus 스타일 제거는 키보드 접근성에 직접적 영향을 줍니다.
aria-haspopup는"menu"로 명시하는 것이 스크린리더 호환성에 유리합니다.- 메뉴 컨테이너와의 연결을 위해
aria-controls추가를 권장합니다.적용 예시(diff):
- <button - className="flex size-[42px] cursor-pointer items-center justify-center rounded-full bg-gray-300 focus:outline-none" + <button + className="flex size-[42px] cursor-pointer items-center justify-center rounded-full bg-gray-300 focus:outline-none focus-visible:ring-2 focus-visible:ring-offset-2 focus-visible:ring-indigo-500" onClick={handleAvatarClick} onKeyDown={handleAvatarKeyDown} - aria-label={`사용자 메뉴 ${listButton ? "닫기" : "열기"}`} + aria-label={`사용자 메뉴 ${listButton ? "닫기" : "열기"}`} + aria-controls="user-menu-home" aria-expanded={listButton} - aria-haspopup="true" + aria-haspopup="menu" tabIndex={0} >사소하지만,
<button>는 기본적으로 포커서블이므로tabIndex={0}는 생략 가능입니다.
🧹 Nitpick comments (17)
service/app/src/app/page.tsx (6)
12-17: FallbackProps 타입 사용으로 시그니처 일원화react-error-boundary가 제공하는 FallbackProps를 사용하면 타입 드리프트를 방지하고 가독성이 좋아집니다.
적용 diff:
-function ErrorFallback({ - resetErrorBoundary, -}: { - error: Error - resetErrorBoundary: () => void -}) { +function ErrorFallback({ resetErrorBoundary }: FallbackProps) {추가(파일 상단 근처):
import type { FallbackProps } from "react-error-boundary"
19-23: role="alert"와 aria-live="assertive" 중복role="alert"는 본질적으로 assertive live region이므로 aria-live를 중복 지정할 필요가 없습니다. 일부 a11y 검사에서 불필요한 속성으로 경고될 수 있습니다.
적용 diff:
- <div - role="alert" - className="flex min-h-screen flex-col items-center justify-center gap-4 bg-background-primary p-4" - aria-live="assertive" - > + <div + role="alert" + className="flex min-h-screen flex-col items-center justify-center gap-4 bg-background-primary p-4" + >
43-47: Skeleton 영역에는 aria-busy가 더 적합로딩 스켈레톤 컨테이너에는 aria-live 대신 aria-busy="true"가 의도를 더 명확히 전달합니다. 현재 구조(로딩 전용 컴포넌트)에서는 main에 busy를 두는 것으로 충분합니다.
적용 diff:
- <main - className="min-h-screen bg-background-primary" - aria-live="polite" - aria-label="페이지 로딩 중" - > + <main + className="min-h-screen bg-background-primary" + aria-busy="true" + aria-label="페이지 로딩 중" + >추가 제안(선택): 개별 스켈레톤 블록들은 스크린리더에 의미가 없으므로 aria-hidden="true"를 부여해 낭독을 줄일 수 있습니다.
77-80: key에 index 사용은 정적 스켈레톤에선 허용되나, 동적 리스트로 변할 경우 대비 필요현재는 정적 placeholder라 문제는 작습니다. 다만 이후 실제 데이터 기반 반복으로 전환될 여지가 있다면, 안정적인 식별자 기반 key로 전환을 고려해 주세요.
99-103: main 요소에 role="main"은 중복HTML5의 main 요소는 이미 landmark 역할을 가집니다. role 속성은 제거해도 됩니다.
적용 diff:
- <main - className="min-h-screen bg-background-primary" - role="main" - aria-label="홈페이지" - > + <main + className="min-h-screen bg-background-primary" + aria-label="홈페이지" + >
117-121: onError 핸들러 타입 추출로 안전성·가독성 향상inline 타입 표기 대신 컴포넌트 prop 타입에서 onError 시그니처를 추출하면 라이브러리 타입 변경에 더 탄탄합니다. 또한 핸들러를 분리하면 테스트와 로깅(예: Sentry) 연계가 용이합니다.
적용 diff:
- onError={( - error: Error, - errorInfo: { componentStack?: string | null | undefined }, - ) => { - console.error("Home page error:", error, errorInfo) - }} + onError={handleHomeError}파일 내 추가(컴포넌트 상단 등):
import type { ComponentProps } from "react" const handleHomeError: NonNullable< ComponentProps<typeof ErrorBoundary>["onError"] > = (error, errorInfo) => { // TODO: 필요 시 Sentry 등으로 전송 console.error("Home page error:", error, errorInfo) }원하시면 프로덕션 빌드에서 console.error를 로거/모니터링으로 라우팅하는 헬퍼도 같이 제안드리겠습니다.
service/app/src/widgets/GameSection.tsx (7)
183-183: 중복 속성 제거: button의 tabIndex={0} 불필요
<button>은 기본적으로 포커스 가능하므로tabIndex={0}는 불필요합니다.- tabIndex={0}
182-182: aria-label 문구 개선(중복 ‘보기’ 제거, 더 간결하게)스크린리더에 더 자연스럽게 읽히도록 문구를 다듬는 것을 권장합니다.
- aria-label={`${game.gameTitle} 게임 미리보기 보기. ${game.questionCount}개의 질문이 있습니다.`} + aria-label={`게임 미리보기: ${game.gameTitle}. 질문 ${game.questionCount}개.`}
112-120: role=alert와 aria-live=assertive 동시 사용은 중복
role="alert"는 암묵적으로aria-live="assertive"를 갖습니다. 둘 다 쓰면冗長합니다. 하나만 유지하세요(보통 role=alert만).- aria-live="assertive"
144-148: 로딩 상태 노출은 컨테이너에 aria-busy로 통일추천 게임 목록 영역에
aria-busy를 추가하면 로딩 중임을 일관되게 알릴 수 있습니다(내부에서 별도의 live region 없이도 충분).- <div - className="flex items-center gap-[80px]" - role="region" - aria-label="추천 게임 목록" - > + <div + className="flex items-center gap-[80px]" + role="region" + aria-label="추천 게임 목록" + aria-busy={isLoading} + >
151-153: 로딩 스켈레톤의 aria-live 제거 제안상위 컨테이너에
aria-busy를 부여하면 여기의aria-live는 불필요합니다. 라이브 영역은 실제 콘텐츠가 동적으로 삽입될 때만 최소한으로 사용하는 것이 좋습니다.- aria-live="polite" - aria-label="게임 목록 로딩 중" + aria-label="게임 목록 로딩 중"
172-195: 사용 중인_index는 관례상index로 명명앞에
_는 “사용하지 않는 변수”를 의미하는 관례가 많습니다. 현재 사용 중이므로index로 명확히 해주세요.- {games.map((game, _index) => ( + {games.map((game, index) => (- priority={_index < 2} + priority={index < 2}
188-197: 이미지 대체 텍스트를 더 구체적으로현재 alt가 게임명만 노출되어 이미지 성격이 모호합니다. “썸네일” 등 맥락을 포함하면 더 명확합니다.
- alt={game.gameTitle} + alt={`${game.gameTitle} 썸네일`}service/app/src/app/dashboard/page.tsx (2)
18-18: API import은 적절. 상세 조회는 react-query fetchQuery로 이관해 캐싱/중복요청 방지 고려
현재handleGameClick에서 직접getGameDetail를 호출합니다. 동일 카드 재클릭 시 중복 요청이 발생할 수 있어queryClient.fetchQuery로 조회를 통일하면 캐시·에러 처리가 일원화됩니다. 공유/공유해제 성공 시에도 목록 쿼리 무효화가 일관되게 수행되는지 점검 부탁드립니다.예시(선택 적용):
// 내부 변경 제안 (참고용) const gameDetail = await queryClient.fetchQuery({ queryKey: ["gameDetail", game.gameId], queryFn: async () => { const res = await getGameDetail(game.gameId) if (res.result !== "SUCCESS" || !res.data) throw new Error("게임 상세 조회 실패") return res.data }, staleTime: 30_000, })공유/공유해제 콜백 내부에서도 성공 시 다음과 같이 보강할 수 있습니다:
await shareGameAction(game) // or await unshareGameAction(game) await queryClient.invalidateQueries({ queryKey: ["infiniteMyGames"] })
259-259: 중앙 헤더 영역 고정폭(1080px)으로 반응형 레이아웃이 깨질 수 있음
헤더 좌/우 폭이 바뀌면 중앙 정렬이 틀어질 수 있습니다.flex-1로 유연 폭을 주는 쪽이 안전합니다.적용 예시(diff):
- <div className="flex w-[1080px] justify-center">{centerContent}</div> + <div className="flex flex-1 min-w-0 justify-center">{centerContent}</div>service/app/src/widgets/HomeNavigation.tsx (2)
76-87: 바깥 클릭 감지:mousedown대신pointerdown사용 고려 + 수동 스크롤 최적화
터치 환경까지 포괄하려면pointerdown이 더 안전합니다. 또한 패시브 리스너를 지정해 스크롤과의 간섭을 줄일 수 있습니다.적용 예시(diff):
- if (listButton) { - document.addEventListener("mousedown", handleClickOutside) - return () => document.removeEventListener("mousedown", handleClickOutside) - } + if (listButton) { + document.addEventListener("pointerdown", handleClickOutside, { passive: true }) + return () => + document.removeEventListener("pointerdown", handleClickOutside as EventListener) + }참고:
closest("[data-user-menu]")패턴과 함께 사용 시 동작은 동일합니다.
156-172: 사용자 메뉴 컨테이너에 id 추가(버튼의 aria-controls와 연결)
이미role="menu"/aria-label이 있어 좋습니다. 버튼과의 연결을 위해id만 부여하면 완성입니다.적용 예시(diff):
- {listButton && ( - <div - className="absolute right-0 top-full z-10 mt-2" + {listButton && ( + <div + id="user-menu-home" + className="absolute right-0 top-full z-10 mt-2" role="menu" aria-label="사용자 메뉴" >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
service/app/src/app/dashboard/page.tsx(3 hunks)service/app/src/app/games/page.tsx(5 hunks)service/app/src/app/page.tsx(6 hunks)service/app/src/widgets/GameSection.tsx(5 hunks)service/app/src/widgets/HomeNavigation.tsx(6 hunks)
✅ Files skipped from review due to trivial changes (1)
- service/app/src/app/games/page.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Component names in code must use PascalCase
Files:
service/app/src/app/page.tsxservice/app/src/app/dashboard/page.tsxservice/app/src/widgets/HomeNavigation.tsxservice/app/src/widgets/GameSection.tsx
🧬 Code graph analysis (2)
service/app/src/widgets/HomeNavigation.tsx (1)
service/app/src/entities/auth/hooks/useAuth.ts (1)
useAuth(18-110)
service/app/src/widgets/GameSection.tsx (1)
shared/design/src/components/gameCard/index.tsx (1)
GameCard(100-105)
⏰ 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). (1)
- GitHub Check: ci
🔇 Additional comments (5)
service/app/src/widgets/GameSection.tsx (2)
6-6: Next/Image 도입 👍 — 부모 컨테이너의 position 확인 필요
fill사용 시 바로 감싸는 컨테이너가position: relative여야 합니다.GameCard.Image가 해당 스타일(또는 className)을 보장하는지 한 번만 확인 부탁드립니다. 보장되지 않으면 이미지가 0x0로 렌더링되거나 레이아웃이 깨질 수 있습니다.
185-211: 조합형 GameCard(API: Image/Badge/SharedBadge/Title) 마이그레이션 전반적으로 깔끔합니다
- 썸네일 유무 분기 처리, 배지/공유배지 조건부 렌더링, Title 분리까지 일관성 있게 적용되었습니다.
- 스켈레톤/오류 상태와도 잘 연결되어 있습니다.
service/app/src/widgets/HomeNavigation.tsx (3)
23-23: useAuth 구조분해에 login/authLoading 추가: 의도대로 로그인 버튼과 연동 확인
개발 모드 분기(handleLoginClick)와aria-busy에서 잘 사용되고 있습니다. 별도 이슈 없습니다.
114-116: 정렬 조건부 클래스 적용 OK
로그인 여부에 따라justify-end/center를 스위칭하는 패턴이 간결합니다.
182-184: 동적 aria-label로 진행 상태 전달 OK
aria-busy와 함께 상태를 명확히 알리고 있습니다. 로딩 스피너 아이콘이 있다면aria-hidden="true"처리도 현재와 일관됩니다.
| <button | ||
| onClick={resetErrorBoundary} | ||
| className="rounded-lg bg-background-interactive-primary px-4 py-2 text-text-interactive-inverse hover:bg-background-interactive-primary-hovered focus:outline-none focus:ring-2 focus:ring-border-interactive-primary focus:ring-offset-2" | ||
| className="rounded-lg bg-background-interactive-primary px-4 py-2 text-text-interactive-inverse hover:bg-background-interactive-primary-hovered focus:outline-none" |
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.
키보드 포커스 링 제거로 인한 접근성 회귀
버튼에서 focus:ring 스타일이 제거되어 키보드 사용자가 포커스를 인지하기 어려워집니다. focus-visible 기반 링을 복원해 주세요. (WCAG 2.4.7 Focus Visible)
적용 diff:
- className="rounded-lg bg-background-interactive-primary px-4 py-2 text-text-interactive-inverse hover:bg-background-interactive-primary-hovered focus:outline-none"
+ className="rounded-lg bg-background-interactive-primary px-4 py-2 text-text-interactive-inverse hover:bg-background-interactive-primary-hovered focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-border-interactive-primary focus-visible:ring-offset-2"📝 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.
| className="rounded-lg bg-background-interactive-primary px-4 py-2 text-text-interactive-inverse hover:bg-background-interactive-primary-hovered focus:outline-none" | |
| className="rounded-lg bg-background-interactive-primary px-4 py-2 text-text-interactive-inverse hover:bg-background-interactive-primary-hovered focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-border-interactive-primary focus-visible:ring-offset-2" |
🤖 Prompt for AI Agents
In service/app/src/app/page.tsx around line 32, the button's focus ring was
removed causing keyboard users to lose visible focus; restore an accessible
focus-visible ring by re-adding focus-visible utility classes (e.g.
focus-visible:ring-2 focus-visible:ring-offset-2 focus-visible:ring-[color] and
focus-visible:ring-offset-[bg] or the design-system equivalents) while keeping
the default focus:outline-none if desired; update the className to include those
focus-visible classes so the ring only appears for keyboard focus.
| onClick={() => handleGameCardClick(game)} | ||
| onKeyDown={(e) => handleGameCardKeyDown(e, game)} | ||
| className="cursor-pointer border-none bg-transparent p-0 focus:outline-none focus:ring-2 focus:ring-border-interactive-primary focus:ring-offset-2 focus:ring-offset-background-primary" | ||
| className="cursor-pointer border-none bg-transparent p-0 focus:outline-none" |
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
키보드 접근성: focus:outline-none 제거
포커스 아웃라인을 제거하면 키보드 사용자에게 포커스 위치가 보이지 않습니다. 디자인에 맞는 커스텀 포커스 링이 없다면 최소한 기본 아웃라인은 유지해야 합니다.
- className="cursor-pointer border-none bg-transparent p-0 focus:outline-none"
+ className="cursor-pointer border-none bg-transparent p-0"📝 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.
| className="cursor-pointer border-none bg-transparent p-0 focus:outline-none" | |
| className="cursor-pointer border-none bg-transparent p-0" |
🤖 Prompt for AI Agents
In service/app/src/widgets/GameSection.tsx around line 181, the class removes
the browser focus ring ("focus:outline-none"), breaking keyboard accessibility;
remove the "focus:outline-none" utility from that element and instead either
rely on the default focus outline or add an accessible focus style (e.g., a
visible focus-visible/focus:ring or custom outline) so keyboard users can see
focus; ensure the chosen style appears on keyboard navigation (use
:focus-visible where appropriate).
|
✅ CI 및 E2E 테스트 성공! 🚀 Preview URL: https://re-creation-4itzpq8l6-whdgur5717s-projects.vercel.app 모든 테스트가 통과했습니다. 코드 리뷰를 진행해주세요! |
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: 0
♻️ Duplicate comments (1)
shared/design/src/components/gameCard/index.tsx (1)
1-2: ReactNode 타입 임포트로 네임스페이스 의존 제거 — 좋습니다이전 커밋에서 지적되었던
React.ReactNode사용 이슈가 타입 임포트로 해결되었습니다. 별도 네임스페이스 임포트 없이도 TS가 안정적으로 동작합니다.
🧹 Nitpick comments (7)
shared/design/src/components/gameCard/index.tsx (7)
6-9: 루트 컨테이너에 polymorphicas지원을 추가하여 시맨틱/접근성 유연성 확보 제안카드가 리스트 항목/섹션/아티클로 쓰일 수 있어 요소 유형을 바꿀 수 있으면 좋습니다. 간단한
as프로퍼티를 추가하면 DOM 깊이를 늘리지 않고도 시맨틱을 개선할 수 있습니다.아래 최소 변경으로 적용 가능합니다:
interface GameCardProps { children: ReactNode className?: string + as?: "div" | "article" | "section" | "li" } const GameCardComponent = forwardRef<HTMLDivElement, GameCardProps>( - ({ children, className }, ref) => { + ({ children, className, as = "div" }, ref) => { + const Component = as as any return ( - <div - ref={ref} - className={cn("relative", className)} - data-testid="game-card" - > + <Component ref={ref} className={cn("relative", className)} data-testid="game-card"> <div className="flex flex-col gap-[14px]">{children}</div> - </div> + </Component> ) }, )Also applies to: 11-21
43-62: Title DOM 중첩 최소화 + 멀티라인 말줄임 관련 클래스 정리 + 시맨틱 요소 선택 지원
- 불필요한 래퍼 div를 제거하면 DOM이 1단계 얕아집니다.
line-clamp-2와text-ellipsis는 중복 의미입니다. 멀티라인에는line-clamp-2만으로 충분합니다.- 제목은 보통 heading이므로
as로h2/h3/p를 선택할 수 있게 하면 접근성이 향상됩니다.-interface GameCardTitleProps { - children: ReactNode - className?: string -} +interface GameCardTitleProps { + children: ReactNode + className?: string + as?: "h2" | "h3" | "p" +} -const GameCardTitle = ({ children, className }: GameCardTitleProps) => { - return ( - <div className="h-[46px] w-[178px]"> - <div - className={cn( - "line-clamp-2 h-[46px] w-[178px] overflow-hidden text-ellipsis text-[19px] font-bold leading-[120%] text-text-primary", - className, - )} - data-testid="game-title" - > - {children} - </div> - </div> - ) -} +const GameCardTitle = ({ children, className, as = "h3" }: GameCardTitleProps) => { + const As = as as any + return ( + <As + className={cn( + "line-clamp-2 h-[46px] w-[178px] text-[19px] font-bold leading-[120%] text-text-primary", + className, + )} + data-testid="game-title" + > + {children} + </As> + ) +}
64-83: Badge 접근성 힌트(레이블) 추가 + 불필요한 gap 제거
- 배지의 의미를 스크린 리더에 전달할 수 있도록
aria-label을 받을 수 있게 하세요.- 자식이 하나라면
gap-[10px]는 불필요합니다.interface GameCardBadgeProps { children: ReactNode className?: string + ariaLabel?: string } -const GameCardBadge = ({ children, className }: GameCardBadgeProps) => { +const GameCardBadge = ({ children, className, ariaLabel }: GameCardBadgeProps) => { return ( <div className={cn( - "absolute left-2 top-2 inline-flex items-center justify-center gap-[10px] rounded-[2px] bg-background-badge-primary px-[5px] py-[2px]", + "absolute left-2 top-2 inline-flex items-center justify-center rounded-[2px] bg-background-badge-primary px-[5px] py-[2px]", className, )} data-testid="question-count" + aria-label={ariaLabel} > <span className="text-[13px] font-normal leading-[120%] text-text-inverse"> {children} </span> </div> ) }참고: PR 설명에 따르면 data-testid는 아직 작업 중이라고 하셨으므로,
question-count라는 도메인 특화 네이밍을 보다 범용적인 값(예:badge)으로 바꾸는 건 후속 PR에서 검토해도 좋겠습니다.
85-99: SharedBadge도 동일한 접근성/불필요 클래스 정리 적용위 배지와 동일하게
gap-[10px]제거 및 접근성 레이블을 받을 수 있게 하면 일관성이 좋아집니다.-const GameCardSharedBadge = ({ children, className }: GameCardBadgeProps) => { +const GameCardSharedBadge = ({ children, className, ariaLabel }: GameCardBadgeProps) => { return ( <div className={cn( - "absolute bottom-2 left-2 inline-flex items-center justify-center gap-[10px] rounded-[2px] bg-background-badge-secondary px-[5px] py-[2px]", + "absolute bottom-2 left-2 inline-flex items-center justify-center rounded-[2px] bg-background-badge-secondary px-[5px] py-[2px]", className, )} data-testid="shared-badge" + aria-label={ariaLabel} > <span className="text-[13px] font-normal leading-[120%] text-text-inverse"> {children} </span> </div> ) }
101-106: Compound 타입 안전성 강화(소비 측 자동완성/타입 추론 개선)
Object.assign의 추론만으로도 동작하긴 하지만, 정식 타입을 부여하면 소비 코드에서GameCard.Image등의 자동완성과 타입 체킹이 더 견고해집니다.적용 예시:
-export const GameCard = Object.assign(GameCardComponent, { +export const GameCard: ForwardRefExoticComponent<GameCardProps & RefAttributes<HTMLDivElement>> & { + Image: typeof GameCardImage + Title: typeof GameCardTitle + Badge: typeof GameCardBadge + SharedBadge: typeof GameCardSharedBadge +} = Object.assign(GameCardComponent, { Image: GameCardImage, Title: GameCardTitle, Badge: GameCardBadge, SharedBadge: GameCardSharedBadge, })위 변경을 위해 상단 타입 임포트를 추가하세요:
// 상단 import 라인 보강 import type { ReactNode, ForwardRefExoticComponent, RefAttributes } from "react"
108-108: DevTools 가독성을 위한 서브컴포넌트 displayName 지정루트에만
displayName이 설정되어 있어 DevTools에서 하위 컴포넌트가 익명으로 보일 수 있습니다. 디버깅 편의를 위해 displayName을 지정하세요.GameCardComponent.displayName = "GameCard" +GameCardImage.displayName = "GameCard.Image" +GameCardTitle.displayName = "GameCard.Title" +GameCardBadge.displayName = "GameCard.Badge" +GameCardSharedBadge.displayName = "GameCard.SharedBadge"
25-39: GameCardImage 컴포넌트 배경 유틸 클래스 최적화 제안 & 중복 정의 미발견
bg-cover,bg-center,bg-no-repeat유틸 클래스는 background-image가 설정될 때만 의미가 있으므로, 현재 children 슬롯만 제공되는 상태에서는 불필요합니다.아래 두 가지 방안 중 하나를 선택하세요.
- 간단 정리: 배경 관련 클래스 제거
- 기능 추가:
imageUrl?: stringprop을 받아style={{ backgroundImage:url(${imageUrl})}}로 렌더링저장소 전체를 검색한 결과,
gameCardImage.tsx파일이나 동일한export const GameCardImage정의는 발견되지 않았습니다. 중복 노출로 인한 혼선은 없습니다.추천 diff 예시:
interface GameCardImageProps { children: ReactNode className?: string + // 이미지 URL을 prop으로 받아 배경 이미지 렌더링 시 사용 + imageUrl?: string } -const GameCardImage = ({ children, className }: GameCardImageProps) => { +const GameCardImage = ({ children, className, imageUrl }: GameCardImageProps) => { return ( <div className={cn( - "relative size-[178px] rounded-[10px] bg-cover bg-center bg-no-repeat", + "relative size-[178px] rounded-[10px]", className, )} + {imageUrl && { style: { backgroundImage: `url(${imageUrl})` } }} > {children} </div> ) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
shared/design/src/components/gameCard/index.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Component names in code must use PascalCase
Files:
shared/design/src/components/gameCard/index.tsx
🧬 Code graph analysis (1)
shared/design/src/components/gameCard/index.tsx (1)
shared/design/src/utils/cn.ts (1)
cn(4-6)
⏰ 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). (1)
- GitHub Check: ci
|
✅ CI 및 E2E 테스트 성공! 🚀 Preview URL: https://re-creation-d1htjmh2e-whdgur5717s-projects.vercel.app 모든 테스트가 통과했습니다. 코드 리뷰를 진행해주세요! |
| import type { MouseEvent } from "react" | ||
|
|
||
| export function stopAnd(fn?: () => void) { | ||
| return (e: MouseEvent) => { | ||
| e.stopPropagation(); | ||
| fn?.(); | ||
| }; | ||
| } |
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.
이런 유틸함수가 굳이 필요할지 잘 모르겠습니다
사용할거라면, 이 함수가 타입을 좀 더 복잡하게 작성해야할것같은데(함수 인자를 넣고 실행하는것도 가능하게끔)
그럴거면 오히려 안쓰는 게 더 낫지않을까? 하는 생각이 드네요
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.
인자 생각하면 빼는 게 나을 것 같네요 롤백하겠습니답
| onClick={handleAvatarClick} | ||
| onKeyDown={handleAvatarKeyDown} | ||
| aria-label={`사용자 메뉴 ${listButton ? '닫기' : '열기'}`} | ||
| aria-label={`사용자 메뉴 ${listButton ? "닫기" : "열기"}`} |
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.
이런 토글에 대한건 aria-expanded에서 이미 알려주고 있어서, 그냥 사용자 메뉴 버튼 이런식으로 하는게 맞는것같습니다
| aria-label={ | ||
| authLoading ? "로그인 처리 중" : "카카오 간편 로그인" | ||
| } |
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.
이것도 로그인버튼 이런식으로 처리해주시면 될것같습니다
|
✅ CI 및 E2E 테스트 성공! 🚀 Preview URL: https://re-creation-fvn99rnub-whdgur5717s-projects.vercel.app 모든 테스트가 통과했습니다. 코드 리뷰를 진행해주세요! |
|
✅ CI 및 E2E 테스트 성공! 🚀 Preview URL: https://re-creation-euupvh68s-whdgur5717s-projects.vercel.app 모든 테스트가 통과했습니다. 코드 리뷰를 진행해주세요! |
|
✅ CI 및 E2E 테스트 성공! 🚀 Preview URL: https://re-creation-oqpeqfh8l-whdgur5717s-projects.vercel.app 모든 테스트가 통과했습니다. 코드 리뷰를 진행해주세요! |
📝 설명
gameCard 컴포넌트의 리팩토링을 진행했습니다.
🛠️ 주요 변경 사항
gameCard 컴포넌트 변경 전
gameCard 컴포넌트 변경 후 (합성 컴포넌트 패턴 적용)
next/image 외부 주입 가능
리뷰 시 고려해야 할 사항
data-testid 및 접근성 관련 로직은 별도 pr로 올릴 예정입니다
Summary by CodeRabbit