Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[2주차] 지민재 미션 제출합니다. #5

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

mimizae
Copy link

@mimizae mimizae commented Sep 20, 2024

배포 링크 🍀

💡이번 미션을 수행하며...💡

  • React를 사용할 때 기계적으로 터미널에 명령어를 쳐서 React 프로젝트를 만들고 바로 컴포넌트를 구현하는 습관이 들어서 😅 뭐든 원리를 알고 내가 App 컴포넌트에 친 코드들이 화면에 렌더링 되는 건지 파악하기 위해 index.js와 index.html 그리고 App 컴포넌트 사이의 관계를 이해한 후 미션에 돌입했습니다.

  • 먼저 UI를 만드는 것에는 이전에 했던 디자인을 거의 차용하기에 어려움이 없었지만 UI 구현 후 기능을 구현하는 데 약간의 어려움이 있었습니다. 대충 알고 있던 동기/비동기 개념으로 useEffect를 사용하려다 보니 todo 아이템이 로컬 스토리지에 저장이 안 되고 삭제 기능에 버그가 생기는 등 문제가 생겨 이것을 해결하는 과정에서 비동기/동기 개념을 확실하게 잡고 넘어갈 수 있었습니다.

  • 또 Key Question에 대한 답변을 공부하며 React 렌더링 최적화를 제 미션에 적용하려고 시도했습니다. 그 결과 useCallback과 React.memo를 사용해 특정 todo 아이템을 조작할 때 다른 todo 아이템이 불필요하게 리렌더링 되는 것을 방지하고 Date를 나타내는 부분은 하루에 한 번, 날짜가 변할 때 리렌더링 되어야 할 텐데 화면의 모든 조작에서 리렌더링이 발생하는 것을 해결하기도 했습니다.

  • 그러나 Context API나 Recoil과 같은 전역 상태를 사용하지 못하다 보니 useState를 사용해 App 컴포넌트에서 상태를 중앙집중화 시키고 컴포넌트를 모듈화 하니 props drilling이 발생해 특정 상태가 변경 되면 항상 App 컴포넌트가 리렌더링 되어 상태 변경과 무관한 컴포넌트 또한 리렌더링 되는 불편함을 겪었습니다. 여기에서 전역 상태관리의 편리함을 깨달을 수 있었습니다.

  • 지난 미션의 기능 외 추가한 기능 중 Modal과 Confirm Modal을 구현한 것이 있는데, 여기에서 지난 Key Question에서 다룬 이벤트 버블링으로 인한 버그를 발견 후 지난 미션에서 공부한 내용을 바탕으로 해결해 매우 뿌듯한 경험이었습니다!! 😆

  • 지난 미션과는 달리 트러블 슈팅 과정을 기록하려고 마음 먹은 뒤 미션에 임하니 특정 에러가 발생했을 때 그 에러를 해결하기에 급급하기 보다는 발생했는지 원인을 분석하는 데 집중할 수 있었습니다. 위에서 언급한 내용들을 모두 트러블 슈팅(노션)에 기록했습니다. 약소하고 정리를 잘 못해 어지러울 수 있지만 둘러보시고 혹시 틀린 내용이 있다면 피드백으로 수정할 수 있고 좋은 내용이 있다면 나누어 드릴 수 있으니 공유합니당…!!!! 😖😖

🔥지난 미션의 기능 외 추가한 기능🔥

  • Modal 창과 Confirm Modal 창을 구현 후 조건부 렌더링해서 입력 값 없이 todo를 추가하려고 할 때, todo 삭제할 때, 모든 todo를 완료했을 때에 띄우던 alert를 대신했습니다.
  • 완료된 todo는 todolist의 아래 쪽으로 이동하도록 하는 sort 함수를 사용했습니다.
  • todo item의 고유 key를 timestapm로 지정해 todo item의 text가 같더라도 구분될 수 있게 했습니다.

