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

Feature/artist circle #34

Merged
merged 4 commits into from
Aug 31, 2024
Merged

Feature/artist circle #34

merged 4 commits into from
Aug 31, 2024

Conversation

East-cloud-of-forest
Copy link
Contributor

@East-cloud-of-forest East-cloud-of-forest commented Aug 13, 2024

Circle 컴포넌트 하나뿐 이지만 확장성을 고려해 Artist 컴포넌트 하위 오브젝트로 넣어놨습니다.

props
artistId
checked

checked 안에 check svg 의 사이즈가 기존 피그마의 작은 컴포넌트 사이즈가 아닌 큰 컴포넌트 사이즈에 들어감에 따라 비례해서 size 를 키웠습니다.
48px -> 63px

Copy link
Member

@kms0219kms kms0219kms left a comment

Choose a reason for hiding this comment

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

프로필이미지 svg로 넣으셨던데, Raw파일을 보니 그냥 png라서 svg로 넣을 필요가 없어보이구요.
추후 확장성을 고려해서 img태그로 빼면서 src를 props로 받는게 좋을 것 같습니다.

어쩌면 artistData 자체를, Array가 아닌 Object로 props에서 끌고와도 될거같아요.

@East-cloud-of-forest
Copy link
Contributor Author

enum 이었던 artist 정보를 props 로 바꿨습니다.

props
src
label
backgroundColor
checked

  • App 파일에서는 임시로 넣을 이미지를 에셋 폴더에 저장 하기는 좀 그래서 나무위키 이미지로 넣어두었습니다.

@kms0219kms
Copy link
Member

@East-cloud-of-forest conflict가 있는거 같은데, resolve 부탁드릴게요~

@kms0219kms kms0219kms self-requested a review August 31, 2024 05:03
Copy link
Member

@kms0219kms kms0219kms left a comment

Choose a reason for hiding this comment

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

LGTM

@kms0219kms kms0219kms merged commit 4cbcfc3 into main Aug 31, 2024
2 checks passed
@kms0219kms kms0219kms deleted the feature/ArtistCircle branch August 31, 2024 07:24
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