-
Notifications
You must be signed in to change notification settings - Fork 10
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주차] 최지원 미션 제출합니다. #1
base: master
Are you sure you want to change the base?
Conversation
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주차 과제 너무 고생하셨습니다!!🥰🥰🥰
코드가 너무 깔끔해서 리뷰하기 너무 편했습니다!
<h1>🐶CEOS 20기 프론트엔드 최고🐶</h1> | ||
</div> | ||
<> | ||
<GlobalStyle /> |
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.
globla style을 잘 지정해주신 점 너무 좋아요👍👍
const slideDownFadeIn = keyframes` | ||
from{ | ||
opacity: 0; | ||
transform: translateY(-1.25rem); | ||
} | ||
to{ | ||
opacity: 1; | ||
transform: translateY(0); | ||
} | ||
`; | ||
|
||
const slideUpFadeOut = keyframes` | ||
from{ | ||
opacity: 1; | ||
transform: translateY(0); | ||
} | ||
to{ | ||
opacity: 0; | ||
transform: translateY(-1.25rem); | ||
} | ||
`; |
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.
애니메이션 잘 쓰시는 것 같아서 너무 부러워요🥲🥲
type="text" | ||
value={inputValue} | ||
onChange={handleChange} | ||
autoFocus |
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.
오 autoFocus
라는 속성은 몰랐는데 저도 배워갑니다!!🙇🙇
const toggleForm = () => { | ||
// 닫힘 시 애니메이션 시간만큼의 지연 필요 | ||
const timer = () => | ||
setTimeout(() => { | ||
setIsFormOpen(!isFormOpen); | ||
}, 300); | ||
|
||
if (isFormOpen) { | ||
setAnimationClassname('fade-out'); | ||
timer(); | ||
} else { | ||
setAnimationClassname('fade-in'); | ||
setIsFormOpen(!isFormOpen); | ||
} | ||
return () => clearTimeout(timer); | ||
}; |
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.
timer를 이용한 애니메이션 관리 너무 좋아요👍👍
const totalCount = todos.length; // 전체 항목 | ||
const doneCount = todos.reduce((count, todo) => { | ||
return todo.checked ? count + 1 : count; | ||
}, 0); // 완료 항목 |
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.
reduce
를 이용해서 함수형 프로그래밍의 원칙을 따르는 점이 너무 좋아요 ;)
const timeId = setInterval(() => setNow(new Date()), 1000); | ||
|
||
return () => clearInterval(timeId); |
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.
초 단위로 시간을 바꿔주시는 점이 좋네요!👍👍
setTodos((prevTodos) => | ||
prevTodos.map((todo) => | ||
todo.id === id | ||
? { ...todo, checked: !todo.checked, id: Date.now().toString() } |
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가 toggle된 경우에 id
를 새로 생성하셨네요! 처음 코드를 읽을 때는 고유해야 할 것 같은 id
가 변경되어서 조금 어색하게 느껴졌어요. 나중에 todo를 정렬하기 위해서 id
를 변경한다는 걸 보고 이해했습니다!
이런 부분은 주석으로 남겨주시면 좋을 것 같아요🥰 아니면 id
와 별개로 lastModifiedAt
같은 속성을 추가해서 관리하는 건 어떠신지 궁금해요!
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.
lastModifiedAt
으로 추가하는 방식 좋을 것 같네요!! id는 고유 정수로 통용되는 개념이다보니 별개로 둔 속성을 정렬에 적용하는게 더 낫겠다는 생각이 듭니다! 감사해요👍
|
||
return ( | ||
<Wrapper className={animationClassname}> | ||
<form onSubmit={handleSubmit} style={{ width: '100%' }}> |
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-component를 빠트리신 게 아닌지 한번 확인 부탁드립니다!
저도 이렇게 짧은 부분은 가끔 인라인으로 바로 쓰긴 하는데 나중에 찾아서 유지보수하기가 힘들더라고요😭😭😭 가능사히면 styled-component를 적용하는 방법을 고려해보시는건 어떨까요?
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.
컴포넌트화 하지 않은 기본 form 태그라 인라인으로 적용했습니다! submit 작동을 위한 것 외에는 스타일적인 역할이 전혀 없는데 빈 공간을 차지하고 있어서 width만 간단히 주었고, 해당 태그에 대한 styled-component가 없이도 동작하더라고요.
이런 예외적인 상황 외에는 가급적 인라인 방식은 놓칠 수 있으니 지양해야겠어요!
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.
안녕하세요.
TodoTemplate에서 props를 통해서 값들을 관리하는 것이 인상적이였어요!! 컴포넌트도 간결하게 나누어 놓아서 리뷰하기 편했고 새로운 아이디어 많이 얻었어요! 또 고민을 많이 한 흔적이 보였어요. 👏 취향차이일 것 같기는 하지만 TodoTemplate 컴포넌트가 많은 상태와 함수를 포함하고 있어 useTodos.js 파일을 따로 만들어서 이를 관리하면 더 좋을 것 같습니다.
멋진 과제 고생하셨습니다~!
const toggleItem = useCallback((id) => { | ||
setTodos((prevTodos) => | ||
prevTodos.map((todo) => | ||
todo.id === id | ||
? { ...todo, checked: !todo.checked, id: Date.now().toString() } | ||
: todo, | ||
), | ||
); | ||
}, []); |
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 정렬하는 기능은 못찾았는데, 없는게 맞는거라면 checked 상태를 토글할 때마다 id를 새로운 값으로 변경하는 것은 없애는 것이 예상치 못한 버그가 발생을 방지할 것 같습니다!
const toggleItem = useCallback((id) => { | |
setTodos((prevTodos) => | |
prevTodos.map((todo) => | |
todo.id === id | |
? { ...todo, checked: !todo.checked, id: Date.now().toString() } | |
: todo, | |
), | |
); | |
}, []); | |
const toggleItem = useCallback((id) => { | |
setTodos((prevTodos) => | |
prevTodos.map((todo) => | |
todo.id === id ? { ...todo, checked: !todo.checked } : todo, | |
), | |
); | |
}, []); |
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.
처음엔 체크를 토글할 때 id를 업데이트하지 않았었는데, 그렇게 되면 추가했던 순서대로 todo id가 정렬되어서 done에 갔다가 다시 돌아올 때 중간에 항목이 끼어들어가는 게 조금 어색하게 느껴지더라구요..! 토글할 때마다 아래쪽으로 밀리도록 의도한 것은 맞으나 말씀해주신 대로 버그를 고려하면 lastUpdated 라는 속성을 따로 두어 업데이트 시키고 id는 고유하게 두면 좋겠다는 생각이 듭니다! 감사합니다😊
import { CircularProgressbar } from "react-circular-progressbar"; | ||
import "react-circular-progressbar/dist/styles.css"; | ||
|
||
const DonutGraph = React.memo(({ percent }) => { |
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.
React.memo는 컴포넌트가 동일한 props로 여러 번 렌더링되는 것을 방지하기 위해 사용하는데, 이 컴포넌트는 부모 컴포넌트에서 props가 변경될 때만 다시 렌더링되어서 React.memo를 안사용하는 것이 성능측면에서 더 좋을 것 같아요!
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.
새로운 props가 들어오지 않더라도 부모컴포넌트가 리렌더링 된다면 자식컴포넌트도 리렌더링 된다고 알고 있습니다..! 인풋 창을 여닫을 때마다 TodoTemplate가 리렌더링 되고, 그 하위 컴포넌트인 DonutGraph도 렌더링이 되더라구요 !!
https://velog.io/@ehdxka3/%EB%A6%AC%EB%A0%8C%EB%8D%94%EB%A7%81%EC%9D%B4-%EC%9D%BC%EC%96%B4%EB%82%98%EB%8A%94-%EC%A1%B0%EA%B1%B4React
if (isNaN(percent)) { | ||
percent = 0; | ||
} |
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.
props를 자식컴포넌트에서 직접 수정하는 것보다 이렇게 변수를 만들어서 하는 것도 좋은 방법일 것 같아요
if (isNaN(percent)) { | |
percent = 0; | |
} | |
const displayPercent = isNaN(percent) ? 0 : percent; | |
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.
별도의 의미를 가진 변수와 함께 한 줄로 훨씬 깔끔하게 쓸 수 있군요!
"react": "^18.3.1", | ||
"react-circular-progressbar": "^2.1.0", |
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.
프로젝트 시작전에 이렇게 스타일을 통일 시켜놓은 점 멋있어요... 👍👍👍
구현 화면 및 기능
✨배포 링크
필수 요건
선택 요건
💭 느낀 점
Vanilla JS로는 요소 하나를 추가하는데도 실제 DOM에 접근해야 하기 때문에 많은 조작이 필요했던 반면, React는 state의 변경을 감지해서 자동적으로 DOM을 업데이트하므로 상태 관리의 효율성을 깨닫게 되었습니다. useState 외에도 useEffect와 같은 리액트 Hook을 통해 의도하는 시점에 값을 자유자재로 다루는 방식의 편리함을 실감할 수 있었습니다.
한편으로는 쪼갠 컴포넌트들 각각이 그만큼 여러 state와 얽혀있게 되면서 불필요한 렌더링이 많이 발생하기도 함을 발견하였습니다. 실제로 기능 구현을 마친 후에 별도로 최적화에 대해 많이 고민해보며 React.memo(), useCallback()을 다양하게 적용할 수 있었습니다. 이전까지는 최적화를 크게 고려하지 않고 짰었던 코드를 반성하게 되기도 하였고, 앞으로는 렌더링에 유의하며 더 나은 코드를 짜도록 해야겠다는 생각이 들었습니다.
+그래프를 css로만 구현하려고 알아보았을 때 본 비교할 수도 없는 길이의 코드가 떠오르며.. 다시 한번 라이브러리의 힘을 느꼈습니다. 저번에 아쉬운 점으로 남겼는데 이번에 추가 사항으로 쉽게 구현할 수 있었습니다😊
🤔 아쉬운 점
제 코드 상에서도 count 값을 얻는 부분에 useMemo를 적용하려다가 괜히 코드가 길어지는 것 같아 제외하였지만, 앞으로 더 알아보면서 최적화하려는 계산의 비용이 크지 않아 지양해야 할 그 경계선을 확립해나가야겠다는 생각이 듭니다.🥲
🔑 Key Questions
(1) Virtual-DOM은 무엇이고, 이를 사용함으로서 얻는 이점은 무엇인가요?
가상 DOM은 실제 DOM과 분리된 복사본으로, 실제로 스크린에 렌더링하지 않으며 UI 변화에 따른 DOM 조작의 비용을 최소화합니다.
컴포넌트의 상태 변경이 감지되면 업데이트 된 내용을 새 Virtual DOM 트리로 만들고, 이를 직전 버전의 DOM 트리와 비교(Diffing)합니다. 트리 간 차이 나는 부분을 렌더러로 보내면 리컨사일러에 의해 실제 DOM에 반영(Reconciliation)하는 작업이 수행됩니다.
만약 가상 DOM이 없다면 변경이 일어나지 않은 부분까지를 포함한 전체가 리렌더링되기 때문에 브라우저에 과부하가 올 수 있어 Virtual DOM은 리액트의 성능 최적화를 돕는데 중요한 역할을 합니다.
(2) React.memo(), useMemo(), useCallback() 함수로 진행할 수 있는 리액트 렌더링 최적화에 대해 설명해주세요. 다른 방식이 있다면 이에 대한 소개도 좋습니다.
그 외
상태를 useState가 아닌 useRef로 정의하여 onChange되는 value에는 ref변수.current에 e.target.value를 할당해주어 상위 컴포넌트의 리렌더링을 막을 수 있습니다.
=> 하지만 이 방식의 경우 입력창을 초기화하기 위한 접근은 불가능하므로 추가적인 useState정의가 필요합니다. 즉, 입력값인 useState와 실제로 onChange, submit에 전달되는 useRef를 따로 정의해야 합니다.
혹은 useRef를 활용하면서 입력 후 약간의 대기 시간을 두고 마지막 입력값이 동일할 경우에만 상태를 업데이트하도록 하는 방법이 있습니다.
(3) React 컴포넌트 생명주기에 대해서 설명해주세요.
📜 참고자료
key question으로 정리한 것 외에도 많은 최적화 방식이 있다는 것을 알게 된 레퍼런스입니다.
React.memo와 useMemo의 차이에만 집중하다가 문득 useCallback과 useMemo의 차이가 궁금해져서 참고했던 자료입니다.
컴포넌트화함에 따라 상위 요소의 css 영향을 받는 만큼 조금이라도 더 일관되게 작성하기 위해 + 추후 협업 시에도 통일성을 고려하면 CSS 속성 선언 순서 의 필요성을 느껴 알아본 자료입니다.