🔑 Key Question

  • Virtual-DOM은 무엇이고, 이를 사용함으로서 얻는 이점은 무엇인가요?
  • React.memo(), useMemo(), useCallback() 함수로 진행할 수 있는 리액트 렌더링 최적화에 대해 설명해주세요. 다른 방식이 있다면 이에 대한 소개도 좋습니다.
  • React 컴포넌트 생명주기에 대해서 설명해주세요.

➡️ 제대로 알고 싶은 마음으로 적다보니 글이 꽤 길어져 노션에 따로 정리해 두었습니다!! 🔥

💭 해답을 찾는 것에 그치지 않고 제 코드에 적용시켜도 보며 이것을 제대로 이해하고 넘어가야 앞으로 프론트엔드 개발자로서 더 나은 UI를 구현하기 위한 고민을 '잘' 할 수 있다고 생각해 이번 Key Question의 해답을 찾는 데에 많은 시간을 투자했습니다.

그럼에도 부족한 부분은 늘 있기 마련일 테니 발견해서 고쳐나가고 싶습니다! 거침없는 피드백 부탁드립니다👍🏻👍🏻

Comment on lines +31 to +35
const [isAllCompleted, setIsAllCompleted] = useState(false); // 모든 todo가 완료되었는지 여부를 추적
const [modalText, setModalText] = useState(''); // 기능마다 모달 text를 다르게 하기 위함
const [isModalOpen, setIsModalOpen] = useState(false); // 모달 상태
const [isConfirmModalOpen, setIsConfirmModalOpen] = useState(false);
const [todoToDelete, setTodoToDelete] = useState(null); // 삭제할 todo

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

주석을 친절하게 잘 달아주셔서 코드 이해하기가 편했습니다!

Comment on lines +42 to +64
useEffect(() => {
const storedTodos = localStorage.getItem('todos');
if (storedTodos) {
try {
setTodos(JSON.parse(storedTodos)); // 데이터가 있다면 JSON 파싱
} catch (error) {
console.error("로컬 스토리지에서 데이터를 불러오는 중 오류 발생:", error);
setTodos([]); // 오류가 있으면 빈 배열로 초기화
}
}
const storedIsAllCompleted = localStorage.getItem('isAllCompleted');
if (storedIsAllCompleted) {
setIsAllCompleted(JSON.parse(storedIsAllCompleted));
}
}, []); // 새로고침 시 이전 데이터를 불러오기. localStorage로부터 todo 배열을 todo state에 저장하고 모든 todo 완료 상태를 저장

useEffect(() => {
// todos 배열이 비어 있지 않을 때만 로컬 스토리지 업데이트, 새로고침 시 todos 배열이 비어진 상태일 때 비동기로 인해 로컬 스토리지를 덮어씌우지 않게 하기 위함.
// 이미 todo를 다 완료해서 삭제 했다면 상관 없지! 빈 배열로 덮어씌워져도.
if (todos.length > 0) {
localStorage.setItem('todos', JSON.stringify(todos));
}
}, [todos]);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

노션에 올려두신 trouble shooting을 확인했을 때에도 이 부분에 대한 깊은 고민이 있으셨던 것 같아요. 특히 useEffect의 동기/비동기에 대해 흥미롭게 읽었답니다 ㅎㅎ(재밌더라구요 ㅋㅋ~)

저도 useEffect로 애먹은 적이 정말정말정말 많았어서..ㅠㅠ 이번에는 다른 방식으로 기능을 구현해보고 싶다는 생각이 있었습니다.

저는 localstorage에 있는 값을 가져올 때, useState를 활용하면서도 해당 파일을 외부로 빼는 방식을 활용하면서 이 부분을 구현해봤어요.

참고한 블로그 글에서 Extracting the localStorage logic여기서부터 읽어보면 좋을것같아요!
물론 useEffect를 활용하는 방법을 저도 매우 편리하게 잘 사용하고 있으나,이렇게 여러 관점으로 바라볼 수 있다면 좋을 것 같아서 공유드려봅니다.

