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

[week6][박민서] 과제 제출 #24

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

Conversation

charijyard
Copy link

@charijyard charijyard commented May 2, 2024

💎 과제 구현 설명

과제를 위해 사용했던 함수, 수정한 파일 등 구현 내용에 대한 간단한 설명을 작성해 주세요.

  • vote history page 구현
  • vote page 구현
  • API 활용하여 데이터 불러오고 수정하기

🏁 PR 체크리스트

  • 코드가 오류 없이 정상적으로 실행되나요?
  • 커밋 메시지 컨벤션(템플릿 활용)을 준수했나요?
  • 과제 마감기한을 준수했나요?

🖼️ Screenshot / Video

스크린샷 2024-05-02 23 33 55 스크린샷 2024-05-02 23 34 05 스크린샷 2024-05-02 23 34 19

🙌 Issue

과제 수행 중 어려웠던 부분이나 궁금했던 점을 자유롭게 작성해 주세요.

@charijyard charijyard requested review from Jack-Chagarr, isuh88 and yebin1926 and removed request for yebin1926 May 2, 2024 14:30
@charijyard charijyard changed the title 과제 구현 [week6][박민서] 과제 제출 May 2, 2024
Comment on lines +122 to +130
onClick={() => vote(1)}
/>
<img
src={thumbsDownImage}
className="w-20 h-20 cursor-pointer"
// ### thumbsDownImage Event ###
onMouseOver={handleThumbsDownHover}
onMouseOut={handleThumbsDownLeave}
onClick={() => vote(-1)}

Choose a reason for hiding this comment

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

잘 하셨는데, 이 부분에서 조금 더 가독성을 높일 수 있는 방법을 알려드릴게요!

  const VoteValue = {
    UP: 1,
    DOWN: -1,
  };

을 위에 선언해주고, vote(1) 대신 vote(VoteValue.UP), vote(-1) 대신 vote(VoteValue.DOWN) 로 값을 입력해줄 수 있어요!

내가 짠 코드도 일주일만 지나면 이게 뭐였지 하는 경우가 많고, 타인과 협업을 할 때는 다른 사람의 코드를 읽을 때 숫자나 문자가 그대로 하드코딩 되어있으면 이해하는데 시간이 필요한 경우가 많아서 이렇게 원하는 값을 좀 더 직관적으로 랩핑해주면 훨씬 더 좋은 코드를 작성할 수 있습니다~!

Copy link
Author

Choose a reason for hiding this comment

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

구체적이고 친절한 코멘트 감사드립니다! 참고해서 다음 번에는 더 좋은 코드 짤 수 있게 하겠습니다 ㅎㅎㅎ

@Jack-Chagarr
Copy link

코드 정상적으로 동작하는 것 확인했습니다~ 수고하셨어요!!! 나중에 시간 되실 때 심화 과제도 한 번 도전해보세요😁

@Jack-Chagarr Jack-Chagarr self-requested a review May 3, 2024 01:48
Copy link
Contributor

@isuh88 isuh88 left a comment

Choose a reason for hiding this comment

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

LGTM~~ 👍

);

const data = response.data;
setImages(data[0]);

Choose a reason for hiding this comment

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

저는 다시 data.id 랑 data.url 을 새로운 변수에 저장해서 그걸 setImages() 로 pass 했는데 그럴 필요가 없었네요!

/>
{images ? (
<img
src={images.url}
Copy link

@yebin1926 yebin1926 May 6, 2024

Choose a reason for hiding this comment

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

삼항 연산자를 src 안에서 하면 더 간단하게 할 수도 있을 것 같아요!

/>
<img
src={thumbsDownImage}
className="w-20 h-20 cursor-pointer"
// ### thumbsDownImage Event ###
onMouseOver={handleThumbsDownHover}
onMouseOut={handleThumbsDownLeave}
onClick={() => vote(-1)}

Choose a reason for hiding this comment

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

짱이에요 수고하셨습니다~!

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.

5 participants