-
Notifications
You must be signed in to change notification settings - Fork 1
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
Feat: #188 일정 상세 모달 수정 버튼 기능 구현 (상세 모달 → 수정 모달 전환) #190
Conversation
Walkthrough이 풀 리퀘스트는 Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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: 0
🧹 Outside diff range and nitpick comments (8)
src/components/task/kanban/TaskItem.tsx (2)
19-20
: 모달 관리 로직이 개선되었습니다.상세 모달과 수정 모달을 독립적으로 제어할 수 있도록
useModal
훅 사용이 업데이트되었습니다. 이는 PR의 목표에 부합하는 변경사항입니다.가독성 향상을 위해 다음과 같이 구조 분해 할당을 사용하는 것을 고려해 보세요:
const [ { showModal: showDetailModal, openModal: openDetailModal, closeModal: closeDetailModal }, { showModal: showUpdateModal, openModal: openUpdateModal, closeModal: closeUpdateModal } ] = [useModal(), useModal()];
50-50
: UpdateModalTask 컴포넌트가 적절히 추가되었습니다.수정 모달 기능을 구현하기 위해
UpdateModalTask
컴포넌트가 추가되었습니다. 이는 PR의 목표에 부합하는 변경사항입니다.
project
와task
객체를 전달하는 대신, 필요한 속성만 전달하는 것이 좋습니다. 예를 들어:<UpdateModalTask projectId={project.id} taskId={task.taskId} onClose={closeUpdateModal} />이렇게 하면 컴포넌트 간의 결합도를 낮출 수 있습니다.
src/pages/project/CalendarPage.tsx (3)
40-48
: selectedTask 상태 초기화 개선기본 Task 객체로 selectedTask 상태를 초기화한 것은 좋은 접근입니다. 이는 항상 일관된 구조를 보장합니다.
타입 안전성을 더욱 높이기 위해 다음과 같은 개선을 고려해 보세요:
const defaultTask: Task = { taskId: 0, statusId: 0, name: '', content: '', startDate: '', endDate: '', sortOrder: 0, }; const [selectedTask, setSelectedTask] = useState<Task>(defaultTask);이렇게 하면 기본 Task 객체를 재사용할 수 있고, 코드의 가독성도 향상됩니다.
58-61
: handleSelectEvent 함수 변경 승인event.task 객체를 구조 분해하고 새로운 Task 객체를 생성하는 방식으로 변경한 것은 좋습니다. 이는 선택된 작업의 구조를 명확히 하고 타입 안전성을 높입니다.
코드의 가독성을 더욱 높이기 위해 다음과 같은 개선을 고려해 보세요:
const handleSelectEvent = (event: CustomEvent) => { const { task } = event; setSelectedTask(task); openDetailModal(); };이렇게 하면 코드가 더 간결해지고, CustomEvent 타입이 올바르게 정의되어 있다면 타입 안전성도 유지됩니다.
135-143
: 모달 렌더링 로직 변경 승인DetailModalTask와 UpdateModalTask의 조건부 렌더링 로직이 잘 구현되었습니다. 특히 DetailModalTask에 openUpdateModal prop을 전달한 것은 PR의 주요 목적인 상세 모달에서 수정 모달로의 전환 기능을 구현하는 데 핵심적인 부분입니다.
prop 타입의 안전성을 높이기 위해 다음과 같은 개선을 고려해 보세요:
- DetailModalTask와 UpdateModalTask 컴포넌트에 대한 prop 타입을 명시적으로 정의합니다.
- 필요한 경우 prop의 필수 여부를 지정합니다.
예를 들어:
type DetailModalTaskProps = { project: Project; task: Task; openUpdateModal: () => void; onClose: () => void; }; type UpdateModalTaskProps = { project: Project; taskId: number; onClose: () => void; };이렇게 하면 컴포넌트 사용 시 타입 오류를 미리 방지할 수 있습니다.
src/components/modal/task/DetailModalTask.tsx (2)
21-21
: 타입 정의에 openUpdateModal 추가 승인
openUpdateModal
프롭의 추가는 PR의 목표와 잘 일치합니다. 상세 모달에서 수정 모달로의 전환 기능을 구현하는 데 필요한 변경사항입니다.타입 안전성을 더욱 높이기 위해 다음과 같이 변경하는 것을 고려해보세요:
openUpdateModal: () => void;대신
openUpdateModal: VoidFunction;이렇게 하면 의도를 더 명확히 표현할 수 있습니다.
39-42
: handleUpdateClick 함수 구현 승인 및 개선 제안
handleUpdateClick
함수의 구현은 PR의 목표를 달성하는 데 적합합니다. 수정 모달을 열고 현재 모달을 닫는 순서가 논리적으로 보입니다.만약
openUpdateModal
이 비동기 작업을 포함할 가능성이 있다면, 다음과 같이 개선할 수 있습니다:const handleUpdateClick = async () => { await openUpdateModal(); handleClose(); };이렇게 하면
openUpdateModal
작업이 완료된 후에 현재 모달이 닫히므로, 잠재적인 레이스 컨디션이나 의도치 않은 UI 동작을 방지할 수 있습니다.src/pages/project/KanbanPage.tsx (1)
Line range hint
28-42
: '!' 연산자의 사용 최소화 및 에러 처리 필요
find
메서드를 사용할 때 결과가undefined
일 수 있으므로, '!' 연산자를 사용하여 단언하기보다는 조건문을 통해null
또는undefined
여부를 확인하는 것이 좋습니다. 이는 런타임 에러를 방지하고 코드의 안정성을 높입니다.코드 수정 제안:
- const sourceTasks = newStatusTasks.find((data) => data.statusId === sourceStatusId)!.tasks; + const sourceStatusData = newStatusTasks.find((data) => data.statusId === sourceStatusId); + if (!sourceStatusData) throw Error('유효하지 않은 sourceStatusId입니다.'); + const sourceTasks = sourceStatusData.tasks; - const destinationTasks = isSameStatus - ? sourceTasks - : newStatusTasks.find((data) => data.statusId === destinationStatusId)!.tasks; + const destinationStatusData = newStatusTasks.find((data) => data.statusId === destinationStatusId); + if (!destinationStatusData) throw Error('유효하지 않은 destinationStatusId입니다.'); + const destinationTasks = isSameStatus ? sourceTasks : destinationStatusData.tasks; - const task = sourceTasks.find((data) => data.taskId === taskId)! as Task; + const foundTask = sourceTasks.find((data) => data.taskId === taskId); + if (!foundTask) throw Error('유효하지 않은 taskId입니다.'); + const task = foundTask as Task;🧰 Tools
🪛 Biome
[error] 25-28: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- src/components/modal/task/DetailModalTask.tsx (2 hunks)
- src/components/task/kanban/TaskItem.tsx (3 hunks)
- src/pages/project/CalendarPage.tsx (3 hunks)
- src/pages/project/KanbanPage.tsx (2 hunks)
🧰 Additional context used
🪛 Biome
src/pages/project/KanbanPage.tsx
[error] 25-28: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
🔇 Additional comments (11)
src/components/task/kanban/TaskItem.tsx (4)
8-8
: 새로운 import 문이 적절히 추가되었습니다.
UpdateModalTask
컴포넌트를 가져오는 import 문이 추가되었습니다. 이는 PR의 목표인 수정 모달 기능 구현에 필요한 변경사항입니다.
26-26
: 태스크 클릭 핸들러가 적절히 수정되었습니다.
handleTaskClick
함수가 상세 모달을 여는 것으로 변경되었습니다. 이는 새로운 모달 관리 방식과 일치하며, PR의 목표에 부합합니다.
47-49
: DetailModalTask 렌더링이 적절히 업데이트되었습니다.
DetailModalTask
컴포넌트에openUpdateModal
prop이 추가되었습니다. 이를 통해 상세 모달에서 수정 모달로의 전환이 가능해졌으며, PR의 목표에 부합합니다.
Line range hint
1-53
: 전반적인 변경 사항이 PR 목표에 잘 부합합니다.이 PR의 주요 목표인 상세 모달에서 수정 모달로의 전환 기능이 성공적으로 구현되었습니다. 주요 변경 사항은 다음과 같습니다:
- 새로운
UpdateModalTask
컴포넌트 import- 상세 모달과 수정 모달을 위한 독립적인 모달 제어 로직
- 태스크 클릭 시 상세 모달 열기
DetailModalTask
에 수정 모달 열기 기능 추가UpdateModalTask
컴포넌트 렌더링 추가이러한 변경 사항들은 칸반 보드와 캘린더 뷰에서 상세 모달에서 수정 모달로의 원활한 전환을 가능하게 합니다. 몇 가지 minor한 개선 사항을 제안했지만, 전반적으로 코드 품질이 우수하며 PR의 목표를 잘 달성했습니다.
src/pages/project/CalendarPage.tsx (2)
16-16
: import 문 변경 승인
Task
타입과DetailModalTask
컴포넌트의 추가 import는 PR의 목적에 부합하며, 새로운 기능 구현에 필요한 요소들을 적절히 가져오고 있습니다.Also applies to: 18-18
52-53
: 모달 처리 로직 개선 승인상세 모달과 수정 모달을 독립적으로 제어할 수 있도록 변경한 것은 매우 적절합니다. 이는 PR의 주요 목적인 상세 모달에서 수정 모달로의 전환 기능을 구현하는 데 필수적인 변경사항입니다.
src/components/modal/task/DetailModalTask.tsx (2)
25-25
: DetailModalTask 컴포넌트 매개변수에 openUpdateModal 추가 승인
DetailModalTask
컴포넌트의 매개변수에openUpdateModal
을 추가한 것은 타입 정의 변경사항과 일치하며, PR의 목표를 달성하는 데 필요한 적절한 변경입니다.
Line range hint
1-138
: 전체 변경사항 요약 및 PR 목표 달성 확인이 PR의 변경사항들은 일정 상세 모달에서 수정 모달로의 전환 기능을 성공적으로 구현했습니다. 주요 변경사항은 다음과 같습니다:
ViewModalTaskProps
타입에openUpdateModal
프롭 추가DetailModalTask
컴포넌트에openUpdateModal
매개변수 추가handleUpdateClick
함수 구현으로 모달 전환 로직 구현이러한 변경사항들은 PR의 목표인 "일정 상세 모달 수정 버튼 기능 구현 (상세 모달 → 수정 모달 전환)"을 정확히 달성하고 있습니다. 제안된 minor 개선사항들을 고려하여 구현을 더욱 견고하게 만들 수 있을 것입니다.
src/pages/project/KanbanPage.tsx (3)
12-15
: 함수createChangedStatus
의 로직이 정확합니다상태 순서 변경을 위한 로직이 올바르게 구현되어 있습니다. 깊은 복사를 통해 원본 데이터를 보호하고 있으며,
sortOrder
를 업데이트하는 처리도 적절합니다.
Line range hint
44-56
:useEffect
훅에서의 상태 관리가 올바릅니다
statusTaskList
의 변경에 따라localStatusTaskList
를 업데이트하는 로직이 정확하게 구현되어 있습니다.
Line range hint
58-75
:handleDragEnd
함수의 이벤트 처리 로직이 정확합니다드래그 종료 시 타입에 따라 적절한 함수를 호출하여 상태 및 일정의 순서 변경을 효과적으로 처리하고 있습니다.
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.
모달 전환 고생하셨습니다! 경로 한번만 확인해주시면 될거같아요~
@Yoonyesol @ice-bear98 리뷰 감사합니다! |
PR Type
What kind of change does this PR introduce?
Related Issues
What does this PR do?
Other information
이전에 타입 수정으로 인해 Calendar 페이지에서 타입 호환 문제가 있는 것 같습니다. 이는 다음 PR에서 처리하도록 하겠습니다.
View
캘린더에서의 수정 모달 전환
칸반보드에서의 수정 모달 전환