Skip to content

Conversation

@lemoncurdyogurt
Copy link
Member

배포링크

https://react-todo-21th-sujin.vercel.app/
달력에서 날짜를 클릭하면 해당 날짜에 할 일 추가하는 페이지로 이동되며, 할 일 추가하는 페이지에서 헤더를 클릭하면 달력이 있는 메인 페이지로 이동이 됩니다.

느낀점

일단, 그 무엇보다 지난 결과물에서 코드 리뷰 받았던 부분들을 최대한 반영하고자 하였다.

Styled-Component를 사용하면 스타일을 관리할 때 직관적이고, 서버 사이드 렌더링을 한다는 점에서 효율이 좋다는 것은 알고 있었지만 한 번도 사용해 보지 않아 스타일을 나중에 적용하는 과정에서 어려움을 겪었다. (지난 코드리뷰에서의 시맨틱 태그를 활용해보고자 열심히 태그를 구성했는데 다 수정해야했었음…)

사실 리액트가 그렇게 익숙한 편이 아니라서 이미 구현된 기능을 다시 바꿔서 작성하는데 에도 많은 시간이 소요가 되었다. 타입스크립트로 해볼려고 했지만 나의 원대한 꿈은 안녕…

+전역상태관리 라이브러리에 대한 제한만 있어서 캘린더 라이브러리 사용해서 달력 구현했는데 나중에 주희님이 달력을 아에 구현한 것 보고 놀람 이슈///

Key Question

1️⃣ Virtual-Dom의 정의 및 사용 시의 장점

실제 DOM과 같은 내용을 담고 있는 복사본으로 JS객체 형태로 메모리에 저장되어 있음. 따라서 DOM의 모든 element와 속성을 공유함. 하지만, 브라우저에 있는 문서에 직접적으로 접근 불가하다는 특징

리액트의 virtual DOM은 두 개로 구성됨

  1. 렌더링 이전 화면 구조를 나타내는 가상돔
  2. 렌더링 이후 보이게될 화면 구조를 나타내는 가상돔

Diffing(첫번째와 두번째 가상돔 비교)을 통해 어떤 element에 차이있는지를 신속히 파악하고 리액트를 이를 통해 차이가 발생한 부분만을 실제 DOM에 적용하게 되는 것

실제 DOM의 조작을 최소화하여 렌더링의 성능을 향상시키고, 상태 변화를 추적하므로 렌더링 일관성까지 유지할 수 있게 됨.

2️⃣ React.memo(), useMemo(), useCallback() 함수로 진행할 수 있는 리액트 렌더링 최적화

  1. useCallback()
    • 특정 함수를 새로 생성하지 않고 재사용할 수 있도록 함
    • 함수 Memoization
  2. useMemo()
    • 결과를 캐싱
    • 같은 결과 값을 사용하는 함수를 여러번 호출하는 것보다 같은 값을 사용할 것이라면 결과 값을 캐싱해놓고 캐싱된 값을 사용하는 것이 효율이 높기 때문
  3. React.memo()
    • 최적화된 컴포넌트(memoization된 컴포넌트)를 반환함
    • React.memo로 감싼 컴포넌트의 props가 변경이 되었는지 확인하고, 변경이 없을 경우 이전 렌더링 결과 재사용
      • state나 context가 변경되었을 때의 리렌더링은 막지 못함

3️⃣ React 컴포넌트 생명주기

  1. 마운팅
    • 컴포넌트가 생성이 되는 단계
    • 최초로 DOM에 삽입이되며 애플리케이션에서 컴포넌트가 시작된다
  2. 업데이트
    • 컴포넌트의 props나 state변경으로 인해 리렌더링이 발생할 때 일어나는 리액트 컴포넌트 생명주기의 중요한 부분
    • 사용자 인터페이스가 애플리케이션의 가장 최신 데이터와 상태를 반영하도록 보장
  3. 언마운팅
    • 메모리 누수를 방지하는데 중요한 역할을 함
    • 타이머, 네트워크요청, 이벤트 리스너 등이 해제되도록 보장

Copy link

@2025314242 2025314242 left a comment

Choose a reason for hiding this comment

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

재미있게 읽었습니다! page를 분리해서 main <-> todo per date 이렇게 이동할 수 있게 만든 점이 좋았습니다!

//미완료된 할 일 갯수 세기
export const countPendingTodos = (todos) => {
return todos.filter(todo => ! todo.completed).length;
} No newline at end of file

Choose a reason for hiding this comment

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

Array.filter는 O(N)의 시간복잡도를, Array.length는 O(1)의 시간복잡도를 가지는 것으로 알고 있습니다. 해당 프로젝트에서 유의미한 차이를 발생시킬 것 같지는 않지만, countTodos 이렇게 해서 total length에서 completed 개수 제하는 방식으로 함수 구성하면 더 좋을 것 같습니다!

// 로컬 시간대에서 'yyyy-mm-dd' 형식으로 반환
const formattedDate = currentDate.toLocaleDateString("en-CA"); // 'yyyy-mm-dd' 형식
return formattedDate;
};

Choose a reason for hiding this comment

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

