카드 투두 구현#43
Conversation
Summary of ChangesHello @jungwon123, 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은 할일 목록을 표시하고 관리할 수 있는 새로운 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. Changelog
Activity
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
카드 투두 컴포넌트 구현을 잘 보았습니다. 전반적으로 컴포넌트 구조와 기능이 명확하게 작성되었습니다. 몇 가지 개선점을 제안드립니다.
TodoCard컴포넌트에서items배열이 변경될 때만checkedCount를 다시 계산하도록useMemo를 사용하여 성능을 최적화할 수 있습니다.onItemCheckedChangeprop이 제공되지 않았을 때 체크박스가 읽기 전용으로 동작하도록 수정하여 의도치 않은 사용자 인터랙션을 방지하는 것이 좋습니다.- Storybook 파일에서
ControlledTodoCard의 props 타입을 더 정확하게 지정하고, 중복되는 상태 업데이트 로직을 함수로 추출하여 코드 유지보수성을 높일 수 있습니다.
자세한 내용은 각 파일의 주석을 참고해주세요.
| checked={item.checked} | ||
| size="small" | ||
| label={<span className={styles.itemLabel}>{item.text}</span>} | ||
| onCheckedChange={(checked) => onItemCheckedChange?.(item.id, checked)} |
There was a problem hiding this comment.
onItemCheckedChange prop이 제공되지 않은 경우, CheckBox는 클릭은 되지만 상태가 변경되지 않아 사용자에게 혼란을 줄 수 있습니다. onItemCheckedChange가 없을 때 CheckBox의 onCheckedChange prop에 undefined를 전달하여 CheckBox가 내부적으로 읽기 전용(read-only) 상태가 되도록 하는 것이 좋습니다. 이렇게 하면 사용자가 체크박스를 변경할 수 없다는 것을 명확히 알 수 있습니다.
| onCheckedChange={(checked) => onItemCheckedChange?.(item.id, checked)} | |
| onCheckedChange={onItemCheckedChange ? (checked) => onItemCheckedChange(item.id, checked) : undefined} |
| }: { | ||
| items: TodoItem[]; | ||
| title: string; | ||
| }) => { |
There was a problem hiding this comment.
ControlledTodoCard 컴포넌트의 props 타입이 { items: TodoItem[]; title: string; }으로 지정되어 있어, ...args로 전달되는 expanded, onKebabClick 등의 다른 TodoCard props에 대한 타입 체크가 이루어지지 않습니다. TodoCardProps를 사용하여 더 정확한 타입을 지정하는 것이 좋습니다. 이렇게 하면 타입 안정성이 향상되고, 컴포넌트의 의도된 사용법을 더 명확하게 할 수 있습니다. (./types/types에서 TodoCardProps를 import해야 합니다.)
}: Omit<TodoCardProps, 'onItemCheckedChange'>) => {
| onItemCheckedChange={(id, checked) => | ||
| setItems1((prev) => prev.map((item) => (item.id === id ? { ...item, checked } : item))) | ||
| } |
There was a problem hiding this comment.
Overview 스토리 내에서 setItems1과 setItems2에 전달되는 상태 업데이트 로직이 중복됩니다. 이 로직은 ControlledTodoCard 컴포넌트의 handleCheckedChange 함수에도 동일하게 존재합니다. 코드 중복을 줄이고 유지보수성을 높이기 위해 이 로직을 별도의 헬퍼 함수로 추출하여 재사용하는 것을 고려해 보세요.
예를 들어, 다음과 같은 헬퍼 함수를 정의할 수 있습니다.
const updateItemChecked = (prev: TodoItem[], id: string, checked: boolean) =>
prev.map((item) => (item.id === id ? { ...item, checked } : item));그리고 이 함수를 handleCheckedChange와 Overview 스토리에서 사용하면 코드가 더 간결해집니다.
// In ControlledTodoCard
const handleCheckedChange = (id: string, checked: boolean) => {
setItems((prev) => updateItemChecked(prev, id, checked));
};
// In Overview story
// ...
onItemCheckedChange={(id, checked) => setItems1((prev) => updateItemChecked(prev, id, checked))}
// ...| expanded = true, | ||
| className, | ||
| }: TodoCardProps) { | ||
| const checkedCount = items.filter((item) => item.checked).length; |
There was a problem hiding this comment.
items prop이 변경되지 않았음에도 컴포넌트가 리렌더링될 때마다 checkedCount가 다시 계산됩니다. items 배열이 클 경우 성능에 영향을 줄 수 있습니다. useMemo를 사용하여 items가 변경될 때만 값을 다시 계산하도록 최적화하는 것을 권장합니다.
이를 위해 react에서 useMemo를 import해야 합니다: import { useMemo } from 'react';
| const checkedCount = items.filter((item) => item.checked).length; | |
| const checkedCount = useMemo(() => items.filter((item) => item.checked).length, [items]); |
4 participants
Summary
카드 투두 구현입니다.
Issue
#42