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

[퍼블리싱] 디테일 페이지 #18

Open
wants to merge 18 commits into
base: dev
Choose a base branch
from
Open

[퍼블리싱] 디테일 페이지 #18

wants to merge 18 commits into from

Conversation

Yeom-JinHo
Copy link
Member

디테일 페이지 퍼블리싱입니다.

Profile부분 PR 닫아주시면 반응형까지 작업해서 말씀드리겠습니다 :)

스크린샷 2022-03-10 오전 12 44 00

스크린샷 2022-03-10 오전 12 44 07

@Yeom-JinHo Yeom-JinHo requested a review from minsoo-web March 14, 2022 09:13
@minsoo-web
Copy link
Contributor

스크린샷 2022-03-19 오후 7 26 57

모바일 뷰에서 width 조정이 필요해보입니다!

@minsoo-web
Copy link
Contributor

  1. 카테고리 최대 개수가 정해져있는지 희윤님한테 한 번 여쭤봐주세요, 만약 제한이 없는 경우 줄바꿈 처리로 바꿔야 할 것 같습니다.

스크린샷 2022-03-19 오후 7 28 58

스크린샷 2022-03-19 오후 7 29 26

  1. width로 처리하는 것이 아니라 padding으로 처리해야 할 것으로 보입니다. 폰트 사이즈의 최소 크기는 10px이기 때문에 크기가 반응형으로 줄어도 여백이 유지되지 않습니다.

스크린샷 2022-03-19 오후 7 30 56

@minsoo-web
Copy link
Contributor

이건 사실 이전 PR때 확인했어야 하는 부분인데 제가 미처 확인하지 못했네요

스크린샷 2022-03-19 오후 7 39 25

스크린샷 2022-03-19 오후 7 39 48

가운데 정렬이 안 되어있는 것 같습니다.
모바일 크기로 보면 더 심해서 발견했습니다 ㅠ

Copy link
Contributor

@minsoo-web minsoo-web left a comment

Choose a reason for hiding this comment

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

리뷰 모두 남겼습니다~! 고생하셨어요! 수정 완료되면 노티 부탁드립니다.

src/components/Common/CircleList/CircleList.tsx Outdated Show resolved Hide resolved
src/components/Common/CircleList/CircleList.tsx Outdated Show resolved Hide resolved
src/components/Common/CircleList/CircleList.scss Outdated Show resolved Hide resolved
src/components/Common/CircleList/CircleList.tsx Outdated Show resolved Hide resolved
src/components/Detail/MusicAbout/UserAbout/UserAbout.tsx Outdated Show resolved Hide resolved
src/components/Detail/MusicAbout/UserAbout/UserAbout.tsx Outdated Show resolved Hide resolved
src/components/Detail/MusicAbout/UserAbout/UserAbout.tsx Outdated Show resolved Hide resolved
@Yeom-JinHo
Copy link
Member Author

희윤님께 음악 카테고리 부분 최대 개수 3개로 전달 받아서 따로 레이아웃은 처리해야할 부분 없을꺼 같고,
클릭 가능요소로 변경하겠습니다.

@Yeom-JinHo
Copy link
Member Author

999 오버시 K,M 사용하도록 3자리까지 디자인 수정하였다고 하셨습니다

@Yeom-JinHo
Copy link
Member Author

999 오버시 K,M 변환 작업은 따로 빼야할꺼 같은데 어디에 빼는게 좋을까요?

@Yeom-JinHo Yeom-JinHo requested a review from minsoo-web April 2, 2022 10:59
@minsoo-web
Copy link
Contributor

999 오버시 K,M 변환 작업은 따로 빼야할꺼 같은데 어디에 빼는게 좋을까요?

utils 폴더하나 만들구 getCounts 같은 함수 하나 만들어서 카운트 개수에 따라서 다르게 보여지게끔 처리하면 될 것 같습니다

Copy link
Contributor

@minsoo-web minsoo-web left a comment

Choose a reason for hiding this comment

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

몇 가지 리뷰 사항이 있습니다! 반영해서 작업해주세요

src/components/Common/CircleList/CircleList.tsx Outdated Show resolved Hide resolved
src/components/Common/CircleList/CircleList.tsx Outdated Show resolved Hide resolved
src/components/Common/CircleList/CircleList.tsx Outdated Show resolved Hide resolved
src/components/Detail/MusicAbout/UserAbout/UserAbout.tsx Outdated Show resolved Hide resolved
src/components/Detail/MusicAbout/UserAbout/UserAbout.tsx Outdated Show resolved Hide resolved
src/components/Common/MiniTrack/MiniTrack.tsx Outdated Show resolved Hide resolved
@Yeom-JinHo Yeom-JinHo requested a review from minsoo-web April 3, 2022 08:32
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