-
Notifications
You must be signed in to change notification settings - Fork 2
[Mod/#52] SignUp 뷰 GUI 적용 및 오류 수정 #55
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
sonms
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.
굿굿 전체적인 디테일만 좀 잡으면 좋을 것 같습니다~
| item{} | ||
| item{} |
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.
item ㅎㅎ 수정~
| Spacer(modifier = Modifier.weight(1f)) | ||
|
|
||
| val isFormValid = state.selectedLocation.isNotEmpty() | ||
| val isFormValid = state.selectedLocation.isNotEmpty() |
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.
폼 판별 변수는 상태로 뷰모델에서 map과 statein으로 관리해보아용
| val isFormValid = state.name.isNotBlank() && | ||
| state.age.isNotBlank() && | ||
| state.selectedGender != SignUpContract.Gender.UNKNOWN | ||
| val isFormValid = state.name.isNotBlank() && |
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.
이거도오ㅗ오오
| onValueChange: (String) -> Unit, | ||
| modifier: Modifier = Modifier, | ||
| placeholder: 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.
기본값이 필요없었다면? Compose 파라미터 기준에 따라 modifier 위로~
| ) | ||
| Column( | ||
| modifier = Modifier | ||
| .fillMaxSize() |
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.
상위에서 fillmaxsize인데 여기서 fillmaxsize로 가린 이유가 있을까요?
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.
하위 요소들만 16dp라 중첩해서 넣었는데 하위 column쪽 요소는 빼고 수정하겠습니다
| ) | ||
| } | ||
|
|
||
| private fun isAgeValid(ageKnown: SignUpContract.AgeKnown, dogAge: String): Boolean { |
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() | ||
| .padding(horizontal = 16.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.
여기도 마찬가지로 상위에 fillmaxsize인데 여기서 flllmaxsize 인 이유?
name: pull_request_template
about: pr 생성용 템플릿입니다~
title: ''
labels: ''
assignees: ''
ISSUE
❗ WORK DESCRIPTIONAdd commentMore actions
📸 SCREENSHOT
📢 TO REVIEWERS