🚀 4단계 - 회원가입(리팩터링) #77
Conversation
malibinYun
left a comment
malibinYun
left a comment
There was a problem hiding this comment.
4단계 미션 고생 많으셨어요 :)
더 시도하고 싶으신 게 있으시다면 반영 후 리뷰 요청을,
마무리하고 싶으시다면 DM이나 코멘트 남겨주세요!
| @Composable | ||
| private fun SignUpTextField( | ||
| label: String, | ||
| onTextChanged: (String) -> Unit, | ||
| text: String, | ||
| inputValidation: SignupValidationResult = Success, | ||
| visualTransformation: VisualTransformation = VisualTransformation.None, | ||
| private fun UsernameTextField( | ||
| username: String, | ||
| onUsernameChanged: (String) -> Unit, | ||
| usernameValidation: SignupValidationResult, |
There was a problem hiding this comment.
이런 분리된 컴포넌트도 파일 분리해보는 건 어떨까요?
SignupScreen 파일에서는 Screen에 대한 관심만 가지게 하고, 각 컴포넌트들은 각 컴포넌트에 대한 관심만 가질 수 있도록이요!
private 을 풀면 테스트도 각 컴포넌트 마다 관심을 집중해서 볼 수 있겠어요.
There was a problem hiding this comment.
각 컴포넌트 별로 Preivew를 따로 볼 수 있게도 구성할 수 있겠어요 :)
| @Test | ||
| fun `형식에 맞지 않는 이메일은 사용할 수 없다`() { | ||
| fun `형식에_맞지_않는_이메일은_사용할_수_없다`() { |
There was a problem hiding this comment.
백틱과 공백으로 구성했을 때 에러가 발생하셨나요 ?_?
There was a problem hiding this comment.
처음 피알날릴땐 괜찮았는데, 그 다음부턴 에러가 발생하더라구요! 작동은 됐지만 보기에 좋지않아서 공백 전부 제거했습니다!
| fun SignupButton( | ||
| buttonText: String, | ||
| buttonTextFontSize: TextUnit, | ||
| buttonTextFontFamily: FontFamily, | ||
| buttonTextColor: Color, | ||
| onButtonClick: () -> Unit, | ||
| containerColor: Color = Color.Unspecified, | ||
| contentColor: Color = Color.Unspecified, | ||
| disabledContainerColor: Color = Color.Unspecified, | ||
| disabledContentColor: Color = Color.Unspecified, | ||
| buttonVerticalPadding: Dp = 0.dp, | ||
| buttonHorizontalPadding: Dp = 0.dp, | ||
| enabled: Boolean = true, | ||
| modifier: Modifier, | ||
| componentDescription: String = stringResource(R.string.signup_default_description), |
There was a problem hiding this comment.
이미 SignupButton 이라는 범위를 정해두었으니, 외부에서 색상 등을 전부 커스텀할 수 있도록 열지 않아도 좋다 생각해요.
| text: String, | ||
| onTextChanged: (String) -> Unit, | ||
| textFontSize: TextUnit = 16.sp, | ||
| textFontFamily: FontFamily = RobotoRegular, | ||
| label: String, | ||
| labelFontSize: TextUnit = TextUnit.Unspecified, | ||
| labelFontFamily: FontFamily = RobotoRegular, | ||
| focusedTextColor: Color = Color.Black, | ||
| unfocusedTextColor: Color = Color.Black, | ||
| focusedLabelColor: Color = Blue50, | ||
| unfocusedLabelColor: Color = Gray, | ||
| unfocusedContainerColor: Color = BlueGray20, | ||
| focusedContainerColor: Color = BlueGray20, | ||
| focusedIndicatorColor: Color = Blue50, | ||
| isError: Boolean = false, | ||
| visualTransformation: VisualTransformation = VisualTransformation.None, | ||
| supportingText: @Composable (() -> Unit)? = null, | ||
| textFieldBackgroundColor: Color = Color.Transparent, | ||
| textFieldRoundedCornerShape: Shape = RoundedCornerShape( | ||
| topStart = 4.dp, | ||
| topEnd = 4.dp, | ||
| ), | ||
| componentDescription: String = stringResource(R.string.signup_default_description), |
There was a problem hiding this comment.
다른 화면에 언제든지 재활용할 수 있을 정도의 조각을 잘 정제해보세요 :)
기본 컴포넌트를 나만의 컴포넌트로 만들때, 제약했던 부분들이 올바르게 동작하는지 테스트하면 좋겠어요.
디자인 시스템의 컴포넌트가 준비되었다 해서 화면을 이루는 모든 컴포넌트가 준비되어있다고 하기에는 어려워요. 저는 현재에 재활용이 되지 않더라도 의미상 나눌 수 있는 것들은 최대한 나눠두려 해요. 가독성을 위한 것도 있고, 미래에 재활용이 될 것을 생각해서도 있어요.
"사실 그 누구도 미래는 알 수 없으니" 라는 이유로 저는 제가 좀 더 신경써서 나눠 놓아요. 덧붙여서, 남겨주신 PR에서는 프로젝트의 성격이 잘 드러난다고 생각해요. 다른 비유를 하자면, |
|
컴포넌트 분리에 대해 고민해보고 막상 적용해보니 어떤느낌인지 알겠습니다! 파일 분리도 괜찮은 방법인것 같아요 :) |
malibinYun
left a comment
malibinYun
left a comment
There was a problem hiding this comment.
4단계 미션까지 고생 많으셨어요!
피드백을 정말 잘 반영해주셨네요 👍👍
이번 미션은 이만 종료히도록 할게요. 질문은 언제든 환영이니 DM 메시지 주세요!
앞으로 남은 미션들 모두 화이팅이에요 💪
| @Test | ||
| fun 회원가입_뷰_헤더가_출력된다() { | ||
| // when: | ||
| composeTestRule.apply { | ||
| setContent { | ||
| HeaderText() | ||
| } | ||
| } | ||
|
|
||
| // then: | ||
| composeTestRule.onNodeWithText("Welcome to Compose \uD83D\uDE80") | ||
| .assertExists() | ||
| } |
There was a problem hiding this comment.
테스트를 꼼꼼하게 작성해주셨네요! 👍
컴포즈가 워낙 preivew 기능이 강력하다보니, 단순한 출력등 종류의 테스트는 preivew로도 대체가 가능해보여요.
preivew와 test. 취사선택을 잘 선택해보세요 :)
| var passwordConfirm by mutableStateOf("") | ||
| val passwordConfirmTextFieldTag = "Password Confirm" | ||
|
|
||
| // when: | ||
| composeTestRule.apply { | ||
| setContent { | ||
| PasswordConfirmTextField( | ||
| passwordConfirm = passwordConfirm, | ||
| onPasswordConfirmChanged = { passwordConfirm = it }, | ||
| passwordConfirmValidation = SignupValidationResult.Success, | ||
| ) | ||
| } | ||
| onNodeWithContentDescription(passwordConfirmTextFieldTag).performTextInput("1234qwer") | ||
| } | ||
|
|
||
| // then: | ||
| composeTestRule.onNodeWithContentDescription(passwordConfirmTextFieldTag) | ||
| .assertTextContains("••••••••") |
There was a problem hiding this comment.
직접 passwordConfirm를 변화시키지 않고, PasswordConfirmTextField에 들어가는 텍스트를 "1234qwer"로 설정하더라도 충분한 테스트란 생각이 들어요 :)
There was a problem hiding this comment.
별도의 컴포넌트 테스트니까 스크린 테스트처럼 굳이 어떤 시나리오를 수행하지 않아도 될 것 같네요!
| @Preview | ||
| @Composable | ||
| private fun PasswordTextFieldPreview() { |
There was a problem hiding this comment.
각 실패한 경우의 Preview도 모두 만들어보는 건 어떨까요?
이 파일만 보고 이 컴포넌트의 모든 경우의 생김새를 모두 볼 수 있다면, 이 컴포넌트에 대한 가장 좋은 설명서가 된다 생각해요 :)
There was a problem hiding this comment.
오.. 저도 모르게 프리뷰 함수는 무조건 하나여야한다는 생각이 고정관념이였던 것 같아요
프리뷰를 그렇게로도 활용할 수 있겠군요! 좋은 아이디어 감사합니다 :)
워낙 컴포즈 Preview가 강력한 탓에, Preview를 꼼꼼히 작성하면 테스트를 작성할 필요성이 줄어들기도 하지요. |
2 participants
📎𝘞𝘰𝘳𝘬 𝘋𝘦𝘴𝘤𝘳𝘪𝘱𝘵𝘪𝘰𝘯
💬𝘛𝘰 𝘙𝘦𝘷𝘪𝘦𝘸𝘦𝘳𝘴
리뷰에 대한 답글을 달다보니 나름 생각이 정리되었습니다!
다만 정해진 디자인 가이드 혹은 컴포넌트가 없다보니, 해당 컴포넌트를 얼마나 범용화?시켜야할지 감이 안잡히는데 어쩔 수 없는 부분일까요?�
기존에 제공되는 컴포저블 함수에 약간의 디자인 혹은 커스텀된 컴포넌트들이 테스트로 어떤 의미를 갖는지 궁금합니다!
적어도 현재 구현된 컴포넌트들은 특정 로직없이 기본 TextField, Button과 디자인을 제외하고 다를게 없어 무엇을 테스트해야할 지 모르겠습니다 🥲
리뷰하느라 시간내주셔서 감사합니다!
...
지금까지 여러 스크린에서 재사용 가능한 컴포넌트는 디자인가이드 혹은 UI가 준비되었을 때 기초 세팅과 함께 이뤄지는, 미리 만들어두는 줄 알았는데 꼭 그런건 아니였군요 ! 그렇다면 머릿속에서 조금 충돌하는 부분이 있습니다!
지금은 미션이기에 컴포넌트를 추출하는 연습이 도움이 될 것 같은데 실무에선 어떤 기준을 가지고 컴포넌트를 분리 및 추출하시나요?
일전에 드로이드나이츠 2023 PR들을 구경하며 컴포넌트 분리에 대한 태환님의 의견을 본 적이 있습니다!
드로이드 나이츠 PR(321)
드로이드 나이츠 PR(327)
말리빈은 '아 거 좀 분리좀 해놓지 ^________^'라고 생각했다고 하셨지만, 사실 그 누구도 미래는 알 수 없으니 어쩔 수 없는 부분이라고 생각해요! 그렇다고 모든 컴포넌트를 분리할 순 없으니 말이죠 ㅎㅎ
재사용 가능성이 핵심인 것 같은데.. 아무래도 이 부분은 경험의.. 영역일까요?
쉽게 읽히는 테스트와 거짓테스트 유의하며 개선해보겠습니다!