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! 이 Pull Request는 Vite를 기반으로 한 React TODO 앱을 구현하는 내용을 담고 있습니다. 기본적인 앱 구조 설정부터 UI 구현, 로컬 스토리지 연동, 그리고 코드 품질 유지를 위한 ESLint 설정까지, TODO 앱 개발의 전반적인 과정을 포함하고 있습니다. 이로써 사용자는 간단하면서도 효과적인 TODO 관리 도구를 사용할 수 있게 되었습니다. 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
|
There was a problem hiding this comment.
Code Review
이 풀 리퀘스트는 Vite와 React를 사용하여 TODO 앱을 구현한 초기 설정입니다. .gitignore, README.md, package.json, package-lock.json, eslint.config.js, index.html 및 SVG 에셋 파일들이 추가되었습니다. 핵심 기능은 useTodos 훅을 통해 TODO 항목을 관리하고, TodoList 및 TodoItem 컴포넌트를 사용하여 UI를 렌더링하는 것입니다. 전반적으로 기능은 잘 작동하지만, 몇 가지 개선할 점이 있습니다. 특히 컴포넌트 내의 인라인 스타일 사용과 DOM 직접 조작 방식은 React의 선언적 특성을 고려하여 개선할 수 있습니다. 또한, useTodos 훅의 lastId 초기화 로직과 storage.js의 타입 검사 로직에 잠재적인 문제가 있습니다.
| const lastId = useRef( | ||
| todos.length === 0 ? 0 : Math.max(...todos.map((item) => item.id)) + 1, | ||
| ); |
There was a problem hiding this comment.
lastId를 초기화하는 로직에서 todos 배열이 비어 있을 경우 Math.max(...todos.map((item) => item.id))는 -Infinity를 반환합니다. 이 경우 lastId.current는 NaN이 되어 새로운 TODO 항목의 ID가 올바르게 생성되지 않을 수 있습니다. todos.length === 0일 때 lastId.current를 0으로 명확하게 설정하는 것이 좋습니다.
| const lastId = useRef( | |
| todos.length === 0 ? 0 : Math.max(...todos.map((item) => item.id)) + 1, | |
| ); | |
| const lastId = useRef( | |
| todos.length === 0 ? 0 : Math.max(...todos.map((item) => item.id)) + 1 | |
| ); |
| ecmaVersion: 2020, | ||
| globals: globals.browser, | ||
| parserOptions: { | ||
| ecmaVersion: 'latest', |
There was a problem hiding this comment.
ESLint 설정에서 languageOptions.ecmaVersion과 parserOptions.ecmaVersion이 모두 지정되어 있습니다. parserOptions.ecmaVersion: 'latest'는 일반적으로 최신 ECMAScript 버전을 의미하므로, languageOptions.ecmaVersion: 2020은 중복되거나 불필요할 수 있습니다. latest를 사용하면 명시적인 버전 지정 없이 최신 문법을 지원할 수 있습니다.
| ecmaVersion: 2020, | |
| globals: globals.browser, | |
| parserOptions: { | |
| ecmaVersion: 'latest', | |
| ecmaVersion: 'latest', |
| onSubmit={handleOnsubmit} | ||
| style={{ | ||
| display: "flex", | ||
| gap: "8px", | ||
| }} | ||
| > | ||
| <input | ||
| type="text" | ||
| name="input" | ||
| placeholder="할일을 입력하세요" | ||
| style={{ | ||
| margin: "10px 20px", | ||
| maxWidth: "250px", | ||
| flex: 1, | ||
| padding: "8px 12px", | ||
| backgroundColor: "#f5f5f5", | ||
| border: "none", | ||
| borderRadius: "6px", | ||
| outline: "none", | ||
| }} | ||
| onFocus={(e) => (e.target.style.backgroundColor = "#ffffff")} | ||
| onBlur={(e) => (e.target.style.backgroundColor = "#f5f5f5")} | ||
| /> | ||
| <button | ||
| type="submit" | ||
| style={{ | ||
| padding: "8px 12px", | ||
| background: "none", | ||
| border: "none", | ||
| color: "#555", | ||
| borderRadius: "6px", | ||
| cursor: "pointer", | ||
| transition: "0.2s", | ||
| }} | ||
| onMouseEnter={(e) => (e.target.style.backgroundColor = "#eaeaea")} | ||
| onMouseLeave={(e) => (e.target.style.backgroundColor = "transparent")} | ||
| > |
There was a problem hiding this comment.
| onFocus={(e) => (e.target.style.backgroundColor = "#ffffff")} | ||
| onBlur={(e) => (e.target.style.backgroundColor = "#f5f5f5")} |
There was a problem hiding this comment.
| }} | ||
| onMouseEnter={(e) => (e.target.style.backgroundColor = "#eaeaea")} |
| <li | ||
| style={{ | ||
| display: "flex", | ||
| alignItems: "center", | ||
| gap: "8px", | ||
| padding: "6px 8px", | ||
| borderRadius: "6px", | ||
| cursor: "pointer", | ||
| textDecoration: todo.completed ? "line-through" : "none", | ||
| listStyle: "none", | ||
| }} | ||
| onMouseEnter={(e) => { | ||
| e.currentTarget.style.backgroundColor = "#f2f2f2"; | ||
| e.currentTarget.querySelector("button").style.opacity = 1; | ||
| }} | ||
| onMouseLeave={(e) => { | ||
| e.currentTarget.style.backgroundColor = "transparent"; | ||
| e.currentTarget.querySelector("button").style.opacity = 0; | ||
| }} |
| onMouseEnter={(e) => { | ||
| e.currentTarget.style.backgroundColor = "#f2f2f2"; | ||
| e.currentTarget.querySelector("button").style.opacity = 1; | ||
| }} | ||
| onMouseLeave={(e) => { | ||
| e.currentTarget.style.backgroundColor = "transparent"; | ||
| e.currentTarget.querySelector("button").style.opacity = 0; | ||
| }} |
There was a problem hiding this comment.
onMouseEnter 및 onMouseLeave 이벤트 핸들러에서 e.currentTarget.style.backgroundColor와 e.currentTarget.querySelector("button").style.opacity를 직접 조작하고 있습니다. React에서는 DOM을 직접 조작하는 것을 피하고, 컴포넌트의 상태를 통해 UI를 업데이트하는 것이 좋습니다. useState 훅을 사용하여 호버 상태를 관리하고, 이 상태에 따라 조건부 스타일을 적용하거나, CSS :hover 가상 클래스를 활용하여 이러한 효과를 구현하는 것이 더 React스러운 접근 방식입니다.
| <ul | ||
| style={{ | ||
| listStyle: "none", | ||
| padding: 0, | ||
| marginTop: "12px", | ||
| maxWidth: "300px", | ||
| margin: "12px", | ||
| }} |
There was a problem hiding this comment.
TodoList 컴포넌트에서도 style 속성을 사용하여 인라인 스타일을 적용하고 있습니다. App.jsx 및 TodoItem.jsx와 동일하게, 스타일을 분리하여 관리하는 것이 좋습니다. 또한, marginTop: "12px"와 margin: "12px"가 함께 사용되고 있는데, margin: "12px"가 모든 방향의 마진을 설정하므로 marginTop을 덮어쓰게 됩니다. 의도한 바가 margin-top만 12px이고 나머지는 0이라면 margin: "12px 0"와 같이 명시적으로 지정하는 것이 좋습니다.
style={{
listStyle: "none",
padding: 0,
margin: "12px 0",
maxWidth: "300px"
}}|
|
||
| if (value === null) return defalutValue; | ||
|
|
||
| if (typeof value === Number) return value; |
There was a problem hiding this comment.
typeof value === Number 조건은 항상 false입니다. localStorage.getItem()은 항상 문자열을 반환하거나 null을 반환하기 때문입니다. 만약 저장된 값이 숫자 타입인지 확인하고 싶다면, JSON.parse(value)를 시도한 후 그 결과의 타입을 확인해야 합니다. 현재 로직에서는 이 줄이 불필요하며, JSON.parse(value)가 숫자를 포함한 모든 유효한 JSON 값을 올바르게 처리할 것입니다. 또한, JSON.parse는 유효하지 않은 JSON 문자열에 대해 오류를 발생시킬 수 있으므로 try-catch 블록으로 감싸는 것이 좋습니다.
| if (typeof value === Number) return value; | |
| try { | |
| const parsedValue = JSON.parse(value); | |
| return parsedValue; | |
| } catch (error) { | |
| console.error("Error parsing stored value:", error); | |
| return defalutValue; | |
| } |
|
css를 깔끔하게 작성하셔서 보기 편하네요! |
|
필수 기능을 잘 구현해주신 것 같습니다! |
|
기능구현도 잘 해주신 것 같고 css도 미니멀하면서 깔끔한것 같습니다. |
📌 개요
React를 사용하여 구현한 ToDo List 애플리케이션입니다.
컴포넌트 분리와 커스텀 훅을 활용하여 상태 관리와 로직을 효율적으로 구성하는 데 목적이 있습니다.
또한, localStorage를 활용하여 별도의 데이터베이스 없이도 할 일 데이터를 저장하고 유지할 수 있도록 구현했습니다.
🚀 주요 기능
🧩 프로젝트 구조
💡 구현 포인트
커스텀 훅(useTodos)을 통해 상태와 비즈니스 로직을 분리
useEffect를 활용하여 todos 변경 시 localStorage와 자동 동기화
컴포넌트를 역할별로 분리하여 가독성과 유지보수성 향상
불필요한 중복 저장 로직을 제거하고 상태 중심으로 데이터 흐름 구성