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

Feat/#25 마이페이지 로그 컴포넌트 생성 #32

Merged
merged 14 commits into from
Jan 3, 2024
Merged

Conversation

YelynnOh
Copy link
Collaborator

PR의 목적을 알려주세요.

마이페이지 로그 컴포넌트 생성

어떤 변경사항이 있는지 알려주세요.

  • 마이페이지에 있는 로그 컴포넌트를 생성했습니다. props 이름은 먼저 풀리퀘 날린 준상님 네이밍이랑 통일했습니다!
    image
  • 컴포넌트 생성 중에 Menu Type 태그 컴포넌트가 common 컴포넌트로 필요할 것 같아서, 따로 MenuTypeFlag 컴포넌트를 만들었습니다.
  • 참고로 방문 횟수 나타나는 태그도 있는데, 이건 준상님이 만들어주셔서 그거 적용하면 될 것 같아요:)
  • 별점을 나타내는 StarRating 컴포넌트를 생성했습니다! 그래서 3.5 넘겨주면 별 3개 채워지는,, 그런 컴포넌트 생성했습니다. 디자인팀한테 논의해보니 별 채우는 건 한 개 씩 채우신다고 하셔서 그렇게 짰습니다!

중점적으로 봐주었으면 하는 부분

  • StarRating 컴포넌트 로직이 저게 최선이었을까..?
  • 이건 작업 하다가 궁금해진 부분인데, components/Log/index.tsx 여기서는 다른 컴포넌트 import문이 자동 완성되는데, 한 단계 더 깊게 폴더를 만들어서 파일 생성 후 작업 시 (ex. components/Log/MyLog/index.tsx) import문이 자동 완성이 안 되더라구요..!? 이거 저만 그런 건지 궁금했습니닷

@YelynnOh YelynnOh self-assigned this Dec 25, 2023
@YelynnOh YelynnOh added the feat New feature or request label Dec 25, 2023
Copy link
Collaborator

@YOOJS1205 YOOJS1205 left a comment

Choose a reason for hiding this comment

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

고생 많으셨어요!

export default function StarRating({ rating }: StarRatingProps) {
const filledStars = Math.floor(rating); // 소수점 이하를 버리고, Filled Star 개수 결정
const emptyStars = 5 - filledStars; // 전체 별의 개수에서 Filled Star 개수를 뺀 나머지는 Empty Star 개수

Copy link
Collaborator

@YOOJS1205 YOOJS1205 Dec 26, 2023

Choose a reason for hiding this comment

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

https://stackoverflow.com/questions/66764119/getting-nextjs-image-component-svgr-webpack-to-play-nicely-together
next/image 와 @svgr/webpack을 혼합해서 사용할 수 있을 것 같아요. 이렇게 된다면 별점 수에 따라서 next/image의 src 속성 분기 처리를 통해 조금 더 깔끔해질 수 있지 않을까라는 생각입니다.

별점이 3점인 경우에 index와의 비교를 통해서 작성할 수 있지 않을까요? 더 좋은 방법이 있을 것 같으니, 다른 팀원들 얘기도 들어보면 좋을 것 같네요!

<Image src={index < 별점 ? FilledStarSamll : EmptyStarSmall} />

Copy link
Member

@sjoleee sjoleee Dec 26, 2023

Choose a reason for hiding this comment

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

아직 제대로는 못봤는데... 요거 리뷰 남길때 쓰는 별점 정하는 컴포넌트랑 같은걸 쓰면 어떨까 싶어요
input range로 만들고 리뷰 남길때만 enabled, 별점을 변경하지 못하는 단순 별점표시라면 disabled 처리해서 값 못바꾸게!

Copy link
Collaborator

Choose a reason for hiding this comment

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

저도 고민했던 지점인데, 별 5개를 재사용할 수 있도록 컴포넌트 분리를 하자는 말씀이시죠?

Copy link
Collaborator Author

@YelynnOh YelynnOh Dec 26, 2023

Choose a reason for hiding this comment

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

음 상조님 말하시는게 뭔지 알 것 같아요! 저도 1) 리뷰 등록할 때 별점 등록하는 거랑 2) 별점 보여주는 컴포넌트를 통일해보면 어떨까 싶었는데, 사용되는 별 아이콘 사이즈가 달라서 우선 2)만 구현을 했습니다. 통일할 수 있다면 좋을 것 같은데, 사이즈 다른 건 어떻게 처리할 수 있을까요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

네 확인해보니 사이즈가 다르네요. 분리하는 것이 좋을지 조금 더 고민해보면 좋을 것 같네요

Copy link
Member

