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

과제 완료 - 한슬희 #7

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

hanseulhee
Copy link
Member

useRef가 뒤늦게 생각이나 후에 다시 커밋해보겠슴니다. ㅠ

@hyesungoh hyesungoh self-requested a review April 14, 2022 06:14
Copy link
Contributor

@hyesungoh hyesungoh left a comment

Choose a reason for hiding this comment

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

굿굿굿입니당 useRef만 사용하시면 이상적일 거 같아요~~


function App() {
const [number, setNumber] = useState(0);
const EVEN = "even";
const ODD = "odd";
Copy link
Contributor

Choose a reason for hiding this comment

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

이런 상수 (constant)는 컴포넌트 외부에서 관리하는 게 좋을 거 같아요!

const [number, setNumber] = useState(0);
const EVEN = "even";
const ODD = "odd";
const title = document.querySelector(".numText");
Copy link
Contributor

Choose a reason for hiding this comment

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

말씀해주신 것처럼 useRef를 사용해보는 방향으로 하는게 좀 더 react스러울 거 같아요~~

title?.classList.add(EVEN);
} else {
title?.classList.remove(EVEN);
title?.classList.add(ODD);
Copy link
Contributor

Choose a reason for hiding this comment

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

현재 작성해주신 내용이 유지보수 측면에서 이점도 있고, 가독성도 좋다고 생각해요! 근데 더 축약할 수 있는 방법에 대해 말씀드리자면

className 초기 상태를 설정 후,

  useEffect(() => {
    title?.classList.toggle(ODD);
    title?.classList.toggle(EVEN);
  }, [number]);

이렇게도 할 수 있을 거 같아요~~~~

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.

2 participants