저도 date key 관련하여 어려움을 겪었습니다 ㅜ-ㅜ 그런데 혹시 setHours를 설정하신 이유가 있을까요? 제가 알기로 toLocaleDateString이 기본적으로 날짜까지만 반환하는 것으로 알고 있어서 이 부분이 궁금합니다. 추가적으로 로컬 시간대로 설정하셨는데, ko-KR이 아닌 en-CA를 사용하신 이유도 궁금합니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

저도 date key 관련하여 어려움을 겪었습니다 ㅜ-ㅜ 그런데 혹시 setHours를 설정하신 이유가 있을까요? 제가 알기로 toLocaleDateString이 기본적으로 날짜까지만 반환하는 것으로 알고 있어서 이 부분이 궁금합니다. 추가적으로 로컬 시간대로 설정하셨는데, ko-KR이 아닌 en-CA를 사용하신 이유도 궁금합니다!

ko-KR로 하면 날짜가 2025.03.24로 분리가 되는데, url만드는 과정해서 .단위로 하면 이상하게 나와서 - 단위로 분리가 되는 en-CA로 사용하게 되었습니다
image

Copy link
Member Author

Choose a reason for hiding this comment

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

저도 date key 관련하여 어려움을 겪었습니다 ㅜ-ㅜ 그런데 혹시 setHours를 설정하신 이유가 있을까요? 제가 알기로 toLocaleDateString이 기본적으로 날짜까지만 반환하는 것으로 알고 있어서 이 부분이 궁금합니다. 추가적으로 로컬 시간대로 설정하셨는데, ko-KR이 아닌 en-CA를 사용하신 이유도 궁금합니다!

setHours하게된 이유가 자꾸 일자가 하나씩 적게 나오게 되는거 같아서 그거에 대해 찾아봤는데 그렇게 하면 보정이되는거 같아서 사용하게 되었습니다. 저도 이렇게 했을 때 어떤 원리로 보정되는건지 잘 모르겠어요ㅠㅠ