Choose a reason for hiding this comment

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

사이즈가 달라요? ㅋㅋㅋㅋㅠㅠ 킹쩔수없군요

Copy link
Collaborator

Choose a reason for hiding this comment

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

ㅠㅠ 분리해서 개발하겠습니다

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

으아 알겠습니다 ㅜㅁㅜ

}

export default function StarRating({ rating }: StarRatingProps) {
const filledStars = Math.floor(rating); // 소수점 이하를 버리고, Filled Star 개수 결정
Copy link
Collaborator

Choose a reason for hiding this comment

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

내림 혹은 반올림 계산은 서버에서 값이 어떻게 내려오는지에 따라서 달라질 것 같군요.
논의해보면 좋을 것 같네요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

넵넵 알겠습니다~!:0 논의해보도록 할게요!

Copy link
Member

@sjoleee sjoleee left a comment

Choose a reason for hiding this comment

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

로그 컴포넌트는 아직 디자인과 논의가 좀 되어야 할 것 같군여...🤔

Comment on lines +46 to +47
{/* <VisitNumFlag visitNum={visitNum} /> */}
<MenuTypeFlag menuType={menuType} />
Copy link
Member

Choose a reason for hiding this comment

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

요 Flag라는 컴포넌트들은 디자인쪽과 한번 이야기해봐야 할 것 같아요.
Chip 컴포넌트를 만든다면 개별로 그때그때 ~Flag를 만들지 않아도 될 것 같습니다🤔

Comment on lines 12 to 29
const getStarNum = (count: number, type: 'filled' | 'empty') => {
const stars = [];
for (let i = 0; i < count; i++) {
stars.push(
<div key={i}>
{type === 'filled' ? <FilledStarSmall /> : <EmptyStarSmall />}
</div>,
);
}
return stars;
};

return (
<div className="flex flex-row items-center">
{getStarNum(filledStars, 'filled')}
{getStarNum(emptyStars, 'empty')}
</div>
);
Copy link
Member

Choose a reason for hiding this comment

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

filled를 그리고 남은 영역은 자동으로 empty가 되면 함수 호출을 한번으로 줄일 수 있을 것 같아요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

오오 그렇게 하면 더 깔끔하겠네요 반영하도록 하겠습니다!:)

Comment on lines 14 to 17
for (let i = 0; i < count; i++) {
stars.push(
<div key={i}>
{type === 'filled' ? <FilledStarSmall /> : <EmptyStarSmall />}
Copy link
Member

Choose a reason for hiding this comment

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

for문을 사용하지 않고 Array.prototype 매서드를 사용하는 방법도 있습니다~!

Array.from({ length: count }).map((_, index) => 
  (<div key={index} />)
)

이러면 div가 count 만큼 생겨납니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

오옹 넵넵 감사합니다~! 고쳐보도록 하겠습니닷👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

e55b985 코드 리뷰 반영해보았습니디~!:)

@YelynnOh YelynnOh linked an issue Dec 27, 2023 that may be closed by this pull request
@YelynnOh
Copy link
Collaborator Author

첫 풀리퀘 이후 수정 사항이 좀 많은 것 같아 커밋 내역들을 정리했습니다!

4e35d83

  • MyLog 컴포넌트 폰트를 caption-12-regular에서 body-14-regular 으로 수정
  • 기타 높이 스타일 h-full로 수정

a9a99a3
e55b985

  • StarRating 컴포넌트 관련 코드 리뷰 반영해 로직 변경

f180ac7

  • StarRating 컴포넌트에서 사용된 별 svg 파일명 snake-case로 통일
  • 원래 EmptyStarSmall였던 svg 파일을 DefaultStarSmall 로 명칭 통일
  • 주석 TODO로 변경

f724dd3

  • StarRating 컴포넌트에 별점 숫자도 같이 보이게 컴포넌트 수정
  • 별점 정수여도 소수점으로 보이게 처리 (ex. 4 > 4.0으로 보이게)

@YelynnOh
Copy link
Collaborator Author

YelynnOh commented Dec 30, 2023

67231b7

  • StarRating 컴포넌트 StarScore로 수정

b0247f5

  • props restaurant에서 store로 변경

머지 전까지 TO DO

  • StarScore 별점 반 개 하는 거 디자인 받으면 수정하기
  • Tag 컴포넌트 머지 되면 반영하기
  • restaurant props 이름 store로 변경

@YelynnOh YelynnOh merged commit 6bb449a into main Jan 3, 2024
2 checks passed
@YelynnOh YelynnOh deleted the feat/#25 branch January 3, 2024 03:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

로그 컴포넌트 작성
3 participants