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단계] 정윤서 미션 제출합니다. #9

Open
wants to merge 1 commit into
base: yunseeo
Choose a base branch
from

Conversation

yunseeo
Copy link

@yunseeo yunseeo commented Jan 13, 2024

  • 카드 중복클릭 문제를 해결하였습니다.
  • 재시작 버튼이 카드 위에 뜨도록 수정하였습니다.

Copy link
Member

@a-honey a-honey left a comment

Choose a reason for hiding this comment

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

클래스 간의 상호작용을 잘 적용해주셨네요👍

불필요한 주석만 삭제해주시고 예외사항만 처리해주시면 깔끔한 코드가 될 거같아요
추가로 특정 기능을 구현한 후에 바로 커밋을 남겨주시면 변경사항을 확인하기 용이하실거예요

수고하셨습니다🚀

this.isGoodCard = isGoodCard;
this.isClicked = false;
this.cardClickHandler = cardClickHandler;
this.alreadyClicked = false;
Copy link
Member

Choose a reason for hiding this comment

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

👍👍

WhenCardClick(btn) {
//if (this.alreadyClicked) return; //이미 고른 카드는 고르지 말고 종료해라.
//this.alreadyClicked = true;
//this.remainingChances--; //아 여기서 계속 2->1 로 줄어들어서 문제 발생.
Copy link
Member

Choose a reason for hiding this comment

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

처음에 문제가 생기면 주석을 작성해서 커밋을 남겨놓고, 추후 해결한 이후에 주석 삭제를 습관화해주시면 좋을거같아요!
필요없는 주석을 삭제하면 파일 용량을 줄일 수 있어요


//2.
cardClickHandler(clickedCard) {
//console.log(this);
Copy link
Member

Choose a reason for hiding this comment

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

현재 this가 무엇인지 파악하기 위한 코드도 확인 이후에 바로 지워주시면 좋습니다


btn.addEventListener('click', this.WhenCardClick.bind(this));
btn.addEventListener('click', () => {
if (!this.isClicked && !this.alreadyClicked) {
Copy link
Member

Choose a reason for hiding this comment

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

기회가 없는 상태에서 다른 버튼을 클릭해도 카드가 뒤집히고 있어요!
기회를 넘겨받아서 예외 사항을 처리해주면 좋겠네요

text-align: center;
}

#RestartBtn {
Copy link
Member

Choose a reason for hiding this comment

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

RestartBtn요소의 여부 상관없이 똑같은 레이아웃을 주면 UX 적으로 좋을 거 같아요!

undercards.innerHTML = ''; //
undercards.appendChild(RestartBtn);

RestartBtn.addEventListener('click', this.WhenBtnClick.bind(this, RestartBtn));
Copy link
Member

Choose a reason for hiding this comment

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

this를 바인딩하는것도 좋은 방법인데, 화살표함수를 사용하면 직관적으로 파악할 수 있어요!
정확히 말하면 화살표함수를 사용하면 호출방식을 체크하지 않아도 현재 스코프의 this를 참조할 수 있어서 코드를 읽기 쉬워져요

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