background-color: #c0392b;
}
}
`;

Choose a reason for hiding this comment

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

저는 이렇게 styled-components 분리할 생각을 못했는데, 너무 좋은 것 같습니다! 한 가지 추천드리고 싶은 게 있는데, 혹시 ThemeProvider 에 대해 아시나요? 저도 공부하면서 알게 되었는데, 해당 컴포넌트 사용하면 전역 변수나 스타일 관리가 용이하여 추천드립니다!

https://reactnativeelements.com/docs/customization/themeprovider

import Main from "./main";

const root = ReactDOM.createRoot(document.getElementById("root"));
root.render(<Main />); No newline at end of file

Choose a reason for hiding this comment

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

import ReactDOM from "react-dom/client";
import Main from "./main";
import React from "react";

ReactDOM.createRoot(document.getElementById("root")).render(
<React.StrictMode>


</React.StrictMode>
);

위 코드처럼 react에서는 strict mode 사용을 권장한다고 하더라고요. implicit한 문제들을 알려준다고 합니다. 저도 잘은 모르지만, 해당 방식도 고려해 보시면 좋을 것 같습니다!

https://react.dev/reference/react/StrictMode


function Header({ title, onClick }) {
return (
<StyledHeader onClick={onClick} style={{ cursor: "pointer" }}>

Choose a reason for hiding this comment

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

혹시 cursor: "pointer" 를 이렇게 내부 스타일로 적용하신 이유가 있을까요?

BlinkMacSystemFont, system-ui, Roboto, "Helvetica Neue", "Segoe UI",
"Apple SD Gothic Neo", "Noto Sans KR", "Malgun Gothic", "Apple Color Emoji",
"Segoe UI Emoji", "Segoe UI Symbol", sans-serif;
}

Choose a reason for hiding this comment

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

이렇게 font 따로 빼서 작성하신 게 너무 좋은 것 같습니다! 다만 한 가지 궁금한 점이, 혹시 appFont.css 파일을 pages 폴더 내에 위치시킨 이유가 있으실까요?

alert("이미 동일한 할 일이 존재합니다.");
setInputValue("");
return;
}

Choose a reason for hiding this comment

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

todo 중복 여부 체크하는 것도 너무 좋은 것 같습니다!

import { useNavigate } from "react-router-dom";
import Header from "../components/Header";
import Calendar from "react-calendar";
import "react-calendar/dist/Calendar.css";

Choose a reason for hiding this comment

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

캘린더 라이브러리가 따로 있군요? 저는 몰라서 제로베이스부터 구현했는데 ㅜㅜ

const todos = JSON.parse(localStorage.getItem(formattedDate)) || [];

const completedCount = todos.filter((todo) => todo.completed).length;
const totalCount = todos.length;

Choose a reason for hiding this comment

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

혹시 CountTodo.jsx 에 있는 함수들을 사용안하고 따로 이렇게 하신 이유가 있으실까요?


useEffect(() => {
const formattedDate = formatDate(date);
const savedTodos = localStorage.getItem(`${formattedDate}`);

Choose a reason for hiding this comment

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

저도 localStorage에 데이터 저장하는 방식 고민을 많이 했는데, 이렇게 date key 별로 분류해서 저장하는 게 더 깔끔한 것 같네요!

Copy link

@seoyeon5117 seoyeon5117 left a comment

Choose a reason for hiding this comment

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

2주차 과제하시느라 수고하셨습니다!

Comment on lines +7 to +11
import {
CalendarWrapper,
CalendarTile,
CalendarSection
} from "../styles/CalendarStyles";

Choose a reason for hiding this comment

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

이 부분 하나하나 import 하지 말고 import * as S from "../styles/CalendarStyles";처럼 사용하면 좋을 것 같아요!

currentDate.setHours(0, 0, 0, 0);

// 로컬 시간대에서 'yyyy-mm-dd' 형식으로 반환
const formattedDate = currentDate.toLocaleDateString("en-CA"); // 'yyyy-mm-dd' 형식

Choose a reason for hiding this comment

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

날짜 캐나다 기준으로 설정되어 있는건가요? 👀

Choose a reason for hiding this comment

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

GlobalStyles에 대해서도 알아보시고 적용하시면 좋을 것 같습니당

Comment on lines +59 to +63
const handleKeyDown = (e) => {
if (e.key === "Enter") {
addTodo();
}
};

Choose a reason for hiding this comment

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

한글은 조합문자라 enter키를 이용해서 이벤트를 발생시키면 이벤트가 두 번 호출되는 문제가 있어서 isComposing을 사용해서 해결하시면 좋을 것 같아요! 크롬으로 확인해보니 이 부분에 오류가 발생하네요

<TodoList>
<ul>
{todos.map((todo, index) => (
<li key={index}>

Choose a reason for hiding this comment

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

key는 index값으로 설정하는 건 권장되지 않습니다! id와 같이 고유한 값으로 설정하시면 좋을 것 같습니다

justify-content: center;
align-items: center;
min-height: 18px;
font-size: 12px;

Choose a reason for hiding this comment

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

폰트는 rem으로 수정하시는걸 추천드려요~


return (
<CalendarWrapper>
<Header title="Canlendar" />

Choose a reason for hiding this comment

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

제목에 오타가...!

Copy link
Member Author

Choose a reason for hiding this comment

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

제목에 오타가...!

오잉 Calendar 맞지 않나요?

Choose a reason for hiding this comment

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

그 아마 헤더 타이틀 부분 말씀하시는 걸거에요..!

Comment on lines +23 to +27
const isDuplicate = todos.some((todo) => todo.text === inputValue.trim());
if (isDuplicate) {
alert("이미 동일한 할 일이 존재합니다.");
setInputValue("");
return;

Choose a reason for hiding this comment

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

중복이 안됐는데 중복이 됐다고 alert가 발생하는데 뭘까여...

Choose a reason for hiding this comment

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

아 한글로 엔터 쳤을 때 발생하는 문제네요

Copy link
Member

@chaeyoungwon chaeyoungwon left a comment

Choose a reason for hiding this comment

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

2주차 과제 수고하셨어요 !!!

Comment on lines +3 to +5
import MainPage from "./pages/Calendar";
import DailyPage from "./pages/DailyTodo";
import "./pages/appFont.css";
Copy link
Member

Choose a reason for hiding this comment

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

파일 구조가 더 복잡해질 경우에는 절대경로를 사용하는 방법이 있습니다 !!
import MainPage from "@/pages/Calendar";

Copy link
Member

Choose a reason for hiding this comment

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

todo에 대해 add/ delete/ check하는 함수들은
다른 파일로 분리하고 불러와서 작성해도 좋을 거 같아요 !!

padding: 20px;
display: flex;
justify-content: center;
`;
Copy link
Member

Choose a reason for hiding this comment

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

할 일이 추가될때마다 컴포넌트 높이가 늘어나는 대신, 최대 높이를 설정하고 스크롤 가능하게 하는 방법도 좋을 거 같습니다 !!

checked={todo.completed}
onChange={() => checkTodo(index)} // 체크 상태 변경
/>
<span>{todo.text}</span>
Copy link
Member

Choose a reason for hiding this comment

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

완료된 todo에 대하여 다른 스타일을 적용하거나, todo가 완료됐으면 모아두는 방식도 화면 상에서 깔끔할 거 같아요 !!

Comment on lines +23 to +27
const isDuplicate = todos.some((todo) => todo.text === inputValue.trim());
if (isDuplicate) {
alert("이미 동일한 할 일이 존재합니다.");
setInputValue("");
return;
Copy link
Member

Choose a reason for hiding this comment

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

중복 체크 기능 좋은 거 같아요 !! 한글 iscomposing만 추가하면 에러 없이 잘 작동할 거 같습니다 :)

Comment on lines +13 to +15
const handleHeaderClick = () => {
navigate("/");
};
Copy link
Member

Choose a reason for hiding this comment

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

지금 기능도 너무 잘 작동하는데, 캘린더 날짜 클릭해서 todoslist로 들어갔을 때
이전 페이지로 돌아갈 수 있는 화살표 버튼이 따로 있으면 사용할 때 더 편할 것 같아요!

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.

4 participants