리스트 페이지 개발#69
Conversation
Summary of ChangesHello @HWAN0218, 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은 작업을 관리하기 위한 포괄적인 인터페이스를 제공하는 새로운 리스트 페이지를 도입합니다. 날짜 기반 작업 보기를 위한 동적 달력 컴포넌트, 작업 및 할 일 목록 작업을 위한 강력한 API 통합, 다양한 화면 크기에 적응하는 반응형 디자인을 특징으로 합니다. 또한 이 페이지에는 상세 작업 보기 오버레이와 작업 및 할 일 목록 관리를 위한 모달이 포함되어 있습니다. 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
리스트 페이지와 관련 컴포넌트 추가를 위한 대규모 업데이트입니다. 전반적으로 기능 구현은 잘 이루어졌으나, 몇 가지 중요한 개선점을 제안합니다. 가장 시급한 문제는 page.tsx에서 React 안티패턴인 document.querySelector를 사용하여 DOM에 직접 접근하는 부분으로, 이는 반드시 수정되어야 합니다. 또한, 900줄이 넘는 ListPage 컴포넌트는 역할에 따라 여러 개의 작은 컴포넌트와 커스텀 훅으로 분리하여 가독성과 유지보수성을 높일 필요가 있습니다. CSS 파일에서는 중복되는 미디어 쿼리와 과도한 !important 사용이 보여 유지보수를 어렵게 할 수 있으므로 리팩터링을 권장합니다. 마지막으로, react-query 훅의 책임 분리, clsx 사용 일관성 등 코드 품질을 높일 수 있는 몇 가지 제안을 포함했습니다.
| const handleSubmitTodoModal = async () => { | ||
| const input = document.querySelector<HTMLInputElement>('input[name="todo"]'); | ||
| const name = (input?.value ?? '').trim(); |
There was a problem hiding this comment.
handleSubmitTodoModal 함수 내에서 document.querySelector를 사용하여 DOM에 직접 접근해 입력 값을 가져오는 것은 React의 선언적 프로그래밍 모델에 반하는 안티패턴입니다. 이는 예기치 않은 버그를 유발할 수 있으며, 컴포넌트 간의 결합도를 높입니다.
AddTodoList 모달 컴포넌트가 내부적으로 input 상태를 관리하고, onSubmit 콜백을 통해 입력된 값을 부모 컴포넌트로 전달하도록 수정해야 합니다.
예시:
// AddTodoList.tsx
export default function AddTodoList({ onSubmit, ... }) {
const [inputValue, setInputValue] = useState('');
const handleSubmit = (e: FormEvent<HTMLFormElement>) => {
e.preventDefault();
onSubmit(inputValue);
};
return (
// ...
<Input value={inputValue} onChange={(e) => setInputValue(e.target.value)} ... />
// ...
);
}
// page.tsx
const handleSubmitTodoModal = async (name: string) => {
if (!name.trim()) return;
// ...
}| export function useCreateTaskComment() { | ||
| const qc = useQueryClient(); | ||
|
|
||
| return useMutation({ | ||
| mutationFn: (vars: { taskId: number; content: string }) => | ||
| fetchJson<Comment>( | ||
| `tasks/${vars.taskId}/comments`, | ||
| { method: 'POST', body: JSON.stringify({ content: vars.content }) }, | ||
| '댓글 작성에 실패했습니다.', | ||
| ), | ||
| onSuccess: async (_, vars) => { | ||
| await qc.invalidateQueries({ queryKey: ['taskComments', vars.taskId] }); | ||
| // commentCount가 바뀌니까 리스트도 갱신 필요 (상위에서 invalidate 추가로 해도 됨) | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
useCreateTaskComment 훅의 onSuccess 콜백에서 주석으로만 상위 컴포넌트의 query invalidation 필요성을 언급하고 있습니다. 이는 훅의 재사용성을 떨어뜨리고, 사용하는 쪽에서 invalidation 로직을 누락할 가능성을 만듭니다.
훅이 스스로 관련 query를 모두 무효화하도록 리팩터링하는 것이 좋습니다. mutationFn의 vars에 groupId, taskListId, dateIso를 추가로 받아 onSuccess에서 taskListByDate query를 직접 무효화하도록 개선할 수 있습니다. 이렇게 하면 훅의 캡슐화가 개선되고 코드가 더 안정적이게 됩니다.
page.tsx에서는 아래와 같이 호출부를 단순화할 수 있습니다.
// page.tsx
await createComment.mutateAsync({
taskId: selectedTask.id,
content: c,
groupId: activeGroupId,
taskListId: selectedTaskListId,
dateIso: selectedDateIso,
});
// await invalidateCurrentList(); // 이 줄이 필요 없어집니다.| export function useCreateTaskComment() { | |
| const qc = useQueryClient(); | |
| return useMutation({ | |
| mutationFn: (vars: { taskId: number; content: string }) => | |
| fetchJson<Comment>( | |
| `tasks/${vars.taskId}/comments`, | |
| { method: 'POST', body: JSON.stringify({ content: vars.content }) }, | |
| '댓글 작성에 실패했습니다.', | |
| ), | |
| onSuccess: async (_, vars) => { | |
| await qc.invalidateQueries({ queryKey: ['taskComments', vars.taskId] }); | |
| // commentCount가 바뀌니까 리스트도 갱신 필요 (상위에서 invalidate 추가로 해도 됨) | |
| }, | |
| }); | |
| } | |
| export function useCreateTaskComment() { | |
| const qc = useQueryClient(); | |
| return useMutation({ | |
| mutationFn: (vars: { taskId: number; content: string; groupId: number; taskListId: number; dateIso: string }) => | |
| fetchJson<Comment>( | |
| `tasks/${vars.taskId}/comments`, | |
| { method: 'POST', body: JSON.stringify({ content: vars.content }) }, | |
| '댓글 작성에 실패했습니다.', | |
| ), | |
| onSuccess: async (_, vars) => { | |
| await qc.invalidateQueries({ queryKey: ['taskComments', vars.taskId] }); | |
| // commentCount가 바뀌므로 taskListByDate 쿼리도 무효화합니다. | |
| await qc.invalidateQueries({ queryKey: ['taskListByDate', vars.groupId, vars.taskListId, vars.dateIso] }); | |
| }, | |
| }); | |
| } |
| items: TodoItem[]; | ||
| }; | ||
|
|
||
| export default function ListPage() { |
There was a problem hiding this comment.
ListPage 컴포넌트가 900줄이 넘어가며 상태 관리, 데이터 페칭, 렌더링 로직, 모달 및 이벤트 핸들링 등 너무 많은 역할을 담당하고 있습니다. 이는 컴포넌트의 복잡도를 높여 가독성, 유지보수성, 테스트 용이성을 저해합니다.
컴포넌트를 더 작고 역할이 명확한 단위로 분리하는 것이 좋습니다. 예를 들어, 상태 관리 로직은 커스텀 훅(예: useListPageState)으로 추출하고, UI는 TodoListPanel, TaskListPanel, TaskDetailOverlay 등과 같은 하위 컴포넌트로 분리하는 것을 고려해 보세요.
| :global([class*='Sidebar-module__'][class*='sidebar']) { | ||
| overflow: visible !important; | ||
| } | ||
| :global([class*='Sidebar-module__'][class*='content']) { | ||
| overflow: hidden !important; | ||
| } | ||
| :global([class*='Sidebar-module__'] button), | ||
| :global([class*='Sidebar-module__'] a), | ||
| :global([class*='Sidebar-module__'] span), | ||
| :global([class*='Sidebar-module__'] p) { | ||
| max-width: 100% !important; | ||
| overflow: hidden !important; | ||
| text-overflow: ellipsis !important; | ||
| white-space: nowrap !important; | ||
| } |
There was a problem hiding this comment.
:global 선택자와 함께 !important를 과도하게 사용하는 것은 다른 컴포넌트의 스타일을 강제로 덮어쓰는 방식으로, CSS 명시도 충돌을 나타내는 신호일 수 있습니다. 이는 장기적으로 스타일 디버깅과 유지보수를 매우 어렵게 만듭니다.
Sidebar 컴포넌트에 스타일을 제어할 수 있는 prop을 추가하거나, 이 페이지에서만 필요한 스타일을 더 높은 명시도를 가진 선택자로 적용하여 !important 사용을 최소화하는 방향으로 리팩터링하는 것을 권장합니다.
| @media (max-width: 1024px) { | ||
| /* 한 줄 컨테이너 */ | ||
| .mobileTodoRow { | ||
| display: flex !important; | ||
| align-items: center !important; | ||
| gap: 12px !important; | ||
| width: 100% !important; | ||
| min-width: 0 !important; | ||
| } | ||
| } | ||
|
|
||
| /* ✅ 모바일/태블릿: (1번처럼) 투두카드 + "할일 추가" 같은 줄 */ | ||
| @media (max-width: 1024px) { | ||
| /* row는 그냥 100% */ | ||
| .mobileTodoRow { | ||
| width: 100% !important; | ||
| min-width: 0 !important; | ||
| } | ||
|
|
||
| /* ✅ 여기서 핵심: 버튼이 같은 줄에 오도록 flex 컨테이너로 만든다 */ | ||
| .mobileTodoInlineCard { | ||
| display: flex !important; | ||
| align-items: center !important; | ||
| gap: 12px !important; | ||
|
|
||
| width: 100% !important; | ||
| flex: 1 1 auto !important; | ||
|
|
||
| /* 기존 clamp 고정폭 제거 */ | ||
| min-width: 0 !important; | ||
| max-width: none !important; | ||
| } | ||
|
|
||
| /* 투두카드 영역은 남는 공간 전부 먹게 */ | ||
| .todoCardWrap { | ||
| flex: 1 1 auto !important; | ||
| min-width: 0 !important; | ||
| } | ||
|
|
||
| .todoCardShell, | ||
| .todoCardShellInner { | ||
| width: 100% !important; | ||
| } | ||
|
|
||
| /* 버튼은 고정폭 유지하면서 오른쪽 */ | ||
| .mobileAddBtnWrap { | ||
| margin-left: 0 !important; | ||
| flex: 0 0 112px !important; | ||
| width: 112px !important; | ||
| } | ||
| } | ||
| @media (max-width: 1024px) { | ||
| /* ✅ 여기 폭 고정(clamp) 전부 무력화: row 전체 폭을 먹게 */ | ||
| .mobileTodoInlineCard { | ||
| width: 100% !important; | ||
| max-width: none !important; | ||
| flex: 1 1 auto !important; | ||
| min-width: 0 !important; | ||
|
|
||
| display: flex !important; | ||
| align-items: center !important; | ||
| gap: 12px !important; | ||
| } | ||
|
|
||
| /* ✅ 버튼을 진짜 맨 오른쪽으로 */ | ||
| .mobileAddBtnWrap { | ||
| margin-left: auto !important; | ||
| flex: 0 0 112px !important; | ||
| width: 112px !important; | ||
| } | ||
| } |
There was a problem hiding this comment.
파일 후반부에 @media (max-width: 1024px) 규칙이 여러 번 중복 선언되어 있습니다. 일부 스타일은 서로를 덮어쓰고 있어 혼란을 야기하고 유지보수를 어렵게 만듭니다. 이 미디어 쿼리 내의 모든 스타일을 하나의 블록으로 통합하여 정리하는 것이 좋습니다.
| function isOpenDetailBlockedTarget(target: HTMLElement | null) { | ||
| if (!target) return false; | ||
|
|
||
| if (target.closest('button, a, input, textarea, select, [role="button"]')) return true; | ||
| if (target.closest('[role="checkbox"]')) return true; | ||
| if (target.closest('[aria-checked]')) return true; | ||
|
|
||
| const labeled = target.closest('[aria-label]') as HTMLElement | null; | ||
| if (labeled) { | ||
| const v = (labeled.getAttribute('aria-label') ?? '').toLowerCase(); | ||
| if ( | ||
| v.includes('체크') || | ||
| v.includes('완료') || | ||
| v.includes('더보기') || | ||
| v.includes('kebab') || | ||
| v.includes('케밥') | ||
| ) { | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| const cls = (target.className ?? '').toString().toLowerCase(); | ||
| if (cls.includes('checkbox') || cls.includes('kebab') || cls.includes('more')) return true; | ||
|
|
||
| if (target.tagName.toLowerCase() === 'svg' || target.tagName.toLowerCase() === 'path') { | ||
| const p = target.parentElement; | ||
| if (p && isOpenDetailBlockedTarget(p)) return true; | ||
| } | ||
|
|
||
| return false; | ||
| } |
There was a problem hiding this comment.
isOpenDetailBlockedTarget 함수가 className이나 aria-label의 문자열 포함 여부로 상세 보기 열람을 막을지 판단하고 있습니다. 이 방식은 클래스명이나 레이블 텍스트가 변경될 경우 쉽게 깨질 수 있어 불안정합니다.
더 안정적인 방법으로, 상세 보기 열람을 막아야 하는 엘리먼트에 data-block-detail-open과 같은 data-* 속성을 부여하고, 이벤트 핸들러에서 target.closest('[data-block-detail-open]')을 확인하는 방식을 사용하는 것을 권장합니다.
| const profile40 = useMemo(() => { | ||
| const url = me?.image || ''; | ||
| return ( | ||
| <div | ||
| style={{ | ||
| width: 40, | ||
| height: 40, | ||
| borderRadius: 12, | ||
| backgroundColor: '#cbd5e1', | ||
| backgroundImage: url ? `url(${url})` : undefined, | ||
| backgroundSize: 'cover', | ||
| backgroundPosition: 'center', | ||
| }} | ||
| /> | ||
| ); | ||
| }, [me?.image]); |
There was a problem hiding this comment.
profile40와 같이 useMemo를 사용해 프로필 이미지를 div의 backgroundImage로 렌더링하고 있습니다. 이 방식은 alt 텍스트를 제공할 수 없어 웹 접근성을 저해하며, 인라인 스타일을 사용하여 유지보수성도 떨어집니다.
프로젝트 내에 이미 ProfileImage 컴포넌트가 있는 것으로 보입니다. 이를 재사용하거나, 시맨틱한 <img> 태그와 alt 속성을 사용하는 별도의 ProfileAvatar와 같은 재사용 가능한 컴포넌트를 만들어 사용하는 것이 좋습니다.
| return ( | ||
| <button | ||
| type="button" | ||
| className={[styles.button, selected ? styles.selected : '', className ?? ''].join(' ')} |
There was a problem hiding this comment.
className을 조합하기 위해 배열과 join을 사용하고 있습니다. 프로젝트의 다른 부분(예: Sidebar.tsx, TaskListItem.tsx)에서는 clsx 유틸리티를 사용하고 있으므로, 일관성을 위해 여기서도 clsx를 사용하는 것이 좋습니다.
| className={[styles.button, selected ? styles.selected : '', className ?? ''].join(' ')} | |
| className={clsx(styles.button, selected && styles.selected, className)} |
|
|
||
| return ( | ||
| <div | ||
| className={`${styles.container} ${className ?? ''}`} |
There was a problem hiding this comment.
className을 조합하기 위해 템플릿 리터럴을 사용하고 있습니다. 프로젝트의 다른 부분에서는 clsx 유틸리티를 사용하고 있으므로, 코드 일관성을 위해 clsx를 사용하는 것이 좋습니다.
| className={`${styles.container} ${className ?? ''}`} | |
| className={clsx(styles.container, className)} |
2 participants
Summary
Issue
Scope
포함
특이사항