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

[TnT-85] 1차 컴포넌트 구현 #14

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from
Open

Conversation

SeonJeongk
Copy link
Contributor

📝 작업 내용


  • 1차로 확정된 컴포넌트들을 구현했습니다.

    Button,TextField, Dialog, TopBar

  • ButtonButtonUtilsgetButtonConfig를 통해 Size(XLarge, Large, Medium, ...) 와 Type(Primary, Gray, GrayOutline, RedOutline), enabled에 맞는 설정을 넘겨받습니다.

  • TopBar뒤로가기만, 뒤로가기 + 제목, 뒤로가기 + 제목 + component의 3가지 옵션이 있습니다.

  • TextField는 입력란과 right component, 에러문구를 보여주는 TnTTextField와 제목과 counter까지 보여주는 TnTLabeledTextField로 구현했습니다.

  • 추가로 기존 Typo와 Color를 추가 및 수정했습니다.

📸 실행 화면

🙆🏻 리뷰 요청 사항

없습니다

👀 레퍼런스

버튼 최소 크기 수정
TextField 커스텀

@SeonJeongk SeonJeongk added 🎨 Design UI 및 디자인 작업 🌻 선정 김씨 집안 막내 김선정 labels Jan 11, 2025
@SeonJeongk SeonJeongk self-assigned this Jan 11, 2025
@hoyahozz hoyahozz self-requested a review January 12, 2025 03:25
Copy link
Member

@hoyahozz hoyahozz left a comment

Choose a reason for hiding this comment

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

디자인 시스템 구현하시느라 너무 고생하셨어요 :->

코멘트 남겨놓았으니 한 번 확인해주세요 ~!

Comment on lines +13 to +19
enum class ButtonSize {
XLarge,
Large,
Medium,
Small,
XSmall,
}
Copy link
Member

Choose a reason for hiding this comment

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

요기 있는 클래스들 모두 패키징이 필요해보입니다!

component > button > model 로 패키징하는거 어떨까요?

)

@Composable
fun getButtonConfig(size: ButtonSize, variant: ButtonType, enabled: Boolean): ButtonConfig {
Copy link
Member

Choose a reason for hiding this comment

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

ButtonConfig 를 따로 분리하신 이유가 있으실까요?

제 생각에 height, contentPadding 과 같은 필드들은 ButtonSize 에서,

color, stroke 와 같은 필드들은 ButtonType 내의 프로퍼티 혹은 메소드로 분리할 수 있을 것처럼 보여요!

Comment on lines +34 to +39
size: ButtonSize = ButtonSize.Medium,
type: ButtonType = ButtonType.Primary,
text: String,
enabled: Boolean = true,
onClick: () -> Unit,
modifier: Modifier = Modifier,
Copy link
Member

Choose a reason for hiding this comment

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

컴포즈에서는 권장되는 파라미터 순서가 있어요!

일반적으로 필수 파라미터 (text)는 가장 최상위에, 옵셔널한 파라미터는 그 하위에 작성하도록 권장하고 있습니다!

그 중에서도, modifier 는 옵셔널한 파라미터 중 최상단에 위치하기를 권장하고 있어요!

Since the modifier is recommended for any component and is used often, placing it first ensures that it can be set without a named parameter and provides a consistent place for this parameter in any component.

Comment on lines +128 to +137
Column(
modifier = Modifier.fillMaxSize(),
verticalArrangement = Arrangement.Top,
horizontalAlignment = Alignment.CenterHorizontally,
) {
Text(
text = text,
style = TnTTheme.typography.h4,
)
}
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
Column(
modifier = Modifier.fillMaxSize(),
verticalArrangement = Arrangement.Top,
horizontalAlignment = Alignment.CenterHorizontally,
) {
Text(
text = text,
style = TnTTheme.typography.h4,
)
}
Text(
text = text,
style = TnTTheme.typography.h4,
modifier = Modifier.align(Alignment.Top)
)

Column 안감싸고 Text 로만 하위 컴포저블 구현 가능할 것 같아요 !_!

isSingleLine: Boolean = false,
showWarning: Boolean = false,
warningMessage: String? = null,
rightComponent: @Composable () -> Unit = {},
Copy link
Member

Choose a reason for hiding this comment

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

우리 버튼 포지션에서 Trailing, Leading 이라는 표현을 썼으니까 요기서도 적용해봐요!

enabled = enabled,
colors = config.colors,
shape = RoundedCornerShape(config.cornerRadius),
border = BorderStroke(width = config.stroke, color = config.borderColor),
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
border = BorderStroke(width = config.stroke, color = config.borderColor),
border = BorderStroke(
width = config.stroke,
color = config.borderColor,
),

취향차이긴 한데 파라미터 두 개이상인 경우 개행해주는거 어떨까요 ?.?


@Preview(showBackground = true, heightDp = 100)
@Composable
fun TnTTextFieldPreview() {
Copy link
Member

Choose a reason for hiding this comment

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

모든 프리뷰 컴포저블 함수는 private 하게 선언해주세요!

@Composable
fun TnTTextField(
value: String,
placeholder: String,
Copy link
Member

Choose a reason for hiding this comment

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

placeHoldernullable 한 타입으로 선언해주는건 어떨까요?

필요하지 않은 경우도 있을 것 같아서요!

.padding(end = 8.dp),
) {
if (value.isEmpty()) {
Text(
Copy link
Member

Choose a reason for hiding this comment

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

오호 이렇게 구현하는 것도 괜찮기는 한데, BasicTextFielddecorationBox 활용하면, PlaceHolder 구현 가능해요!

참고 : https://reco-dy.tistory.com/16

// back button
@Preview(showBackground = true)
@Composable
fun TnTTopBarPreview1() {
Copy link
Member

Choose a reason for hiding this comment

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

1, 2, 3 숫자 대신 좀 더 의미있는 함수명을 작성해봅시당!

TnTTopBarOnlyBackButtonPreview 와 같은 이름은 어때요? ㅎㅎ

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌻 선정 김씨 집안 막내 김선정 🎨 Design UI 및 디자인 작업
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TNT-85] 디자인 시스템 1차 컴포넌트 구현
2 participants