Comment on lines +85 to +101
const addTodo = useCallback((newTodo) => {
// 입력된 할 일이 빈 문자열이거나 공백만 있을 경우 처리
if (newTodo.trim() === '') {
setModalText('오늘의 할 일을 입력해 주세요!🍀'); // 모달에 띄울 텍스트 설정
setIsModalOpen(true);
return;
}

setTodos(prevTodos => [
...prevTodos,
{
text: newTodo,
completed: false,
timestamp: Date.now() // 고유한 timestamp 추가
}
]);
},[]); // useCallback으로 함수 재생성을 방지, todo 상태가 변화해도 재생성 x

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이번 key question에 소개된 함수들을 프로젝트 여기저기에 잘 활용하신거같아요~!

Comment on lines +43 to +64
const InputForm = ({ addTodo }) => {
const inputRef = useRef(null);

const handleSubmit = (event) => {
event.preventDefault();
if (inputRef.current) {
const inputValue = inputRef.current.value.trim();
addTodo(inputValue); // 빈 문자열인 경우도 addTodo로 전달 후 addTodo에서 함수 종료
inputRef.current.value = ''; // 입력 필드 초기화
}
};

return (
<Form onSubmit={handleSubmit}>
<Input
type="text"
ref={inputRef}
placeholder="할 일을 입력하세요"
/>
<SubmitButton type="submit">추가</SubmitButton>
</Form>
);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useRef사용에 익숙치 않아서 이 부분은 보면서 도움을 많이 받았습니다.
저도 민재님이 trouble shooting에 쓴 내용 처럼 onChange에 대한 이벤트가 발생할 때마다 값을 담아두는 방식으로 구현했었습니다.

<input
        type="text"
        placeholder="할 일 추가"
        value={inputValue} // inputValue 상태값을 value 속성에 연결 : 값 가져오려면 필요함
        onChange={(e) => setInputValue(e.target.value)}
      />

이렇게 썼는데, 민재님의 초기 코드와 매우 유사하더라구요.
이 방식은 사용자가 키보드를 누를 때마다 이벤트를 감지해서 처리해줘야 한다는 점에서 굳이 이방법밖에는 없을까? 에 대한 고민이 있었는데 시의적절한 useRef의 사용으로 해결할 수 있을것같습니다. 저도 담에 기회가 생긴다면 잘 적용할수있을거같아요 ㅎㅎ

Comment on lines +28 to +32
${({ $isOpen }) =>
$isOpen &&
css`
animation: ${slideDown} 0.3s ease-out forwards;
`}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

조건부 스타일을 잘 알고계신거같아요!
전 삼항연산자로만 작성했는데 이런 방식도 있었군요. 감사합니다

Comment on lines +20 to +23
const sortedTodos = [...todos].sort((a, b) => {
if (a.completed === b.completed) return 0;
return a.completed ? 1 : -1; // 완료된 항목을 맨 아래로 보내기
});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

완료한 항목은 밑으로 내리는 로직을 이렇게 구현하셨군요
이 정렬로직 알고리즘 문제 풀 때 많이 봤던 방식인데 여기서 보니까 신기하기도 하고, 이런 식으로 적용할 수도 있구나 싶네요 ㅎㅎ 저는 왜 이렇게 정렬할 생각을 하지 못했을까요ㅜㅜ 배워갑니다


return (
<ModalBackground onClick={onCancel}>
<ModalContainer $isOpen={isOpen} onClick={(e) => e.stopPropagation()}>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

버블링 문제를 해결하신 과정을 잘 읽었습니다!
저도 이후에 비슷한 문제가 발생하면 stopPropagation을 활용해봐야겠습니다.

Comment on lines +116 to +125
const handleConfirmDelete = useCallback(() => {
setTodos(prevTodos => {
const updatedTodos = prevTodos.filter(todo => todo.timestamp !== todoToDelete);
localStorage.setItem('todos', JSON.stringify(updatedTodos));
return updatedTodos;
});
setIsConfirmModalOpen(false);
setModalText('todo를 삭제했습니다. 🥲');
setIsModalOpen(true);
}, [todoToDelete]);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

모달 창을 활용해서 항목을 정말 삭제할 것인지에 대해 물어보는 방식이 좋았습니다.

image
다만 이 부분에서 모달 닫는 버튼 같은게 추가로 있다면 좀 더 좋을 것같다고 생각됩니다.

Copy link

@Shunamo Shunamo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

민재님~!! 정성스러운 리드미 + 정성스러운 주석 + 노션에 따로 정리까지 해두시다니 ...
열정 대박이에요 🥹 사소한 디테일 신경쓰시면서 구현하신 것 같아요
리뷰하면서 저까지 뿌듯해졌어여 ㅎㅎ
이번 과제 고생 많으셨어요 😍😍

font-display: swap;
src: url('/fonts/PretendardVariable.woff2') format('woff2-variations');
}
`; // 전체 페이지의 기본 여백 제거, 코드의 중복을 줄이고 모든 컴포넌트에 일관된 스타일 적용
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

글로벌 스타일링ㅎㅎ 꼼꼼해요 😳

App.js는 프로젝트의 전반적인 구조를 다루는 파일인데, 여기에 글로벌 스타일까지 넣으면 조금 복잡해지고 추후 수정시 번거로울 수 있어요!
GlobalStyle.js 같은 파일로 따로 분리해서 작성하고 App.js에 import해서 사용하면 더 좋을 것 같아요 👍 👍

}
}, [totalTodos, completedTodos, isAllCompleted]);

const addTodo = useCallback((newTodo) => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<Main> 컴포넌트로 전달되는 addTodo, deleteTodo, toggleTodoCompletion<ConfirmModal>컴포넌트로 전달되는 handleConfirmDelete 함수에 useCallback을 적용하신 건 성능 측면에서 좋은 접근 방법인 것 같아요! 👍 👍

함수들이 자식 컴포넌트로 전달 -> useCallback을 사용하지 않으면 -> 부모 컴포넌트 리렌더링시 -> 새로 생성된 함수들이 자식 컴포넌트에 전달 -> 불필요한 리렌더링 발생

또 각 함수의 의존성 배열을 적절히 설정하셔서 상태 변화에 따라서 필요할 때만 재생성되도록 관리하신 것도 좋은 접근인 것 같아요!

import React from 'react';
import styled, { keyframes } from 'styled-components';

export const slideDown = keyframes`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

slideDown 애니메이션을 Modal.js 파일에서도 재사용하고 계신 것 같아요!
컴포넌트 재사용 하는 건 좋은 습관이라고 생각해요 😙
지금처럼 작은 프로젝트에서는 어떤 파일에 어떤 스타일이 있는지 알아서 사용하기 어렵지 않지만 프로젝트 볼륨이 커지면 헷갈릴 수도 있으니, animation.js 같은 파일에 따로 분리해서 작성하고, 필요할 때마다 꺼내서 사용하면 유지보수 시에 더욱 편리할 거예요!

Comment on lines +35 to +45
const Modal = ({ ModalText, isOpen, onClose }) => {
if (!isOpen) return null;

return (
<ModalBackground onClick={onClose}>
<ModalContainer $isOpen={isOpen} onClick={(e) => e.stopPropagation()}>
<p>{ModalText}</p>
</ModalContainer>
</ModalBackground>
);
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

알림까지 모달로 표시해주시다니... 디테일 챙기느라 고생햇서요🤩

지금 ConfirmModal 컴포넌트에도 Modal 스타일 임포트해서 사용하고 계신데,
두 모달창이 같은 컨테이너, 같은 스타일을 사용하되 취소, 삭제 버튼에만 차이가 있는 거라면
조건부 랜더링을 사용해서 하나의 컴포넌트 내에서 관리해주시면 더 의미있는 컴포넌트가 될 것 같아요!

Suggested change
const Modal = ({ ModalText, isOpen, onClose }) => {
if (!isOpen) return null;
return (
<ModalBackground onClick={onClose}>
<ModalContainer $isOpen={isOpen} onClick={(e) => e.stopPropagation()}>
<p>{ModalText}</p>
</ModalContainer>
</ModalBackground>
);
};
const Modal = ({ isOpen, onClose, content, isConfirmModal, onConfirm }) => {
if (!isOpen) return null;
return (
<ModalBackground onClick={onClose}>
<ModalContainer $isOpen={isOpen} onClick={(e) => e.stopPropagation()}>
<p>{content}</p>
{isConfirmModal && (
<ButtonGroup>
<ModalButton $cancel onClick={onClose}>취소</ModalButton>
<ModalButton onClick={onConfirm}>삭제</ModalButton>
</ButtonGroup>
)}
</ModalContainer>
</ModalBackground>
);
};

이런식으로 버튼 텍스트를 onCloseonConfirm과 같은 상태에 맞게 동적으로 바꿔서 사용한다면 모달창 관리가 더 쉬워질 것 같아요!
(물론 app.js에서도 추가적인 수정이 필요하겠지만요 ...🥹)

Copy link

@s-uxun s-uxun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 민재 님! 이번 코드리뷰를 맡은 송유선입니다. 😊

전반적으로 깔끔한 디자인과 코드 구성, 그리고 공부를 많이 하신 게 느껴지는 주석들을 보며 저도 많이 배우는 시간이었습니다! 과제하느라 고생하셨고 다음 스터디도 함께 파이팅해요🔥

Comment on lines +58 to +64
useEffect(() => {
// todos 배열이 비어 있지 않을 때만 로컬 스토리지 업데이트, 새로고침 시 todos 배열이 비어진 상태일 때 비동기로 인해 로컬 스토리지를 덮어씌우지 않게 하기 위함.
// 이미 todo를 다 완료해서 삭제 했다면 상관 없지! 빈 배열로 덮어씌워져도.
if (todos.length > 0) {
localStorage.setItem('todos', JSON.stringify(todos));
}
}, [todos]);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useEffect 안에서 todos의 길이가 0일 때 로컬 스토리지를 덮어쓰지 않도록 하신 부분을 보면 코드를 정말 섬세하게 짜려고 많이 노력하신 것 같아요! 전 로컬스토리지 관련해서 처음에 약간 애를 먹었거든요,,,😂 특히 app.js에서는 더욱 더 주석을 상세하게 달아주셔서 저도 보면서 많은 공부를 할 수 있었습니다!!

  • '이미 todo를 다 완료해서 삭제 했다면 상관 없지! 빈 배열로 덮어씌워져도.' 부분,,, 너무 귀여운 거 아닌가요,,,🤍

}
}, [totalTodos, completedTodos, isAllCompleted]);

