-
Notifications
You must be signed in to change notification settings - Fork 20
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
[호두] 회원가입 미션 1, 2단계 제출합니다. #14
base: junyoung-won
Are you sure you want to change the base?
Conversation
- 추후 상태 값 활용 예정
- 불필요한 import 제거 - SignUpTitle 을 SignUpLayouts.kt 로 이동 - Surface 의 padding 속성을 SignUpLayout 에 적용
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단계 미션 요구사항 잘 구현해주신 것 같네요 !
특히 요구사항에 해당하는 부분을 커밋단위로 구분해주셔서 리뷰하기 편했습니다.
코멘트 몇개 남겨두었으니 확인 부탁드려요~
// 2. 힌트를 참고하여 Preview를 노출시킨다. | ||
// 3. Preview의 interactive 모드를 활용하여 버튼을 클릭해본다. | ||
|
||
class LayoutBasicsTest { |
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.
오홍 README에 성공해야 하는 테스트를 추가했군요~~ 👍
) | ||
|
||
@Composable | ||
fun SignupTheme( | ||
fun SignUpTheme( |
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: String, | ||
onClick: () -> Unit, | ||
) { | ||
SignUpTheme { |
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.
(궁금🤔) 컴포넌트에 따로 테마를 지정해준 이유가 있나요 ?
현재 앱의 루트에서 SignUpTheme을 적용시켜주고 있는데 컴포넌트마다 따로 테마를 지정할 필요가 없을 것 같아요 !
import androidx.compose.ui.text.input.PasswordVisualTransformation | ||
import androidx.compose.ui.text.input.VisualTransformation | ||
|
||
enum class InputType( |
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.
헐 대박~ InputType에 대해 enum 클래스로 정의해두셨네요 !
저는 password에 대해서만 처리해줬어서 분리하지 않았는데, Email과 같은 다른 textfield가 추가되면 enum클래스로 관리할 수도 있겠네요 ! 👍
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한 함수의 선언 위치가 큰 단위부터 작은 단위로 가는게 조금 더 가독성 있는 것 같아요 !
Layout부터 나오고 그 아래로 호출되는 Component 함수가 나오는게 자연스러운 것 같습니다 !
아마 그래서 호두가 @Preview
를 제일 위에 선언 한 것 같은데 Preview
함수를 제일 밑에 선언하는게 관습? 컨벤션 이라고 하더라구요.
컴포즈 코드랩에서도 preview가 맨 밑에 있습니다 !
Composable
fun Greeting(name: String, modifier: Modifier = Modifier) {
Surface(color = MaterialTheme.colorScheme.primary) {
Text(
text = "Hello $name!",
modifier = modifier.padding(24.dp)
)
}
}
@Preview(showBackground = true)
@Composable
fun GreetingPreview() {
BasicsCodelabTheme {
MyApp()
}
}
} | ||
|
||
@Composable | ||
fun SignUpInteractionLayer( |
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.
InteractionLayer
와 SignUpLayout
로 나누게 된 이유가 궁금합니다 !
) { | ||
var value by rememberSaveable { mutableStateOf("") } | ||
TextField( | ||
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.
지금처럼 OneLineTextInput
내부에서 modifier를 만들어서 관리한다면 해당 TextInput 컴포넌트는 동일한 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.
현재 Ktlint가 통과하지 않고 있어유 🥲
[*.{kt,kts}]
ktlint_function_naming_ignore_when_annotated_with=Composable
오둥이가 올려준 방법을 참고하여 Ktlint를 설정할 수 있을 것 같아요 !
val Typography = Typography( | ||
bodyLarge = TextStyle( | ||
titleLarge = TextStyle( |
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.
타이포그래피 설정을 통해 스타일을 쉽게 관리할 수 있군요~~ 배워갑니다 !
안녕하세요!! 꼬상~~~ 🐿️ 😆
오랜만이야!!!ㅋㅋㅋㅋㅋ
꼬상은 레벨 3때 선릉에서 한번 봤지만... 그래도 그립네요!!
오랜만에 미션에서 만나게 되었는데, 이번에는 리뷰어&리뷰이로 만났네요! 재밌을 것 같아요!!
열심히 공부해서 서로 많이 배워가고 이야기도 많이 나누면 좋겠습니다! 잘 부탁드려요~ 🙇♂️
나름 생각한대로 개선&리팩터링 해보았습니다!
그래도 아직은 이렇게 하는게 맞는지 잘 모르겠네요...
주요 구현 내용과 추가로 필요한 사항들을 아래에 정리해보았습니다!
주요 구현 내용
1단계
2단계
TextField
에 활용하였습니다.추가로 필요한 사항
Composable
들에 대한 테스트 코드 작성이 필요합니다!