-
Notifications
You must be signed in to change notification settings - Fork 1
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
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.
고생 많으셨어요!
export default function StarRating({ rating }: StarRatingProps) { | ||
const filledStars = Math.floor(rating); // 소수점 이하를 버리고, Filled Star 개수 결정 | ||
const emptyStars = 5 - filledStars; // 전체 별의 개수에서 Filled Star 개수를 뺀 나머지는 Empty Star 개수 | ||
|
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.
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} />
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.
아직 제대로는 못봤는데... 요거 리뷰 남길때 쓰는 별점 정하는 컴포넌트랑 같은걸 쓰면 어떨까 싶어요
input range로 만들고 리뷰 남길때만 enabled, 별점을 변경하지 못하는 단순 별점표시라면 disabled 처리해서 값 못바꾸게!
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.
저도 고민했던 지점인데, 별 5개를 재사용할 수 있도록 컴포넌트 분리를 하자는 말씀이시죠?
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.
음 상조님 말하시는게 뭔지 알 것 같아요! 저도 1) 리뷰 등록할 때 별점 등록하는 거랑 2) 별점 보여주는 컴포넌트를 통일해보면 어떨까 싶었는데, 사용되는 별 아이콘 사이즈가 달라서 우선 2)만 구현을 했습니다. 통일할 수 있다면 좋을 것 같은데, 사이즈 다른 건 어떻게 처리할 수 있을까요?
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.
사이즈가 달라요? ㅋㅋㅋㅋㅠㅠ 킹쩔수없군요
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.
으아 알겠습니다 ㅜㅁㅜ
} | ||
|
||
export default function StarRating({ rating }: StarRatingProps) { | ||
const filledStars = Math.floor(rating); // 소수점 이하를 버리고, Filled Star 개수 결정 |
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.
넵넵 알겠습니다~!: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.
로그 컴포넌트는 아직 디자인과 논의가 좀 되어야 할 것 같군여...🤔
{/* <VisitNumFlag visitNum={visitNum} /> */} | ||
<MenuTypeFlag menuType={menuType} /> |
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.
요 Flag라는 컴포넌트들은 디자인쪽과 한번 이야기해봐야 할 것 같아요.
Chip 컴포넌트를 만든다면 개별로 그때그때 ~Flag를 만들지 않아도 될 것 같습니다🤔
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> | ||
); |
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.
filled를 그리고 남은 영역은 자동으로 empty가 되면 함수 호출을 한번으로 줄일 수 있을 것 같아요!
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.
오오 그렇게 하면 더 깔끔하겠네요 반영하도록 하겠습니다!:)
for (let i = 0; i < count; i++) { | ||
stars.push( | ||
<div key={i}> | ||
{type === 'filled' ? <FilledStarSmall /> : <EmptyStarSmall />} |
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.
for문을 사용하지 않고 Array.prototype 매서드를 사용하는 방법도 있습니다~!
Array.from({ length: count }).map((_, index) =>
(<div key={index} />)
)
이러면 div가 count 만큼 생겨납니다!
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.
e55b985 코드 리뷰 반영해보았습니디~!:)
첫 풀리퀘 이후 수정 사항이 좀 많은 것 같아 커밋 내역들을 정리했습니다!
|
PR의 목적을 알려주세요.
마이페이지 로그 컴포넌트 생성
어떤 변경사항이 있는지 알려주세요.
MenuTypeFlag
컴포넌트를 만들었습니다.StarRating
컴포넌트를 생성했습니다! 그래서 3.5 넘겨주면 별 3개 채워지는,, 그런 컴포넌트 생성했습니다. 디자인팀한테 논의해보니 별 채우는 건 한 개 씩 채우신다고 하셔서 그렇게 짰습니다!중점적으로 봐주었으면 하는 부분
StarRating
컴포넌트 로직이 저게 최선이었을까..?components/Log/index.tsx
여기서는 다른 컴포넌트 import문이 자동 완성되는데, 한 단계 더 깊게 폴더를 만들어서 파일 생성 후 작업 시 (ex.components/Log/MyLog/index.tsx
) import문이 자동 완성이 안 되더라구요..!? 이거 저만 그런 건지 궁금했습니닷