[BOM-490] feat: 완료한 챌린지 카드에서 해당 챌린지 대시보드로 이동#294
Hidden character warning
Conversation
jaeyoung-kwon
left a comment
jaeyoung-kwon
left a comment
There was a problem hiding this comment.
🤖 PR Review
⚠️ 수정이 필요한 리뷰 코멘트가 있습니다.
완료 챌린지 카드에 대시보드 이동 기능을 추가한 변경입니다. FAIL 카드 클릭 비활성화, 수료증 버튼의 stopPropagation 처리는 의도대로 잘 구현되어 있습니다. 다만 challengeId가 없는 완료 챌린지에서의 이동 처리와 클릭 카드의 키보드 접근성 두 가지를 확인해 주세요.
🚨 0 Critical ·
수정 필요
- web/src/pages/my-page/components/MyChallengeSection/CompletedChallengeCard.tsx:40 — challengeId가 없는 완료 챌린지에서
/challenge/undefined/dashboard로 이동할 수 있음 (인라인 코멘트 참조). - web/src/pages/my-page/components/MyChallengeSection/CompletedChallengeCard.tsx:56 — 클릭 가능한 카드에 키보드/스크린리더 접근성이 없음 (인라인 코멘트 참조).
참고
- web/src/pages/my-page/components/MyChallengeSection/CompletedChallengeCard.tsx:80 —
CertButton의disabled={isFail}은 항상false
CertButton은grade !== 'FAIL'일 때만 렌더링되므로 이 분기에서isFail은 항상false입니다.disabledprop이 실질적으로 아무 효과가 없어 읽는 사람에게 혼동을 줄 수 있습니다. 제거하거나 의도를 명확히 하는 주석을 남기는 것을 검토해 주세요.
📋 검증 과정
- Claude structured review 결과 중 확신도가 있는 항목만 정리했습니다.
- Critical/Major는 inline comment로 게시하고, Minor는 참고 항목으로 summary에 포함합니다.
- 자동 생성된 OpenAPI 타입 선언 파일과 lock 파일은 리뷰 대상에서 제외합니다.
🤖 Claude PR Review
Generated by Claude Code
| const handleCardClick = () => { | ||
| if (isFail) return; | ||
|
|
||
| navigate({ | ||
| to: '/challenge/$challengeId/dashboard', | ||
| params: { challengeId: String(challengeId) }, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
/challenge/undefined/dashboard로 이동할 수 있음
MyCompletedChallenge.challengeId는 OpenAPI 스키마상 number | undefined입니다(web/src/types/openapi.d.ts의 CompletedChallengeResponse.challengeId?: number). 실제로 90번째 줄에서는 CertificateModal을 렌더링할 때 challengeId !== undefined를 명시적으로 체크하고 있어, 이 값이 비어 있을 수 있다는 걸 이미 인지하고 있는 것으로 보입니다.
하지만 handleCardClick은 isFail만 체크하고 challengeId의 존재 여부는 확인하지 않습니다. 등급이 FAIL이 아니면서 challengeId가 없는 카드를 클릭하면 String(challengeId)가 "undefined" 문자열이 되어 /challenge/undefined/dashboard로 이동하게 됩니다.
Claude 검증: 변경 diff의 web/src/pages/my-page/components/MyChallengeSection/CompletedChallengeCard.tsx:40-47 기준으로 확인했습니다. 실제 코드와 다르면 이 코멘트는 무시해 주세요. (confidence 0.75).
수정 제안:
이 finding이 여전히 유효한지 먼저 확인한 뒤 (예: 실제 API 응답에서 완료 챌린지의 challengeId가 항상 존재하는지), 유효하다면 handleCardClick에서 if (isFail || challengeId === undefined) return;처럼 가드를 추가하는 것을 검토해 주세요.
Generated by Claude Code
There was a problem hiding this comment.
챌린지 id가 없으면 클릭 이벤트가 발생하지 않도록 가드를 추가했습니다
| return ( | ||
| <> | ||
| <Container> | ||
| <Container disabled={isFail} onClick={handleCardClick}> |
There was a problem hiding this comment.
Container가 onClick으로 실질적인 네비게이션 트리거가 되었지만 role="button", tabIndex, 키보드 핸들러(Enter/Space)가 없어 마우스 없이는 이 카드로 대시보드에 진입할 수 없습니다.
같은 저장소의 web/src/pages/challenge/index/components/ChallengeCard/CardContainer.tsx는 동일한 "클릭 가능한 카드" 패턴에서 interactive = Boolean(onClick) && !disabled를 기준으로 role, tabIndex, onClick을 조건부로 부여하는 방식을 이미 사용하고 있어, 이 PR만 해당 컨벤션을 따르지 않게 됩니다.
Claude 검증: 변경 diff의 web/src/pages/my-page/components/MyChallengeSection/CompletedChallengeCard.tsx:56 기준으로 확인했습니다. 실제 코드와 다르면 이 코멘트는 무시해 주세요. (confidence 0.7).
수정 제안:
이 finding이 여전히 유효한지 먼저 확인한 뒤, CardContainer.tsx의 패턴을 참고해 role="button"/tabIndex={0}/키보드 핸들러를 추가하거나, 팀에서 이 정도는 허용 범위라고 판단하면 그대로 두어도 됩니다.
Generated by Claude Code
There was a problem hiding this comment.
div 태그에 onClick이 props로 전달되고 있어서 접근성 관련 코멘트가 달린 것 같아, button 태그로 수정했습니다!
There was a problem hiding this comment.
수정했지만.. button 안에 button(수료증 확인)이 있는 중첩스타일이라서 div as="button" 으로 변경하고 접근성 개선 코드를 추가했습니다
2 participants
📌 What
2026-07-14.11.40.21.mov
❓ Why
🔧 How
CompletedChallengeCard의Container에useNavigate로/challenge/$challengeId/dashboard이동 연결grade === 'FAIL'이면disabledprop으로 클릭·호버 비활성화stopPropagation으로 카드 이동 이벤트 차단👀 Review Point