refactor: 카드 클릭 시 상세페이지 이동 - #81
Conversation
…into refactor/mover-estimate
…into refactor/mover-estimate
…into refactor/mover-estimate
…into refactor/mover-estimate
…into refactor/mover-estimate
…into refactor/mover-estimate
…into refactor/mover-estimate
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthrough
ChangesSent Estimate 상세 이동
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant SentEstimateCard
participant markInternalDetailNavigationOnClick
participant EstimateDetailRoute
User->>SentEstimateCard: 카드 또는 완료 오버레이 링크 클릭
SentEstimateCard->>markInternalDetailNavigationOnClick: 내부 상세 이동 추적
SentEstimateCard->>EstimateDetailRoute: estimate.id 기반 상세 경로 이동
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/estimate/sent/SentEstimateCard.tsx`:
- Line 37: SentEstimateCard의 viewDetail 및 onViewDetail prop 계약을 명확히 하세요.
onViewDetail을 필수 prop으로 변경하거나, prop이 없을 때 카드의 포커스 가능 상태와 상호작용/링크 핸들러를 렌더링하지 않도록
조정하세요.
- Around line 40-52: Replace the clickable article behavior in SentEstimateCard
with a next/link Link targeting APP_ROUTES.MOVER_ESTIMATES.SENT_DETAIL, keeping
markInternalDetailNavigationOnClick in the Link onClick. Separate the
completed-state button and Link as sibling elements, and remove the parent role,
tabIndex, onClick, and onKeyDown handlers so their focus and keyboard semantics
do not overlap.
- Around line 46-50: SentEstimateCard의 onKeyDown 핸들러에서 Space 키의 자동 반복 입력을 무시하도록
event.repeat를 검사하고, Enter 동작은 유지하세요. Enter와 Space를 눌렀을 때
viewDetail(onViewDetail)이 각각 한 번만 호출되는 테스트를 추가하거나 갱신하세요.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 321ac75e-3372-409d-9abc-c4bbe02ce591
📒 Files selected for processing (2)
src/components/estimate/sent/SentEstimateCard.tsxsrc/icons/index.ts
juengseulki
left a comment
There was a problem hiding this comment.
📋 PR 리뷰
👍 좋았던 점
- 기사님의 보낸 견적 카드 전체에서 상세 페이지로 이동할 수 있도록 수정했습니다.
- 완료 상태뿐 아니라 채팅 조율이 필요한 미완료 견적에서도 상세 진입이 가능해졌습니다.
- 카드 클릭, 기존 상세보기 버튼, 키보드 입력이 동일한
viewDetail()함수를 재사용합니다. - 카드에
role="link"와tabIndex={0}을 적용해 키보드 접근을 지원했습니다. - Enter와 Space 키 모두 상세 이동 동작을 실행하도록 구성했습니다.
- Space 키 입력 시 기본 스크롤을 방지해 조작 경험을 안정적으로 만들었습니다.
aria-label에 고객명과 상세보기 목적을 포함해 카드의 동작을 설명했습니다.focus-visibleoutline을 추가해 키보드 포커스 위치를 명확하게 표시했습니다.- 내부 상세보기 버튼에서는 이벤트 전파를 중단해 부모 카드 클릭과 중복 실행되지 않도록 했습니다.
- 기존 완료 견적의 상세보기 버튼 UI는 유지하면서 카드 전체 이동 기능만 확장했습니다.
- 현재 PR은 열려 있고 병합 가능한 상태이며 변경 파일은 2개입니다.
🔍 확인 및 제안
필수 수정사항은 보이지 않았습니다.
카드 전체에 링크 역할을 부여하면서 내부에 동일한 이동을 수행하는 버튼이 남아 있습니다.
현재 두 동작의 목적이 같고 이벤트 전파도 차단되어 있어 기능상 문제는 없습니다.
다만 추후 카드 내부에 찜, 메뉴, 채팅 등 별도의 액션이 추가되면
부모 카드의 클릭 처리와 내부 인터랙션의 역할 충돌을 다시 확인하는 것이 좋겠습니다.
또한 onViewDetail은 Optional이지만 카드에는 항상 링크 역할과 클릭 스타일이 적용됩니다.
모든 사용처에서 callback을 보장한다면 괜찮지만,
그렇지 않다면 prop을 필수로 만들거나 상호작용 속성을 조건부로 적용하는 편이 더 안전합니다.
FrameIcon export는 이번 카드 변경에서 사용되지 않는 것으로 보여
충돌 해결 과정에서 의도적으로 포함된 변경인지 확인하면 좋겠습니다.
전체적으로 채팅 기능 도입 이후 SENT 상태에서도 상세 화면에 접근해야 하는 요구를
기존 카드 구조를 크게 변경하지 않고 잘 반영한 것으로 보입니다.
수고하셨습니다! 😊
To Reviewer 내용 기준으로 충돌 해결 이후 카드 이동 동작과
기존 상세보기 버튼의 이벤트 충돌 여부를 중점적으로 확인했습니다!
충돌 해결로 인해 기존 상세보기 기능이 깨진 부분은 보이지 않았습니다.
현재 카드 전체에 onClick이 추가됐고,
기존 완료 견적의 상세보기 버튼은 그대로 유지됩니다.
내부 버튼에서는 event.stopPropagation()을 사용하고 있어
버튼 클릭 시 다음과 같이 동작합니다.
- 내부 상세보기 버튼의
viewDetail()실행 - 부모 카드의
onClick은 실행되지 않음 - 상세 이동이 중복으로 호출되지 않음
카드 자체를 클릭한 경우에는 부모의 viewDetail()만 실행됩니다.
키보드 접근도 함께 확인했습니다.
- Tab으로 카드에 포커스 가능
- Enter로 상세 이동
- Space로 상세 이동
- Space 입력 시 페이지 스크롤 방지
- Focus outline 노출
- 고객명 기반
aria-label제공
따라서 마우스 클릭뿐 아니라 키보드 조작에서도 상세 페이지에 접근할 수 있는 구조입니다.
확인할 만한 선택적 사항은 두 가지입니다.
첫째, onViewDetail이 Optional인데 카드에는 항상 링크 역할과 클릭 스타일이 적용됩니다.
실제 모든 사용처에서 callback을 전달한다면 문제없지만,
전달되지 않는 사용처가 있다면 클릭 가능한 것처럼 보이고 동작하지 않을 수 있습니다.
둘째, FrameIcon export가 추가됐지만 이번 카드 diff에서는 사용되지 않습니다.
충돌 해결 과정에서 다른 변경과 함께 들어온 것인지 한 번 확인하면 좋겠습니다.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/estimate/sent/SentEstimateCard.tsx`:
- Around line 44-49: 완료 상태에서 동일한 detailHref를 사용하는 카드 링크와 오버레이 링크가 중복 탭 정지를 만들고
있습니다. SentEstimateCard의 전체 카드 Link에 완료 상태일 때 tabIndex={-1}을 적용해 키보드 포커스 대상에서
제외하고, 오버레이 Link의 포커스 및 탐색 동작은 유지하세요.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1e27d044-b542-4e3b-8fd9-b3f9baebe993
📒 Files selected for processing (2)
src/components/estimate/sent/SentEstimateCard.tsxsrc/components/estimate/sent/SentEstimatesPage.tsx
💤 Files with no reviewable changes (1)
- src/components/estimate/sent/SentEstimatesPage.tsx
📋 작업 내용
🔥 변경 사항
✅ 체크리스트
📷 스크린샷 (선택)
🔗 관련 이슈
Closes #
💬 To Reviewer
Summary by CodeRabbit