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/#12] 공통 아이콘 제작 #13

Merged
merged 13 commits into from
Jan 10, 2025
Merged

Conversation

gitsuhyun
Copy link
Collaborator

@gitsuhyun gitsuhyun commented Jan 10, 2025

🏠 ISSUE

❗ WORK DESCRIPTION

  • 공통 아이콘을 제작했습니다.
    • RoomieKeyword 제작
    • RoomieTag 제작
    • RoomieRoomAsset 제작
  • ic_middle_dot 아이콘을 추가했습니다.

📸 SCREENSHOT

RoomieKeyword RoomieTag RoomieRoomAsset

💻 주요 코드

  • preview 참고하시면 될 것 같습니다!

📢 TO REVIEWERS

  • keyword 부분 보니까 text만 들어가는게 아니라 중간에 작은 점이 들어가는건 어떻게 해야할까 하다가 그냥 content()로 뺀 버전도 하나 만들었는데 이렇게하니 keyword만 3가지 버전이 있는데 괜찮나요..??
  • 네이밍 짓는건 정말 어려운것 같아염... 한번 확인해주세요!!

Copy link
Collaborator

@hyeeum hyeeum left a comment

Choose a reason for hiding this comment

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

궁금한 점들 달아놓았으니 확인해주세요!
그리고 Box>Text 대신 Text로만 컴포넌트 만드는 연습도 하면 좋을 듯! (굉장히 편함)
모르면 바로 삐삐쳐요!!

Comment on lines 36 to 49
Box(
modifier = modifier
.background(color = backgroundColor, shape = RoundedCornerShape(size = 4.dp))
.noRippleClickable {
onClick()
}
.padding(horizontal = 8.dp, vertical = 4.dp),
contentAlignment = Alignment.Center
) {
Text(
text = text,
style = textStyle
)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Box 없이 Text만으로도 구현할 수 있어요
modifier을 잘 사용해보셔요! 다른 컴포넌트들도 마찬가지!

modifier = Modifier
.size(32.dp)
.clip(CircleShape)
.fillMaxSize()
Copy link
Collaborator

Choose a reason for hiding this comment

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

clip 후에 fillMaxSize 넣어준 이유는 무엇인가요??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

image를 부모 컴포넌트에 맞게 확장하려 넣은건데 생각해보니 size를 고정해서 불필요한 코드인것 같네요 삭제하겠습니다!!

Copy link
Collaborator

@kangyein9892 kangyein9892 left a comment

Choose a reason for hiding this comment

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

혜음이가 너무 다 잘 이야기해줬네요... 늦어서 미안해요 😥 야무지게 잘 만들었는데요!?! 고생하셨어요 ㅎㅎ

Copy link
Collaborator

@hyeeum hyeeum left a comment

Choose a reason for hiding this comment

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

코드 짱 깔끔해짐 히히 너무 좋아요!!!

}
}

@Composable
fun RoomieBorderKeyword(
fun RoomieOutlinedChip(
Copy link
Collaborator

Choose a reason for hiding this comment

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

네이밍 센스 죽인다...

Comment on lines +144 to +146
RoomieTextWithDotChip(
firstText = "성별",
secondText = "n인실",
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.

프리뷰 만들면서 이게 맞나 싶었는데 디폴트 지정하니 좋네여~!!!!

) {
Text(
modifier = modifier
.width((LocalConfiguration.current.screenWidthDp * 0.106).dp)
Copy link
Collaborator

@hyeeum hyeeum Jan 10, 2025

Choose a reason for hiding this comment

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

엇 그런데 아래에서 패딩값을 선언해줘서 이 width는 필요없지 않나요??

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

피그마에 있는거로 봐선 글자수에 상관없이 같은 width를 의도하신 것 같은데 width 지정을 안하면 width가 각자 따로 놀더라구용 그래서 지정해줬습니다!!

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

@hyeeum hyeeum left a comment

Choose a reason for hiding this comment

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

하 진짜 빠르다 최고애요

@gitsuhyun gitsuhyun merged commit 61c916b into develop Jan 10, 2025
@gitsuhyun gitsuhyun deleted the feature/#12-common-icon branch January 10, 2025 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature] 공통 아이콘 제작
3 participants