Conversation
Summary of ChangesHello, 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! 이 PR은 React와 Vite를 활용하여 기본적인 할 일 관리 애플리케이션의 개발을 완료합니다. 주요 기능으로는 할 일 추가, 삭제, 완료 상태 토글, 그리고 localStorage를 통한 데이터 영구 저장이 포함됩니다. 또한, 메인 페이지에서는 현재 할 일의 진행 상황을 한눈에 볼 수 있도록 통계 정보를 제공하여 사용자 경험을 향상시켰습니다. 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. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
src/hooks/useTodos.js
Outdated
| useEffect(() => { | ||
| saveTodos('todos', todos) | ||
| }) |
There was a problem hiding this comment.
useEffect에 의존성 배열이 없어 컴포넌트가 리렌더링될 때마다 saveTodos 함수가 불필요하게 호출됩니다. 이는 성능 저하의 원인이 될 수 있습니다. todos 상태가 변경될 때만 localStorage에 저장하도록 의존성 배열에 [todos]를 추가하는 것이 좋습니다.
| useEffect(() => { | |
| saveTodos('todos', todos) | |
| }) | |
| useEffect(() => { | |
| saveTodos('todos', todos) | |
| }, [todos]) |
| export function useTodoContext() { | ||
| const context = useContext(TodoContext) | ||
| return context | ||
| } |
There was a problem hiding this comment.
useTodoContext 훅이 TodoProvider 외부에서 사용될 경우 context 값이 undefined가 되어 에러가 발생할 수 있습니다. 개발 중에 이런 실수를 쉽게 발견하고 디버깅을 용이하게 하기 위해, context가 undefined일 때 명시적으로 에러를 발생시키는 것이 좋은 습관입니다.
| export function useTodoContext() { | |
| const context = useContext(TodoContext) | |
| return context | |
| } | |
| export function useTodoContext() { | |
| const context = useContext(TodoContext) | |
| if (context === undefined) { | |
| throw new Error('useTodoContext must be used within a TodoProvider') | |
| } | |
| return context | |
| } |
| } catch (err) { | ||
| console.log('load error') | ||
| return null | ||
| } |
There was a problem hiding this comment.
localStorage에서 데이터를 불러오는 중 에러가 발생했을 때 null을 반환하고 있습니다. useTodos 훅에서 useState(getTodos('todos'))의 초기값으로 null이 들어가게 되면, todos를 배열로 예상하고 사용하는 다른 컴포넌트(예: Main.jsx의 todos.length)에서 런타임 에러가 발생할 수 있습니다. 에러 발생 시에도 빈 배열 []을 반환하여 예상치 못한 에러를 방지하는 것이 더 안전합니다.
| } catch (err) { | |
| console.log('load error') | |
| return null | |
| } | |
| } catch (err) { | |
| console.error('Failed to load todos from localStorage', err) | |
| return [] | |
| } |
| const handleOnSubmit = (e) => { | ||
| e.preventDefault() // form 기본 동작(페이지 새로고침) 방지 | ||
| const form = e.target | ||
| // 공백만 입력한 경우도 막기 위해 trim() 사용 | ||
| if (form.input.value.trim().length == 0) { | ||
| alert('할 일을 입력해주세요') | ||
| return | ||
| } | ||
| addTodo(form.input.value) | ||
| form.input.value = '' | ||
| } |
There was a problem hiding this comment.
현재 폼의 입력값을 DOM에 직접 접근하여 가져오고 있습니다 (e.target.input.value). React에서는 useState를 사용하여 입력값을 상태로 관리하는 '제어 컴포넌트(Controlled Component)' 패턴을 사용하는 것이 더 안정적이고 일반적입니다. 이렇게 하면 React의 상태 관리 시스템 안에서 값을 다룰 수 있어 코드가 더 예측 가능해집니다.
아래와 같이 리팩토링하는 것을 추천합니다.
useState를 import하고 컴포넌트 상단에 상태를 추가합니다.
import { useState } from 'react'
// ...
const [text, setText] = useState('')handleOnSubmit함수를 수정하여 state를 사용합니다.
const handleOnSubmit = (e) => {
e.preventDefault()
if (text.trim().length === 0) {
alert('할 일을 입력해주세요')
return
}
addTodo(text)
setText('')
}<input>엘리먼트를 수정하여 state와 연결합니다.
<input
// ... other props
value={text}
onChange={(e) => setText(e.target.value)}
/>| } | ||
|
|
||
| const removeTodo = (id) => { | ||
| const filterTodo = todos.filter((todo) => todo.id != id) |
There was a problem hiding this comment.
느슨한 비교(!=) 대신 타입까지 엄격하게 비교하는 !== 연산자를 사용하는 것이 예기치 않은 타입 변환으로 인한 버그를 방지하는 데 도움이 됩니다.
| const filterTodo = todos.filter((todo) => todo.id != id) | |
| const filterTodo = todos.filter((todo) => todo.id !== id) |
| } | ||
|
|
||
| const toggleTodo = (id) => { | ||
| const updateTodo = todos.map((todo) => (todo.id == id ? { ...todo, completed: !todo.completed } : todo)) |
There was a problem hiding this comment.
느슨한 비교(==) 대신 타입까지 엄격하게 비교하는 === 연산자를 사용하는 것이 예기치 않은 타입 변환으로 인한 버그를 방지하는 데 도움이 됩니다.
| const updateTodo = todos.map((todo) => (todo.id == id ? { ...todo, completed: !todo.completed } : todo)) | |
| const updateTodo = todos.map((todo) => (todo.id === id ? { ...todo, completed: !todo.completed } : todo)) |
| setTodos([]) | ||
| } | ||
|
|
||
| return { todos, setTodos, addTodo, removeTodo, toggleTodo, removeAllTodos } |
There was a problem hiding this comment.
setTodos를 직접 노출하면 이 hook을 사용하는 컴포넌트에서 addTodo, removeTodo 등의 로직을 거치지 않고 todos 상태를 마음대로 변경할 수 있습니다. 이는 상태 관리의 일관성을 해칠 수 있습니다. hook의 캡슐화를 위해 setTodos를 직접 반환하기보다는, 상태 변경이 필요할 때 사용할 수 있는 함수들(addTodo, removeTodo 등)만 노출하는 것이 좋습니다.
| return { todos, setTodos, addTodo, removeTodo, toggleTodo, removeAllTodos } | |
| return { todos, addTodo, removeTodo, toggleTodo, removeAllTodos } |
| const leftCount = todos.filter((todo) => !todo.completed).length | ||
| const completedCount = todos.filter((todo) => todo.completed).length | ||
| const percent = count === 0 ? 0 : Math.round((completedCount / count) * 100) |
There was a problem hiding this comment.
leftCount와 completedCount를 계산하기 위해 todos 배열을 두 번 순회하고 있습니다. 할 일의 개수가 많아지면 성능에 미미한 영향을 줄 수 있습니다. reduce를 사용하여 배열을 한 번만 순회하면서 두 값을 동시에 계산하면 더 효율적입니다.
const { leftCount, completedCount } = todos.reduce(
(acc, todo) => {
if (todo.completed) {
acc.completedCount++
} else {
acc.leftCount++
}
return acc
},
{ leftCount: 0, completedCount: 0 },
)
const count = todos.length
const percent = count === 0 ? 0 : Math.round((completedCount / count) * 100)
|
removeAllTodo, 진행률을 나타내는 ui 좋은 기능인것 같아요!! 역시 킹락현 |
|
todo.id 를 lastId.current + 1 의 방식이 아닌 crypto.randomUUID() 를 이용하여 고유의 id 를 부여하는 게 인상적입니다! 진행률 프로그래스바와 UI도 사용자가 사용하기에 간편해보입니다! |
|
UI 면에서 있어서 잘 짜여진 코드 인거 같네요 |
@Sinou3936 한 화면에 UI를 넣으면 간단한 TODO 관리 앱이라 가볍게 사용하기 좋을 것 같아요! |
|
완료한 할일과 진행률을 보여주는 기능이 인상 깊어요. 사용자가 사용하기 편해보입니다. |
5 participants
✅ 기능 구현
📁 프로젝트 구조
💻 실행 화면
스크린 샷
영상
❗참고 사항
디자인 관련
피드백 관련