const addTodo = useCallback((newTodo) => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addTodo나 toggleTodoCompletion 등의 함수에 useCallback을 사용하여 상태가 변경되지 않으면 함수가 재생성되지 않도록 최적화한 부분이 좋은 것 같습니다!

Comment on lines +20 to +23
const sortedTodos = [...todos].sort((a, b) => {
if (a.completed === b.completed) return 0;
return a.completed ? 1 : -1; // 완료된 항목을 맨 아래로 보내기
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sort 메서드를 활용하여 완료된 항목을 맨 아래로 내리도록 정렬하는 건 좋은 방법인 것 같습니다! 👍🏻 다만 지금은 매번 todos 배열을 복사하고 정렬하도록 코드가 작성되어 있는데, useMemo를 활용하여 todos 배열이 변경될 때만 재정렬하도록 한다면 불필요한 연산을 줄일 수 있지 않을까하고 생각합니다!

참고로 저는 이 부분을 다음과 같이 작성했는데요..!

const sortedTodos = useMemo(() => { return [...todos].sort((a, b) => a.done - b.done); }, [todos]);

같은 부분임에도 서로 다른 로직을 활용한 게 흥미로웠습니다! ㅎㅎ 뭔가 제 생각엔 간단한 boolean 값 비교만을 기준으로 생각한다면 제 코드가 좀 더 효율적일 것 같고, 이후 다른 기준이나 논리적 확장을 고려한다면 민재 님께서 작성하신 코드가 유지보수 측면에서 좀 더 유리할 것 같습니다😊

overflow-wrap: break-word;
`;

const DeleteButton = styled.button`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

제가 배포된 버셀 링크로 보니까 화면 너비가 최소로 줄어들면 추가 버튼은 그대로인데 삭제 버튼의 텍스트가 깨져서 세로로 정렬이 되는 문제가 있더라구요..! 그래서 코드를 어떻게 수정하면 좋을지 알아보려고 파일을 다운 받아 로컬에서 돌려봤는데 로컬에서는 안 깨졌습니다... 🤔 제가 자세히 확인해보니까 버튼 외에도 둘 사이에 헤더 높이나 폰트 크기 등에 약간 차이가 있더라구요... 혹시 현재 깃 코드와 배포링크의 코드가 약간 다른가요..?? 만약 같다면 이런 문제는 왜 발생한 걸까요...

Comment on lines +6 to +14
// html 문서에서 id가 root인 DOM 요소를 가져와서 React의 루트 컨테이너로 설정. 이 컨테이너 안에 React 컴포넌트들이 렌더링 됨.
// 즉, html의 id가 root인 DOM 요소가 React의 가장 큰 바깥 컨테이너가 되고, 그 컨테이너에 App 컴포넌트가 전달 되어 렌더링 된다.
root.render(
<React.StrictMode>
<App />
</React.StrictMode>
);
// <React.StrictMode>:
// React 컴포넌트가 더 안전하고 일관성 있게 동작하도록 돕는 래퍼 컴포넌트. 개발 모드에서만 활성화되며, 배포 시에는 제거 됨.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분까지 주석을 다실 줄은 몰랐는데.. 공부를 많이 하신 게 느껴지네요! 👍🏻

Copy link

@Programming-Seungwan Programming-Seungwan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이번 과제 진행하시느라 고생 많으셨습니다 :)

Comment on lines +2 to +6
import Header from './components/Header';
import Main from './components/Main';
import Modal from './components/Modal';
import ConfirmModal from './components/ConfirmModal';
import styled, { createGlobalStyle } from 'styled-components';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

현재 애플리케이션은 규모가 아주 큰 것이 아니라, 상대 경로의 import 문을 사용하셔도 괜찮지만 나중에 타입스크립트나 규모가 큰 프로젝트를 할 경우에는 컴포넌트가 굉장히 많아지는 경우가 발생해요! 이럴 때에는 절대 경로 문법을 javascript와 ts에서 사용하는 것이 저는 유지 보수적인 측면에서 더 편하더라구요

관련 레퍼런스 참고해 보시면 좋을 것 같습니다!

Comment on lines +45 to +50
try {
setTodos(JSON.parse(storedTodos)); // 데이터가 있다면 JSON 파싱
} catch (error) {
console.error("로컬 스토리지에서 데이터를 불러오는 중 오류 발생:", error);
setTodos([]); // 오류가 있으면 빈 배열로 초기화
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try ~ catch 문이 한 뎁스 바깥 쪽에 있어야하지 않을까 싶어요. 일단 if(storedTodos) 문을 통과했다는 것은 제대로 받아와 졌으니까요!

Comment on lines +52 to +56
const storedIsAllCompleted = localStorage.getItem('isAllCompleted');
if (storedIsAllCompleted) {
setIsAllCompleted(JSON.parse(storedIsAllCompleted));
}
}, []); // 새로고침 시 이전 데이터를 불러오기. localStorage로부터 todo 배열을 todo state에 저장하고 모든 todo 완료 상태를 저장

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const storedIsAllCompleted = localStorage.getItem('isAllCompleted');
if (storedIsAllCompleted) {
setIsAllCompleted(JSON.parse(storedIsAllCompleted));
}
}, []); // 새로고침 시 이전 데이터를 불러오기. localStorage로부터 todo 배열을 todo state에 저장하고 모든 todo 완료 상태를 저장
if (localStorage.getItem('isAllCompleted')) {
setIsAllCompleted(JSON.parse(localStorage.getItem('isAllCompleted')));
}
}, []); // 새로고침 시 이전 데이터를 불러오기. localStorage로부터 todo 배열을 todo state에 저장하고 모든 todo 완료 상태를 저장

