-
Notifications
You must be signed in to change notification settings - Fork 10
[2주차] 신수진 미션 제출합니다. #7
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
base: main
Are you sure you want to change the base?
[2주차] 신수진 미션 제출합니다. #7
Conversation
2025314242
left a comment
There was a problem hiding this 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 |
There was a problem hiding this comment.
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; | ||
| }; |
There was a problem hiding this comment.
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를 사용하신 이유도 궁금합니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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; | ||
| } | ||
| } | ||
| `; |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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한 문제들을 알려준다고 합니다. 저도 잘은 모르지만, 해당 방식도 고려해 보시면 좋을 것 같습니다!
|
|
||
| function Header({ title, onClick }) { | ||
| return ( | ||
| <StyledHeader onClick={onClick} style={{ cursor: "pointer" }}> |
There was a problem hiding this comment.
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; | ||
| } |
There was a problem hiding this comment.
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; | ||
| } |
There was a problem hiding this comment.
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"; |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 localStorage에 데이터 저장하는 방식 고민을 많이 했는데, 이렇게 date key 별로 분류해서 저장하는 게 더 깔끔한 것 같네요!
seoyeon5117
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2주차 과제하시느라 수고하셨습니다!
| import { | ||
| CalendarWrapper, | ||
| CalendarTile, | ||
| CalendarSection | ||
| } from "../styles/CalendarStyles"; |
There was a problem hiding this comment.
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' 형식 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
날짜 캐나다 기준으로 설정되어 있는건가요? 👀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GlobalStyles에 대해서도 알아보시고 적용하시면 좋을 것 같습니당
| const handleKeyDown = (e) => { | ||
| if (e.key === "Enter") { | ||
| addTodo(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
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}> |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제목에 오타가...!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제목에 오타가...!
오잉 Calendar 맞지 않나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그 아마 헤더 타이틀 부분 말씀하시는 걸거에요..!
| const isDuplicate = todos.some((todo) => todo.text === inputValue.trim()); | ||
| if (isDuplicate) { | ||
| alert("이미 동일한 할 일이 존재합니다."); | ||
| setInputValue(""); | ||
| return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
중복이 안됐는데 중복이 됐다고 alert가 발생하는데 뭘까여...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아 한글로 엔터 쳤을 때 발생하는 문제네요
chaeyoungwon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2주차 과제 수고하셨어요 !!!
| import MainPage from "./pages/Calendar"; | ||
| import DailyPage from "./pages/DailyTodo"; | ||
| import "./pages/appFont.css"; |
There was a problem hiding this comment.
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";
There was a problem hiding this comment.
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; | ||
| `; |
There was a problem hiding this comment.
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> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
완료된 todo에 대하여 다른 스타일을 적용하거나, todo가 완료됐으면 모아두는 방식도 화면 상에서 깔끔할 거 같아요 !!
| const isDuplicate = todos.some((todo) => todo.text === inputValue.trim()); | ||
| if (isDuplicate) { | ||
| alert("이미 동일한 할 일이 존재합니다."); | ||
| setInputValue(""); | ||
| return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
중복 체크 기능 좋은 거 같아요 !! 한글 iscomposing만 추가하면 에러 없이 잘 작동할 거 같습니다 :)
| const handleHeaderClick = () => { | ||
| navigate("/"); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
지금 기능도 너무 잘 작동하는데, 캘린더 날짜 클릭해서 todoslist로 들어갔을 때
이전 페이지로 돌아갈 수 있는 화살표 버튼이 따로 있으면 사용할 때 더 편할 것 같아요!

배포링크
https://react-todo-21th-sujin.vercel.app/
달력에서 날짜를 클릭하면 해당 날짜에 할 일 추가하는 페이지로 이동되며, 할 일 추가하는 페이지에서 헤더를 클릭하면 달력이 있는 메인 페이지로 이동이 됩니다.
느낀점
일단, 그 무엇보다 지난 결과물에서 코드 리뷰 받았던 부분들을 최대한 반영하고자 하였다.
Styled-Component를 사용하면 스타일을 관리할 때 직관적이고, 서버 사이드 렌더링을 한다는 점에서 효율이 좋다는 것은 알고 있었지만 한 번도 사용해 보지 않아 스타일을 나중에 적용하는 과정에서 어려움을 겪었다. (지난 코드리뷰에서의 시맨틱 태그를 활용해보고자 열심히 태그를 구성했는데 다 수정해야했었음…)
사실 리액트가 그렇게 익숙한 편이 아니라서 이미 구현된 기능을 다시 바꿔서 작성하는데 에도 많은 시간이 소요가 되었다. 타입스크립트로 해볼려고 했지만 나의 원대한 꿈은 안녕…
+전역상태관리 라이브러리에 대한 제한만 있어서 캘린더 라이브러리 사용해서 달력 구현했는데 나중에 주희님이 달력을 아에 구현한 것 보고 놀람 이슈///
Key Question
1️⃣ Virtual-Dom의 정의 및 사용 시의 장점
실제 DOM과 같은 내용을 담고 있는 복사본으로 JS객체 형태로 메모리에 저장되어 있음. 따라서 DOM의 모든 element와 속성을 공유함. 하지만, 브라우저에 있는 문서에 직접적으로 접근 불가하다는 특징
리액트의 virtual DOM은 두 개로 구성됨
Diffing(첫번째와 두번째 가상돔 비교)을 통해 어떤 element에 차이있는지를 신속히 파악하고 리액트를 이를 통해 차이가 발생한 부분만을 실제 DOM에 적용하게 되는 것
실제 DOM의 조작을 최소화하여 렌더링의 성능을 향상시키고, 상태 변화를 추적하므로 렌더링 일관성까지 유지할 수 있게 됨.
2️⃣ React.memo(), useMemo(), useCallback() 함수로 진행할 수 있는 리액트 렌더링 최적화
3️⃣ React 컴포넌트 생명주기