-
Notifications
You must be signed in to change notification settings - Fork 0
[TnT-85] 1차 컴포넌트 구현 #14
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
Conversation
- TextButton, IconButton, BottomButton - ButtonUtils에서 버튼 설정 관리
hoyahozz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
디자인 시스템 구현하시느라 너무 고생하셨어요 :->
코멘트 남겨놓았으니 한 번 확인해주세요 ~!
| enum class ButtonSize { | ||
| XLarge, | ||
| Large, | ||
| Medium, | ||
| Small, | ||
| XSmall, | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요기 있는 클래스들 모두 패키징이 필요해보입니다!
component > button > model 로 패키징하는거 어떨까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵 수정해두겠습니다!
| ) | ||
|
|
||
| @Composable | ||
| fun getButtonConfig(size: ButtonSize, variant: ButtonType, enabled: Boolean): ButtonConfig { |
There was a problem hiding this comment.
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 내의 프로퍼티 혹은 메소드로 분리할 수 있을 것처럼 보여요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
모든 정보를 한 곳에서 처리하려고 하다보니 ButtonConfig로 분리했습니다
각 클래스에 관련된 속성들을 분리하여 관리하는게 더 좋아보이네요! 수정해보겠습니다👍
| size: ButtonSize = ButtonSize.Medium, | ||
| type: ButtonType = ButtonType.Primary, | ||
| text: String, | ||
| enabled: Boolean = true, | ||
| onClick: () -> Unit, | ||
| modifier: Modifier = Modifier, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
컴포즈에서는 권장되는 파라미터 순서가 있어요!
일반적으로 필수 파라미터 (text)는 가장 최상위에, 옵셔널한 파라미터는 그 하위에 작성하도록 권장하고 있습니다!
그 중에서도, modifier 는 옵셔널한 파라미터 중 최상단에 위치하기를 권장하고 있어요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안 그래도 구현하면서 순서에 대해 고민했습니다.. ;-;
첨부 링크 참고해서 수정해보겠습니다!
| Column( | ||
| modifier = Modifier.fillMaxSize(), | ||
| verticalArrangement = Arrangement.Top, | ||
| horizontalAlignment = Alignment.CenterHorizontally, | ||
| ) { | ||
| Text( | ||
| text = text, | ||
| style = TnTTheme.typography.h4, | ||
| ) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| 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 로만 하위 컴포저블 구현 가능할 것 같아요 !_!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오매 수정했습니다 감사합니다!
| isSingleLine: Boolean = false, | ||
| showWarning: Boolean = false, | ||
| warningMessage: String? = null, | ||
| rightComponent: @Composable () -> Unit = {}, |
There was a problem hiding this comment.
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), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| border = BorderStroke(width = config.stroke, color = config.borderColor), | |
| border = BorderStroke( | |
| width = config.stroke, | |
| color = config.borderColor, | |
| ), |
취향차이긴 한데 파라미터 두 개이상인 경우 개행해주는거 어떨까요 ?.?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
옹 코드가 길어지는 것 때문에 개행하지 않고 구현했습니다
개행하는거 좋아요~! 수정해두겠습니다
|
|
||
| @Preview(showBackground = true, heightDp = 100) | ||
| @Composable | ||
| fun TnTTextFieldPreview() { |
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
placeHolder 도 nullable 한 타입으로 선언해주는건 어떨까요?
필요하지 않은 경우도 있을 것 같아서요!
| .padding(end = 8.dp), | ||
| ) { | ||
| if (value.isEmpty()) { | ||
| Text( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오호 이렇게 구현하는 것도 괜찮기는 한데, BasicTextField 내 decorationBox 활용하면, PlaceHolder 구현 가능해요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
옹 넵 참고해서 수정했습니다!!
| // back button | ||
| @Preview(showBackground = true) | ||
| @Composable | ||
| fun TnTTopBarPreview1() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1, 2, 3 숫자 대신 좀 더 의미있는 함수명을 작성해봅시당!
TnTTopBarOnlyBackButtonPreview 와 같은 이름은 어때요? ㅎㅎ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TnTTopBarOnlyBackButtonPreview, TnTTopBarBackButtonWithTitlePreview, TnTTopBarWithAllComponentsPreview로 수정했습니다!
- 파라미터 2개 이상인 경우 개행 추가 - BuildConfig의 역할을 각 클래스 내 메소드로 분리 - [size] height, contentPadding, cornerRadius, textStyle - [type] stoke, borderColor, button colors
- TopBar Preview 함수명 변경
- textField 갭 8.dp로 수정
- 입력란 padding 8.dp로 수정 - 아래 라인 색상 Neutral200으로 변경
|
리뷰 남겨주신 부분 모두 반영했습니다! 확인 한 번 부탁드립니다👍 @hoyahozz |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
디스코드로 얘기 나눴던 내용 적어둡니다 ~
| contentPadding = size.contentPadding, | ||
| modifier = modifier | ||
| .height(size.height) | ||
| .defaultMinSize(minWidth = 1.dp, minHeight = 1.dp), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| .defaultMinSize(minWidth = 1.dp, minHeight = 1.dp), | |
| .defaultMinSize(minWidth = Dp.Hairline), |
| maxLength: Int = 15, | ||
| isSingleLine: Boolean = false, | ||
| showWarning: Boolean = false, | ||
| optional: Boolean = false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| optional: Boolean = false, | |
| isRequired: Boolean = false, |
| title: String = "", | ||
| onBackClick: () -> Unit = {}, | ||
| modifier: Modifier = Modifier, | ||
| trailingComponent: @Composable () -> Unit = {}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| trailingComponent: @Composable () -> Unit = {}, | |
| trailingComponent: @Composable RowScope.() -> Unit = {}, |
| import androidx.compose.ui.unit.dp | ||
| import co.kr.tnt.designsystem.theme.TnTTheme | ||
|
|
||
| enum class ButtonType( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recomposition 카운트 지속적으로 모니터링하기
| Box( | ||
| contentAlignment = Alignment.Center, | ||
| modifier = modifier | ||
| .fillMaxWidth() | ||
| .height(60.dp) | ||
| .padding(start = 16.dp, top = 20.dp, end = 16.dp, bottom = 12.dp), | ||
| ) { | ||
| IconButton( | ||
| onClick = onBackClick, | ||
| modifier = Modifier | ||
| .size(32.dp) | ||
| .align(Alignment.CenterStart), | ||
| ) { | ||
| Icon( | ||
| painter = painterResource(R.drawable.ic_arrow_left), | ||
| contentDescription = "Go back", | ||
| ) | ||
| } | ||
|
|
||
| Text( | ||
| text = title, | ||
| style = TnTTheme.typography.h4, | ||
| color = TnTTheme.colors.neutralColors.Neutral900, | ||
| textAlign = TextAlign.Center, | ||
| maxLines = 1, | ||
| overflow = TextOverflow.Ellipsis, | ||
| ) | ||
|
|
||
| Row( | ||
| modifier = Modifier.align(Alignment.CenterEnd), | ||
| ) { | ||
| trailingComponent() | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Box( | |
| contentAlignment = Alignment.Center, | |
| modifier = modifier | |
| .fillMaxWidth() | |
| .height(60.dp) | |
| .padding(start = 16.dp, top = 20.dp, end = 16.dp, bottom = 12.dp), | |
| ) { | |
| IconButton( | |
| onClick = onBackClick, | |
| modifier = Modifier | |
| .size(32.dp) | |
| .align(Alignment.CenterStart), | |
| ) { | |
| Icon( | |
| painter = painterResource(R.drawable.ic_arrow_left), | |
| contentDescription = "Go back", | |
| ) | |
| } | |
| Text( | |
| text = title, | |
| style = TnTTheme.typography.h4, | |
| color = TnTTheme.colors.neutralColors.Neutral900, | |
| textAlign = TextAlign.Center, | |
| maxLines = 1, | |
| overflow = TextOverflow.Ellipsis, | |
| ) | |
| Row( | |
| modifier = Modifier.align(Alignment.CenterEnd), | |
| ) { | |
| trailingComponent() | |
| } | |
| } | |
| CenterAlignedTopAppBar( | |
| modifier = modifier, | |
| title = { | |
| Text( | |
| text = title, | |
| style = TnTTheme.typography.h4, | |
| color = TnTTheme.colors.neutralColors.Neutral900, | |
| textAlign = TextAlign.Center, | |
| maxLines = 1, | |
| overflow = TextOverflow.Ellipsis, | |
| ) | |
| }, | |
| actions = trailingComponent, | |
| navigationIcon = { | |
| IconButton( | |
| onClick = onBackClick, | |
| modifier = Modifier.size(32.dp) | |
| ) { | |
| Icon( | |
| painter = painterResource(R.drawable.ic_arrow_left), | |
| contentDescription = "Go back", | |
| ) | |
| } | |
| } | |
| ) |
- trailingComponent에 BoxScope 적용 - optional isRequired로 변경 - if-else문 true 기준으로 작성 - OutlinedTextField 디자인 수정사항 반영


📝 작업 내용
1차로 확정된 컴포넌트들을 구현했습니다.
Button은
ButtonUtils의 getButtonConfig를 통해Size(XLarge, Large, Medium, ...) 와Type(Primary, Gray, GrayOutline, RedOutline), enabled에 맞는 설정을 넘겨받습니다.TopBar는
뒤로가기만,뒤로가기 + 제목,뒤로가기 + 제목 + component의 3가지 옵션이 있습니다.TextField는 입력란과 right component, 에러문구를 보여주는
TnTTextField와 제목과 counter까지 보여주는TnTLabeledTextField로 구현했습니다.추가로 기존 Typo와 Color를 추가 및 수정했습니다.
📸 실행 화면
🙆🏻 리뷰 요청 사항
없습니다
👀 레퍼런스
버튼 최소 크기 수정
TextField 커스텀