Todo 기능 구현#2
Conversation
Dobbymin
added
✨ Feature
Summary of ChangesHello @Dobbymin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 Pull Request는 캘린더 페이지의 핵심 기능을 구현하여 사용자가 월별 캘린더에서 일정 수를 한눈에 확인하고, 특정 날짜를 선택하여 우측 패널에서 해당 일정을 생성, 관리, 완료/미완료 처리할 수 있도록 합니다. 이를 통해 캘린더 기반의 효율적인 일정 관리 경험을 제공합니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이 PR은 캘린더 페이지의 핵심적인 Todo 기능들을 구현했습니다. 전반적으로 Zustand를 이용한 상태 관리, React Query를 통한 데이터 페칭, 그리고 컴포넌트 구조화가 잘 이루어져 있습니다. 다만, 몇 가지 개선할 점들이 보입니다. API 에러 메시지의 구체화, 일부 훅과 컴포넌트의 성능 및 재사용성 개선, 그리고 사용자 경험(UX) 향상을 위한 제안들을 포함했습니다. 특히 window.prompt 사용과 같은 부분은 더 나은 UI/UX를 위해 개선이 필요해 보입니다. 자세한 내용은 각 파일에 대한 리뷰 코멘트를 참고해주세요.
| }); | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error("Failed to create todo"); |
There was a problem hiding this comment.
에러 메시지가 "Failed to create todo"로 되어 있어, 실제 기능인 카테고리 생성과 맞지 않습니다. "Failed to create category" 또는 한글로 "카테고리 생성에 실패했습니다."와 같이 명확한 메시지로 수정하는 것이 디버깅에 도움이 됩니다. 또한, 다른 API 함수들에서도 유사한 문제가 발견되었습니다. 애플리케이션 전반의 에러 메시지를 한국어로 통일하고, 각 API 기능에 맞는 구체적인 메시지를 사용하시는 것을 권장합니다.
| throw new Error("Failed to create todo"); | |
| throw new Error("카테고리 생성에 실패했습니다"); |
| }); | ||
|
|
||
| const handleCreateCategory = () => { | ||
| const name = window.prompt("추가할 카테고리 이름을 입력해주세요."); |
There was a problem hiding this comment.
window.prompt()를 사용하여 사용자 입력을 받는 것은 현대적인 웹 애플리케이션의 사용자 경험(UX)과 어울리지 않으며, UI 커스터마이징이 불가능합니다. Chakra UI의 Modal이나 Popover와 Input 컴포넌트를 조합하여 더 나은 사용자 경험을 제공하는 입력창을 구현하는 것을 강력히 권장합니다.
| dayjs.extend(weekday); | ||
| dayjs.extend(isoWeek); | ||
| dayjs.extend(weekOfYear); |
There was a problem hiding this comment.
dayjs.extend() 함수가 컴포넌트 함수 내에서 호출되고 있어, 컴포넌트가 리렌더링될 때마다 불필요하게 실행됩니다. 플러그인 확장은 애플리케이션 로드 시 한 번만 실행되면 충분하므로, 이 코드를 모듈의 최상단(import문 아래)으로 이동시켜 성능 저하를 방지하는 것이 좋습니다.
| const [activeInputId, setActiveInputId] = useState<number | null>(null); | ||
| const [inputValue, setInputValue] = useState(""); |
There was a problem hiding this comment.
현재 inputValue 상태가 모든 카테고리의 할 일 추가 입력창에서 공유되고 있습니다. 이로 인해 한 카테고리에서 텍스트를 입력한 후 다른 카테고리의 입력창을 열면 이전에 입력한 내용이 그대로 남아있어 사용자 경험을 해칠 수 있습니다. 카테고리를 변경할 때 입력값을 초기화하거나, 각 카테고리별로 입력 상태를 관리하는 것이 좋습니다. 예를 들어, 카테고리 버튼의 onClick 핸들러에서 setInputValue("")를 호출하여 입력창을 초기화할 수 있습니다.
| const selectedDate = useTodosStore((state) => state.selectedDate); | ||
| const setSelectedDate = useTodosStore((state) => state.setSelectedDate); | ||
|
|
||
| const updateSelectedDate = (date: string | null) => { | ||
| setSelectedDate(date); | ||
| }; | ||
|
|
||
| return { selectedDate, updateSelectedDate }; |
There was a problem hiding this comment.
updateSelectedDate 함수가 useSelectedDate 훅이 렌더링될 때마다 새로 생성되고 있습니다. 이 함수가 useEffect의 의존성 배열에 사용되거나 메모이제이션된 컴포넌트의 prop으로 전달될 경우 불필요한 리렌더링을 유발할 수 있습니다. Zustand의 setter 함수는 안정적이므로, setSelectedDate를 직접 사용하거나 useCallback으로 감싸서 안정적인 함수를 반환하는 것이 좋습니다. 더 간단하고 효율적인 방법은 setSelectedDate를 직접 가져와서 반환하는 것입니다.
const selectedDate = useTodosStore((state) => state.selectedDate);
const updateSelectedDate = useTodosStore((state) => state.setSelectedDate);
return { selectedDate, updateSelectedDate };|
|
||
| export const updateTodoAPI = async (todoId: number, isCompleted: boolean) => { | ||
| const response = await fetch(`${BASE_URL}/api/todos/${todoId}`, { | ||
| method: "PUT", |
There was a problem hiding this comment.
PR 설명에서는 PATCH 요청으로 isCompleted 상태를 토글한다고 되어 있는데, 실제 코드에서는 PUT 메소드를 사용하고 있습니다. PATCH는 리소스의 일부만 수정할 때 사용하는 것이 의미상 더 적절합니다. 백엔드 API가 PATCH를 지원한다면 메소드를 변경하는 것을 고려해보세요. 만약 PUT이 의도된 것이라면 PR 설명을 수정하여 일관성을 맞추는 것이 좋겠습니다.
| method: "PUT", | |
| method: "PATCH", |
| export * from "./calendar"; | ||
| export * from "./category"; | ||
| export * from "./link"; | ||
| export * from "./category"; |
There was a problem hiding this comment.
./category 모듈이 중복으로 export 되고 있습니다. 불필요한 코드는 제거하여 코드 청결성을 유지하는 것이 좋습니다.
| // 오늘 날짜 여부 | ||
| const isToday = dayjs().format("YYYYMMDD") === current.format("YYYYMMDD"); | ||
| // 보여질 날짜가 아닌 경우 (다른 달의 날짜인 경우) | ||
| const isNone = current.format("MM") === viewDate.format("MM") ? "" : "none"; |
There was a problem hiding this comment.
isNone 변수에 문자열("" 또는 "none")을 할당하여 삼항 연산자에서 사용하는 방식은 코드의 가독성을 떨어뜨릴 수 있습니다. current.format("MM") !== viewDate.format("MM")와 같이 boolean 값을 반환하는 isOutOfMonth 같은 변수를 선언하여 명시적으로 true/false를 사용하면 코드를 이해하기가 더 쉬워집니다. 이후 opacity={isOutOfMonth ? 0 : 1}와 같이 사용할 수 있습니다.
| const isNone = current.format("MM") === viewDate.format("MM") ? "" : "none"; | |
| const isOutOfMonth = current.format("MM") !== viewDate.format("MM"); |
| onSuccess: () => { | ||
| queryClient.invalidateQueries({ | ||
| queryKey: requestDate ? TODO_QUERY_KEYS.todos.list(requestDate) : TODO_QUERY_KEYS.todos.all, | ||
| }); | ||
| }, | ||
| }); | ||
|
|
||
| const { mutate: updateTodoMutate } = useMutation({ | ||
| mutationFn: ({ todoId, isCompleted }: { todoId: number; isCompleted: boolean }) => | ||
| updateTodoAPI(todoId, isCompleted), | ||
| onSuccess: () => { | ||
| queryClient.invalidateQueries({ | ||
| queryKey: requestDate ? TODO_QUERY_KEYS.todos.list(requestDate) : TODO_QUERY_KEYS.todos.all, | ||
| }); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
createTodoMutate와 updateTodoMutate의 onSuccess 핸들러에서 쿼리를 무효화하는 로직이 중복되고 있습니다. 또한, requestDate는 selectedDate ?? dayjs().format("YYYY-MM-DD")로 인해 항상 문자열 값을 가지므로, requestDate ? ... : ... 조건문은 불필요합니다. 이 로직을 별도의 함수로 추출하여 중복을 제거하고, 불필요한 조건문을 삭제하여 코드를 간결하게 만들 수 있습니다.
| export const changeMonth = (setViewDate: (date: Dayjs) => void, viewDate: Dayjs, action: MonthChangeAction): void => { | ||
| setViewDate(getChangedMonth(viewDate, action)); | ||
| }; |
There was a problem hiding this comment.
changeMonth 함수가 export되고 있지만, 프로젝트 내에서 사용되지 않는 것으로 보입니다. 불필요한 코드는 혼란을 줄 수 있으므로, 사용하지 않는다면 제거하는 것이 좋습니다.
There was a problem hiding this comment.
Pull request overview
This PR implements the core TODO functionality for a calendar application, enabling users to visualize and manage daily tasks. The implementation follows Feature-Sliced Design architecture with proper separation between entities, features, and shared layers.
Key Changes:
- Calendar UI with monthly view and todo count visualization overlay
- Date selection state management using Zustand with React Query for data fetching
- Complete CRUD operations for todos and categories with API integration
Reviewed changes
Copilot reviewed 48 out of 54 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
| src/features/main/ui/TodoSection.tsx | Main todo management UI with create, toggle, and display functionality |
| src/features/main/ui/CalendarSection.tsx | Calendar view component with date selection and month navigation |
| src/features/main/apis/todos.api.ts | Todo API endpoints for CRUD operations |
| src/features/main/apis/category.api.ts | Category creation API endpoint |
| src/features/main/components/features/calendar/* | Calendar UI components (CalendarDays, CalendarHeader, CalendarWeekdays) |
| src/features/main/components/features/category/Category.tsx | Category creation UI component |
| src/features/main/hooks/useGetTodos.ts | React Query hooks for fetching todos |
| src/entities/todos/model/store/useTodosStore.ts | Zustand store for selected date state management |
| src/entities/todos/model/hooks/useTodos.ts | Custom hook wrapper for selected date state |
| src/entities/todos/model/types/todos.type.ts | TypeScript types for Todo, Category, and related data structures |
| src/shared/constants/base-url.ts | API base URL configuration |
| src/widgets/layouts/components/navigator/Navigator.tsx | Updated navigation icons from outline to colored versions |
| package.json | Added immer dependency for Zustand middleware |
| eslint.config.js | Added prettier/prettier rule configuration |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }); | ||
|
|
||
| const handleCreateCategory = () => { | ||
| const name = window.prompt("추가할 카테고리 이름을 입력해주세요."); |
There was a problem hiding this comment.
Using window.prompt for category creation is not user-friendly and blocks the entire UI. Consider replacing this with a proper modal dialog or inline input component for better UX and accessibility. Window.prompt also has accessibility issues as it's not screen-reader friendly and doesn't support modern form validation.
|
|
||
| export const useGetTodos = (date?: string) => { | ||
| return useQuery({ | ||
| queryKey: TODO_QUERY_KEYS.todos.list(date ?? ""), |
There was a problem hiding this comment.
The query key structure is inconsistent. When date is an empty string, it still creates a query key ["todos", ""] which may lead to unexpected caching behavior. Consider using a different key structure or ensuring date is always a valid string or undefined.
| <Box cursor="pointer" onClick={() => toggleTodoStatus(todo.todosId, todo.isCompleted)}> | ||
| <Flex | ||
| alignItems="center" | ||
| justifyContent="center" | ||
| borderRadius="full" | ||
| boxSize={8} | ||
| position="relative" | ||
| > | ||
| <Icon as={PiFlowerFill} color={todo.isCompleted ? "primary" : "neutral.400"} boxSize={8} /> | ||
| <Flex | ||
| position="absolute" | ||
| alignItems="center" | ||
| justifyContent="center" | ||
| boxSize={5} | ||
| borderRadius="full" | ||
| bg={todo.isCompleted ? "primary" : "neutral.400"} | ||
| > | ||
| {todo.isCompleted && <Icon as={FiCheck} color="white" boxSize={4} />} | ||
| </Flex> | ||
| </Flex> | ||
| </Box> |
There was a problem hiding this comment.
Missing ARIA labels or semantic HTML for the todo completion button. Screen readers won't be able to convey the purpose or state of this interactive element. Consider adding aria-label with dynamic text indicating the todo content and completion state (e.g., "Mark 'task name' as complete" or "Mark 'task name' as incomplete").
| const handleCreateCategory = () => { | ||
| const name = window.prompt("추가할 카테고리 이름을 입력해주세요."); | ||
| if (!name) return; | ||
|
|
||
| createCategoryMutate(name); |
There was a problem hiding this comment.
Missing input validation for empty or whitespace-only category names. Users can create categories with just spaces or empty strings after trimming. Add validation in handleCreateCategory to check if the trimmed name is empty before calling the mutation.
| export * from "./calendar"; | ||
| export * from "./category"; | ||
| export * from "./link"; | ||
| export * from "./category"; |
There was a problem hiding this comment.
The duplicate export of "category" should be removed. This will cause a linting warning and is redundant.
| const changeMonth = (action: MonthChangeAction) => { | ||
| setViewDate(getChangedMonth(viewDate, action)); | ||
| }; |
There was a problem hiding this comment.
The changeMonth function shadows the imported utility function of the same name from line 18. This creates confusion and potential bugs. The local function on line 24 should be renamed to avoid this naming conflict (e.g., handleChangeMonth or onMonthChange).
| @@ -0,0 +1 @@ | |||
| export const BASE_URL = "http://localhost:8081"; | |||
There was a problem hiding this comment.
The BASE_URL is hardcoded to localhost. This will break in production environments. Consider using environment variables (e.g., import.meta.env.VITE_API_BASE_URL) to make the API URL configurable across different environments.
| rounded="full" | ||
| size="sm" | ||
| colorScheme="primary" | ||
| onClick={() => setActiveInputId(activeInputId === category.categoryId ? null : category.categoryId)} |
There was a problem hiding this comment.
The input value is not cleared when the active input changes. If a user types in one category's input, then clicks another category's plus button, the previously typed text will appear in the new input. This should be cleared when activeInputId changes.
| boxSize={8} | ||
| position="relative" | ||
| > | ||
| <Icon as={PiFlowerFill} color={todo.isCompleted ? "primary" : "neutral.400"} boxSize={8} /> |
There was a problem hiding this comment.
The color logic for completed todos appears inverted. When isCompleted is true, the icon color is "primary", but when false, it's "neutral.400". Looking at line 104, the center background also uses "primary" for completed state. However, the PR description mentions the icon should be "white" as default and "neutral.400 + checkmark" when completed. The current implementation doesn't match this description.
| const { mutate: createCategoryMutate } = useMutation({ | ||
| mutationFn: (categoryName: string) => postCategoryAPI(categoryName, requestDate), | ||
| onSuccess: () => { | ||
| queryClient.invalidateQueries({ queryKey: TODO_QUERY_KEYS.todos.list(requestDate) }); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
There's no error handling in the Category component for failed mutation. When category creation fails, the user receives no feedback. Add an onError callback to the mutation configuration to display error messages.
📝 요약 (Summary)
캘린더 페이지의 핵심 기능을 구현했습니다.
월별 캘린더 UI에 일정 수를 시각적으로 표시하고, 날짜 선택 시 우측 패널에서 해당 일정을 관리할 수 있도록 구현했습니다. 일정 생성, 카테고리 추가, 완료/미완료 토글 기능을 모두 연동시켰습니다.
✅ 주요 변경 사항 (Key Changes)
(todos.list(YYYY-MM-DD))로 캐시 최적화💻 상세 구현 내용 (Implementation Details)
1. 상태 관리 (Zustand)
selectedDate상태 및setSelectedDate액션 제공2. 일정 및 카테고리 관리
{ categoryName, todoDate, content }페이로드로 일정 생성{ categoryName, todoDate }페이로드로 카테고리 생성🚀 트러블 슈팅 (Trouble Shooting)
문제 1: 새로 생성한 카테고리에 일정 추가 시 VALIDATION_ERROR
원인: API가
categoryName, todoDate, content를 모두 요구하는데, 기존 코드는todoData, date형식으로 전송해결:
postTodoAPI를 객체 페이로드 기반으로 수정하고, TodoSection에서 카테고리 이름을 함께 전달하도록 변경문제 2: 새로 생성한 카테고리가 UI에 즉시 반영되지 않음
원인: 백엔드 todos_view_flat 뷰가 INNER JOIN 기반이라, 할 일이 0개인 카테고리는 행이 생성되지 않음
해결: 백엔드의 LEFT JOIN 수정으로 빈 카테고리도 포함되도록 함. 프론트는 단순 캐시 무효화(
invalidateQueries)로 리페치문제 3: 할 일 완료 표시 UI 설계
원인: 기존 UI에서 체크박스 느낌의 명확한 완료 표시 부족
해결: 꽃 아이콘 기본 상태를 white 중앙으로, 완료 시 neutral.400 + 체크마크로 변경하여 시각적 구분 강화
📸 스크린샷 (Screenshots)
#️⃣ 관련 이슈 (Related Issues)