그냥 깔끔하게 이렇게도 쓸 수 있겠네요!


return (
<ModalBackground onClick={onClose}>
<ModalContainer $isOpen={isOpen} onClick={(e) => e.stopPropagation()}>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Styled-components 라이브러리에서 특정 컴포넌트에 속성을 넘기는 방법을 아주 잘 알고 계신 것 같습니다. 또한 ModalText를 속성으로 받아서 상황에 맞는 모달을 띄우시는 방식도 너무 좋구요 :)

Comment on lines +103 to +125
const toggleTodoCompletion = useCallback((todoTimeStamp) => {
setTodos(prevTodos =>
prevTodos.map(todo =>
todo.timestamp === todoTimeStamp ? { ...todo, completed: !todo.completed } : todo
)
);
}, []);

const deleteTodo = useCallback((todoTimestamp) => {
setTodoToDelete(todoTimestamp);
setIsConfirmModalOpen(true);
}, []);

const handleConfirmDelete = useCallback(() => {
setTodos(prevTodos => {
const updatedTodos = prevTodos.filter(todo => todo.timestamp !== todoToDelete);
localStorage.setItem('todos', JSON.stringify(updatedTodos));
return updatedTodos;
});
setIsConfirmModalOpen(false);
setModalText('todo를 삭제했습니다. 🥲');
setIsModalOpen(true);
}, [todoToDelete]);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

