1week 과제 제출 #2
1week 과제 제출 #2
Conversation
vvan2
left a comment
vvan2
left a comment
There was a problem hiding this comment.
고생 많으셨습니다!! 시험기간이라 겹쳐서 정신없으셨을텐데, 잘 구현해 주신 것 같습니다! 기능 관련해서 아직 완료 안된 부분은 꼭 추가해주세요!! 제가 단 코드 리뷰가 정답은 아니기 때문에 내용을 확인해보면서 궁금한 것이 생기면 말씀해주세요! 도움이 될 것 같아서 자료링크나, 간단한 내용들도 작성해봤습니다.
| <resources> | ||
| <string name="app_name">dive</string> | ||
| <string name="title_activity_login">LoginActivity</string> | ||
| <string name="title_activity_signup">SignupActivity</string> |
There was a problem hiding this comment.
string을 추출하여 사용해 주셨군요, string 을 추출해서 resource와 같이 사용하는 이유가 무엇이며 어떤 상황에 사용하는게 좋을까요? 한 번 찾아보시면서 공부해보는 것을 추천드립니다
|
|
||
| @Preview(showBackground = true) | ||
| @Composable | ||
| fun LoginScreen() { |
There was a problem hiding this comment.
preview를 사용할때에는 다른 곳에서 참조 하지 못하게 접근 제한자를 사용하시는 것을 추천드립니다!
|
|
||
| @Composable | ||
| fun SignupHeaderSection() { | ||
| var text by remember { mutableStateOf("") } |
There was a problem hiding this comment.
현재 var text by remember { mutableStateOf("") }를 사용하고 계신데요!
Text로만 이루어진 SignupHeaderSection() 에서는 사용되지 않고 있습니다. 각각의 역할을 간략하게 나타내자면
-
mutableStateOf: Compose가 값의 변화를 감지하고 자동으로 리컴포지션을 트리거합니다.
textfield 실습 때 배웠 듯이 일반 변수var text = ""는 값이 바뀌어도 Compose가 감지하지 못해 UI가 업데이트되지 않습니다.( -
remember: 리컴포지션이 일어나도 상태 값을 메모리에 보존합니다.
remember가 없으면 리컴포지션 시마다 초기값으로 리셋되어 TextField에 입력한 값이 사라집니다 -
by (위임): .value 접근을 생략하여 코드를 간결하게 만듭니다.
- by 없이: text.value = "hello"
- by 사용: text = "hello"
현재 SignUpHeaderSection은 정적 텍스트만 표시하므로 상태 관리가 불필요합니다. 마침, 이번 2주차때 state에 대해서 배웠으니, 코드를 수정하면서 같이 공부해보시면 좋을 것 같습니다!
|
|
||
| @Composable | ||
| fun LoginHeaderSection() { | ||
| var text by remember { mutableStateOf("") } |
There was a problem hiding this comment.
사용하신 text 에 대한 state 개념은 아래쪽에 달아놨습니다 ! 확인부탁 드릴게요
| fun LoginHeaderSection() { | ||
| var text by remember { mutableStateOf("") } | ||
|
|
||
| Text( |
There was a problem hiding this comment.
Login 쪽과 Signup 쪽에 공통적으로 HeaderSection을 사용하고 있으시네요! HeaderSection이라는 컴포넌트를 컴포져블 함수로 만들어서 재사용하는 것이 좋아보입니다! 컴포넌트의 재사용성과 공통 컴포넌트에 대해서 찾아보시면 도움이 될 것 같습니다.
현재 HeadSection에서 사용하는 Text의 값을 파라미터로 넘겨서 text : String 필요한 곳에서 값을 할당해주면 더욱 편리하겠죠?!
| .padding(10.dp), | ||
| label = { Text("비밀번호 입력") }, | ||
| singleLine = true | ||
| ) |
|
|
||
| @Composable | ||
| fun FieldSection() { | ||
| var text by remember { mutableStateOf("") } |
There was a problem hiding this comment.
현재 Textfield 두개의 value 가 동일한 변수로 사용되고 있습니다! 분리해서 사용해 주세요!
| fontSize = 20.sp, | ||
| fontWeight = FontWeight.Bold, | ||
| modifier = Modifier | ||
| .clickable(onClick = {}) |
There was a problem hiding this comment.
현재 페이지에서는 불필요한 clickable이라 판단 됩니다! 어떤 의도로 사용하신 걸까요?
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .padding(10.dp), | ||
| colors = ButtonDefaults.buttonColors(containerColor = Color.Unspecified), |
There was a problem hiding this comment.
색상을 Unspecified로 설정하기보다는, 제거해서 기본 색상을 사용하거나 특정 color 로 바꿔주는 것이 더 좋아보입니다!
There was a problem hiding this comment.
SignupActivity 또한 login,main activity 에서 작성한 리뷰를 기반으로 수정해보시는 것을 추천드립니다!
cmj7271
left a comment
cmj7271
left a comment
There was a problem hiding this comment.
주완님이 너무 잘 작성해주셔서 더 얹을 말이 별로 없군요... 수고하셨습니닷
| var pwtext by remember { mutableStateOf("") } | ||
|
|
||
| Column( | ||
| modifier = Modifier.fillMaxWidth(), |
There was a problem hiding this comment.
저도 이 부분 고민을 했는데, 이미 잘 적어주셔서 제 생각만 조금 적어두겠습니다.
저는 고민을 해보다가
- 함수인자로 modifier 를 받고, 컴포저블 함수 내 최상위 컴포넌트에 그대로 주입
- 하위 컴포넌트는 필요에 의해 조정
의 순서로 작성했습니다.
1번에서 그대로 주입한 이유는 인자로 받은 Modifier 를 변경해서 설정하면,
외부에서 설정할 때 예기치 못한 행동을 할 수도 있다는 부분을 알게 되었습니다.
그래서, 저라면 최상위 컴포넌트인
Column(
modifier = Modifier.fillMaxWidth(),
// ...에서는 modifier 를 함수인자로 받고,
LoginFieldSection(modifier = Modifier.fillMaxWidth()) 로 사용하지 않았을까 싶습니다!
znayeonzn
left a comment
znayeonzn
left a comment
There was a problem hiding this comment.
시험기간에 과제하시느라 넘넘 수고하셨습니다!! 코드 잘 보고 배우고 가요!! 화이팅 ~
| fontWeight = FontWeight.Bold, | ||
| modifier = Modifier | ||
| .align(Alignment.Start) | ||
| .padding(start = 10.dp) |
There was a problem hiding this comment.
전체적으로 padding을 10으로 주셨는데 Text, TextField에 하나씩 다 입력하기보다는
최상단 modifier에 padding값으로 줘도 좋을 것 같아요!
|
|
||
| fun isValidId(id: String) = id.length in 6..10 | ||
| fun isValidPassword(password: String) = password.length in 8..12 | ||
| fun isValidNickname(nickname: String) = nickname.isNotBlank() && nickname.trim().isNotEmpty() |
There was a problem hiding this comment.
isNotBlank()만으로도 이미 trim().isNotEmpty() 조건을 포함한다고 알고 있습니다..!
fun isValidNickname(nickname: String) = nickname.isNotBlank()
이건 어떨까요?!
| fun SignupFieldSection( | ||
| id: String, | ||
| onIdChange: (String) -> Unit, | ||
| idError: Boolean, | ||
| pw: String, | ||
| onPwChange: (String) -> Unit, | ||
| pwError: Boolean, | ||
| nick: String, | ||
| onNickChange: (String) -> Unit, | ||
| nickError: Boolean, | ||
| mbti: String, | ||
| onMbtiChange: (String) -> Unit | ||
| ) { |
| modifier = Modifier | ||
| .clickable(onClick = {}) | ||
| .padding(start = 10.dp) | ||
| .fillMaxWidth() | ||
| ) |
There was a problem hiding this comment.
1주차 세미나 때 배운 것처럼 modifier의 순서에 대해 찾아보면 좋을 거 같습니다!
지금 코드에서는 UI적으로는 왼쪽으로 10.dp 패딩 값이 있지만, 터치 가능한 영역은 화면 끝까지 확장되어 있는 것 같아요!
UX적인 측면에서 텍스트의 영역만 클릭 가능하도록 순서를 바꿔보는 거 어떨까요?!
chattymin
left a comment
chattymin
left a comment
There was a problem hiding this comment.
세미나 과제 Notion에 보시면 ✅ PR에 시연 영상 추가는 필수입니다. 과제의 모든 기능 명세를 확인할 수 있는 영상을 첨부해주세요.라는 내용이 있습니다. 영상 추가부탁드립니다
필수과제