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

Chip 구현 #27

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Chip 구현 #27

wants to merge 2 commits into from

Conversation

minjun011026
Copy link

Summary

Chip 구현입니다.
Figma의 요구사항을 기반으로 구현하였습니다.
추가적으로 leadingIcon의 기본값은 InfoCircle, trailingIcon의 기본값은 Close로 설정하였고
각 Icon과 Icon의 onClick은 nullable처리를 하여 선택적으로 사용하도록 구현하였습니다.

Describe your changes

image
image

@2wndrhs
Copy link
Member

2wndrhs commented Dec 31, 2024

git rebase -i 쓰면 커밋 하나로 합칠 수 있어요
image

Copy link
Member

@kangyuri1114 kangyuri1114 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다! 리뷰 확인해주세요!

app/src/main/kotlin/com/yourssu/handy/demo/ChipPreview.kt Outdated Show resolved Hide resolved
compose/src/main/kotlin/com/yourssu/handy/compose/Chip.kt Outdated Show resolved Hide resolved
compose/src/main/kotlin/com/yourssu/handy/compose/Chip.kt Outdated Show resolved Hide resolved
Comment on lines +93 to +100
Text(
text = if (text.length < 10) text else text.take(9) + "...",
color = textColor,
fontSize = 14.dp,
style = textStyle
)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Text(
text = if (text.length < 10) text else text.take(9) + "...",
color = textColor,
fontSize = 14.dp,
style = textStyle
)
Text(
text = text,
color = textColor,
style = textStyle,
maxLines = 1, // 최대 1줄
overflow = TextOverflow.Ellipsis // 길이 초과 시 말줄임표로
)

maxLines , overflow 속성 활용 시 text = if (text.length < 10) text else text.take(9) + "...", 처럼 직접 구현하실 필요없습니다!

이미 textStyle로 typo를 적용하셨어서 fontsize를 굳이 한번 더 적용하실 필요 없어요
HandyTextStyle 들어가보시면 이미 적용되어있습니다!

Copy link
Author

Choose a reason for hiding this comment

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

overflow는 Modifier나 부모 layout 같은 배치 공간의 크기를 넘어설 때 말 줄임표 처리하는 것으로 알고 있는데
Figma의 Spec에는

Chip 하나당 최대 글자 수 10자 이내(공백 포함)로 제한됩니다. 최대 글자 수를 넘을 시 말줄임표 처리됩니다.

로 작성되어 있습니다.
Typography와 TextOverflow를 찾아봤는데 이 부분을 TextOverflow를 이용해서 처리해주는 부분을 발견하지 못 했습니다!
혹시 글자수에 따른 말줄임표 기능이 TextOverflow에 있는지 알려주시면 감사하겠습니다!

compose/src/main/kotlin/com/yourssu/handy/compose/Chip.kt Outdated Show resolved Hide resolved
compose/src/main/kotlin/com/yourssu/handy/compose/Chip.kt Outdated Show resolved Hide resolved
compose/src/main/kotlin/com/yourssu/handy/compose/Chip.kt Outdated Show resolved Hide resolved
@cometj03
Copy link
Member

git rebase -i 쓰면 커밋 하나로 합칠 수 있어요
image

오 이건 저도 몰랐네요 꿀팁 감사합니다

feat: Chip 구현

feat: Chip 구현

feat: Chip 구현

feat: Chip 구현
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
Author

Choose a reason for hiding this comment

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

앗 SearchBar를 만들어야 하는 줄 알고 만들었던 건데 같이 올려버렸네요

@cometj03
Copy link
Member

커밋 메시지를 좀 더 상세히 적어주시면 좋을 것 같아요!

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.

4 participants