전체적으로 todo 를 토글하는 함수 등을 작성해주신 것이 굉장히 좋아요! 다만 지금 함수들이 전반적으로 app.jsx 파일에 몰려있는데 이를 util 디렉터리 등으로 빼주고 하면 나중에 프로젝트 구성을 빠르게 살펴보기에 더 좋지 않을까 싶습니다 :)

Comment on lines +32 to +35
<ButtonGroup>
<ModalButton $cancel onClick={onCancel}>취소</ModalButton>
<ModalButton onClick={onConfirm}>삭제</ModalButton>
</ButtonGroup>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

현재 Modal 컴포넌트와 ConfirmModal 컴포넌트가 백그라운드와 컨테이너 요소를 사용하고, 전달 받는 prop 속성에서 공통점을 보이는 것 같아요.
현재의 만들어주신 방식도 괜찮고 아예 특정 상태로 이 두 컴포넌트를 조건부로 렌더링 해줘도 괜찮을 것 같습니다.

Comment on lines +44 to +63
const inputRef = useRef(null);

const handleSubmit = (event) => {
event.preventDefault();
if (inputRef.current) {
const inputValue = inputRef.current.value.trim();
addTodo(inputValue); // 빈 문자열인 경우도 addTodo로 전달 후 addTodo에서 함수 종료
inputRef.current.value = ''; // 입력 필드 초기화
}
};

return (
<Form onSubmit={handleSubmit}>
<Input
type="text"
ref={inputRef}
placeholder="할 일을 입력하세요"
/>
<SubmitButton type="submit">추가</SubmitButton>
</Form>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useRef() 훅을 사용하셔서 form 요소를 컨트롤 해주셨네요! 이를 react 진영에서는 제어 컴포넌트와 비제어 컴포넌트 개념으로 다루고 있어요!

전자는 useState() 훅으로 상태로 선언해두고, 이를 input 등의 요소에 valueonChange 속성에 달아주어 사용자의 입력과 같은 인터랙션이 상태를 변경시키는 방식이고, 후자는 html dom의 구성을 가져가는 방식을 말합니다.

각각의 방식은 유효성 검사, 렌더링에 있어서 장단점을 보이고 있으니 이에 관련된 공부를 해보시는 것도 좋을 것 같아요.

관련 레퍼런스를 참고해보셔도 좋을 것 같습니다

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants