Conversation
closes #103
|
@coderabbitai review |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ Action performedReview finished.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough자소서 작성 플로우가 4단계에서 5단계로 확장되었다. 제목 입력 단계가 추가되었고, 문항 입력과 초안 선택 단계가 뒤로 이동했다. 선택한 질문의 초안을 상세히 조회하는 Changes자소서 작성 플로우
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
actor 작성자
participant CoverLetterPage
participant CoverLetterStep3
participant CoverLetterStep4
participant CoverLetterStep5
participant CoverLetterStep6
작성자->>CoverLetterStep3: 제목 입력
CoverLetterStep3->>CoverLetterPage: 제목 저장 및 다음 단계 요청
CoverLetterPage->>CoverLetterStep4: 문항 입력 화면 표시
작성자->>CoverLetterStep4: 문항과 글자 수 입력
CoverLetterStep4->>CoverLetterPage: 초안 생성 단계로 진행
CoverLetterPage->>CoverLetterStep5: 초안 목록 표시
작성자->>CoverLetterStep5: 질문 선택
CoverLetterPage->>CoverLetterStep6: 선택된 질문과 초안 표시
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/features/coverLetter/components/CoverLetterStep5.jsx (1)
1-50: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift"모두 선택" 기능이 실제로는 아무 동작도 안 해요.
selectedIds/isAllSelected/handleToggleSelectAll을 만들어서 "모두 선택" 버튼으로 카드 스타일(selectedprop)을 바꿔주긴 하는데, 그 이후에selectedIds를 사용하는 코드가 없어요.DraftQuestionCard의 클릭 핸들러(onSelect)는onSelectQuestion(페이지의setActiveQuestionId)에 바로 연결되어 있어서 상세보기로 이동하는 용도이고,selectedIds와는 별개예요. 복사(handleCopy)도 개별 id 기준으로만 동작하고 선택된 여러 항목을 한 번에 복사하는 로직은 없습니다.즉 사용자가 "모두 선택"을 눌러도 카드 테두리만 바뀔 뿐 실질적으로 얻는 게 없어서, 이 리뷰 스택에서 의도한 "여러 문항 초안을 선택해서 복사"하는 기능은 아직 안 붙어있는 상태로 보여요. 선택된 항목들을 모아서 복사하는 버튼(예: "선택 항목 복사하기")을 추가하거나, 아니면 이번 PR 범위에서 "모두 선택" UI 자체를 빼는 방향으로 정리가 필요할 것 같아요.
🤖 Prompt for 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. In `@src/features/coverLetter/components/CoverLetterStep5.jsx` around lines 1 - 50, Connect selectedIds to a meaningful action in CoverLetterStep5 by adding a button that copies the selected draft answers together, using the existing handleCopy logic or an equivalent multi-item handler; ensure the action is disabled or safely handles an empty selection. If multi-copy is out of scope, remove selectedIds, isAllSelected, handleToggleSelectAll, the select-all button, and the selected prop instead of keeping nonfunctional selection UI.src/pages/ai/CoverLetterPage.jsx (1)
55-101: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift페이지 컴포넌트가 점점 무거워지고 있어요.
title,questions,draftAnswers,draftVariant,activeQuestionId상태와handleGenerateDrafts,handleRegenerateDrafts,handleFinish,goBack,renderStep로직이 전부 페이지 컴포넌트 안에 몰려 있어요. 지금 PR에서 단계가 4→5로 늘고title상태까지 추가되면서 페이지 파일이 점점 더 커지고 있습니다.
useCoverLetterFlow같은 커스텀 훅을 만들어features/coverLetter/hooks하위로 상태/핸들러를 옮기면, 페이지는 훅에서 값을 받아 렌더링만 담당하게 되어 유지보수가 훨씬 쉬워져요.// features/coverLetter/hooks/useCoverLetterFlow.js export function useCoverLetterFlow(navigate) { const [step, setStep] = useState(1); const [title, setTitle] = useState(''); // ...나머지 상태/핸들러 return { step, title, setTitle, goNext, goBack, /* ... */ }; }당장 급한 버그는 아니라서 이번 PR에서 꼭 처리하지 않아도 되지만, 다음 단계가 추가될 걸 생각하면 미리 정리해두는 게 좋을 것 같아요!
As per path instructions: "페이지 컴포넌트는 얇게 유지하는 것이 이 프로젝트의 원칙입니다. 비즈니스 로직이나 복잡한 UI가 페이지에 직접 들어 있으면 features/ 하위 도메인 폴더로 분리하도록 제안해주세요."
🤖 Prompt for 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. In `@src/pages/ai/CoverLetterPage.jsx` around lines 55 - 101, 페이지 컴포넌트의 상태와 비즈니스 로직이 과도하게 집중되어 있으므로, `features/coverLetter/hooks`에 `useCoverLetterFlow` 커스텀 훅을 추가해 `title`, `questions`, `draftAnswers`, `draftVariant`, `activeQuestionId` 상태와 `handleGenerateDrafts`, `handleRegenerateDrafts`, `handleFinish`, `goBack`, `renderStep` 로직을 이동하세요. 훅은 페이지가 렌더링에 필요한 상태와 핸들러를 반환하고, `CoverLetterPage`는 이를 받아 단계 UI를 렌더링하는 얇은 컴포넌트로 유지하세요.Source: Path instructions
🧹 Nitpick comments (2)
src/pages/ai/CoverLetterPage.jsx (1)
77-87: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
activeIndex가 -1이 될 가능성도 살짝 방어해두면 좋아요.
questions.findIndex가 못 찾으면 -1을 반환하는데, 이 경우questions[-1]은undefined가 되어CoverLetterStep6의question.content에서 에러가 날 수 있어요. 지금 흐름에서는activeQuestionId가 항상questions배열에서 온 id라 실제로 터질 일은 없어 보이지만, 나중에 로직이 바뀌면 조용히 문제가 될 수 있으니 참고해주세요.🤖 Prompt for 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. In `@src/pages/ai/CoverLetterPage.jsx` around lines 77 - 87, In the case 5 branch of CoverLetterPage, guard the activeIndex result from questions.findIndex before rendering CoverLetterStep6. If activeIndex is -1, avoid passing questions[activeIndex] and follow the existing safe fallback behavior; preserve the current rendering path when the question is found.src/features/coverLetter/components/CoverLetterStep4.jsx (1)
32-68: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win문항 내용이 비어있어도 다음 단계로 넘어갈 수 있어요.
Step3에서는 제목이 비어있으면
disabled={title.trim() === ''}로 막아주는데, 여기 "초안 생성하기" 버튼은 문항 내용이 하나도 없어도 그냥 눌려요. 그러면 빈 문항으로 초안 생성이 진행돼서 다음 화면(Step5/6)에 빈 카드가 뜰 수 있어요.Step3와 같은 패턴으로 맞춰주면 좋을 것 같아요:
+ const hasEmptyContent = questions.some((q) => q.content.trim() === ''); + const handleGenerateDraft = () => { // TODO: 백엔드 연동 시 AI 초안 생성 API 요청 onNext(); };<BottomArea> - <PrimaryButton onClick={handleGenerateDraft}> + <PrimaryButton onClick={handleGenerateDraft} disabled={hasEmptyContent}> 자소서 초안 생성하기(Step3처럼
PrimaryButton에:disabled스타일도 함께 추가해주면 좋아요!)🤖 Prompt for 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. In `@src/features/coverLetter/components/CoverLetterStep4.jsx` around lines 32 - 68, Update handleGenerateDraft and the “자소서 초안 생성하기” PrimaryButton so draft generation is disabled when every question’s trimmed content is empty, matching Step3’s title validation pattern. Pass the disabled state to PrimaryButton and add the corresponding disabled styling consistent with Step3, while preserving generation when at least one question contains content.
🤖 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.
Outside diff comments:
In `@src/features/coverLetter/components/CoverLetterStep5.jsx`:
- Around line 1-50: Connect selectedIds to a meaningful action in
CoverLetterStep5 by adding a button that copies the selected draft answers
together, using the existing handleCopy logic or an equivalent multi-item
handler; ensure the action is disabled or safely handles an empty selection. If
multi-copy is out of scope, remove selectedIds, isAllSelected,
handleToggleSelectAll, the select-all button, and the selected prop instead of
keeping nonfunctional selection UI.
In `@src/pages/ai/CoverLetterPage.jsx`:
- Around line 55-101: 페이지 컴포넌트의 상태와 비즈니스 로직이 과도하게 집중되어 있으므로,
`features/coverLetter/hooks`에 `useCoverLetterFlow` 커스텀 훅을 추가해 `title`,
`questions`, `draftAnswers`, `draftVariant`, `activeQuestionId` 상태와
`handleGenerateDrafts`, `handleRegenerateDrafts`, `handleFinish`, `goBack`,
`renderStep` 로직을 이동하세요. 훅은 페이지가 렌더링에 필요한 상태와 핸들러를 반환하고, `CoverLetterPage`는 이를 받아
단계 UI를 렌더링하는 얇은 컴포넌트로 유지하세요.
---
Nitpick comments:
In `@src/features/coverLetter/components/CoverLetterStep4.jsx`:
- Around line 32-68: Update handleGenerateDraft and the “자소서 초안 생성하기”
PrimaryButton so draft generation is disabled when every question’s trimmed
content is empty, matching Step3’s title validation pattern. Pass the disabled
state to PrimaryButton and add the corresponding disabled styling consistent
with Step3, while preserving generation when at least one question contains
content.
In `@src/pages/ai/CoverLetterPage.jsx`:
- Around line 77-87: In the case 5 branch of CoverLetterPage, guard the
activeIndex result from questions.findIndex before rendering CoverLetterStep6.
If activeIndex is -1, avoid passing questions[activeIndex] and follow the
existing safe fallback behavior; preserve the current rendering path when the
question is found.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 75bd2500-8fd7-448e-b34d-fe3636dfa147
📒 Files selected for processing (5)
src/features/coverLetter/components/CoverLetterStep3.jsxsrc/features/coverLetter/components/CoverLetterStep4.jsxsrc/features/coverLetter/components/CoverLetterStep5.jsxsrc/features/coverLetter/components/CoverLetterStep6.jsxsrc/pages/ai/CoverLetterPage.jsx
2 participants
☘️ 작업한 이슈
🍀 작업한 내용
Step5(개별 문항 상세)를 Step4Step6으로 재정렬🍃 작업 포인트
maxLength+slice로 이중 방어)radius.full완전 pill 형태로 Figma와 동일하게 맞춤📷 작업 GIF
Summary by CodeRabbit