Conversation
sonms
left a comment
sonms
left a comment
There was a problem hiding this comment.
고생하셨습니다! 전체적으로 타입세이프티 네비게이션과 modifier, 재사용 컴포저블 관련해서 보완할 점이 보입니다! 홧팅홧팅!
| verticalArrangement = Arrangement.spacedBy(31.dp), | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .fillMaxWidth() |
| } | ||
|
|
||
| @Composable | ||
| internal fun DaytimeCard( |
There was a problem hiding this comment.
internal 같은 경우 접근제한자로 같은 모듈에서만 사용할 수 있도록 하는데 현재 저희 프로젝트 구조상 app 모듈 하나만 사용하는 구조라 큰 의미는 없을 것 같은데 넣으신 이유가 있으실까요?
| @Composable | ||
| internal fun DaytimeCard( | ||
| daytime: String, | ||
| daystate: String, |
There was a problem hiding this comment.
재사용 가능한 Composable함수를 만들 때 Modifier를 인자로 받는 방식을 준수해주세요요ㅛㅇ
| ) { | ||
| Column ( | ||
| color = PawKeyTheme.colors.white2, | ||
| shape = RoundedCornerShape(12.dp) |
There was a problem hiding this comment.
background의 shape는 배경은 둥글게 보이지만 그 안에 있는 내용이 값을 넘어설 경우 여전히 표시될 수 있습니다 이를 방지하고자 clip을 사용하는데 현재 상황은 의도된 것인가요?
| shape = RoundedCornerShape(12.dp) | ||
| ), | ||
| ) { | ||
| Column() |
| fontSize = 24.sp | ||
| ), | ||
| modifier = modifier.padding(start = 4.dp) | ||
| modifier = modifier.padding(start = 4.dp), |
There was a problem hiding this comment.
보통 최상위(현재는 Row)에서 한 번 modifier를 사용해서 구현하고 나머지 안쪽 부분은 Modifier를 사용해서 구현하면 됩니다. 이러한 이유는 제가 나중에 질문할 것이니 공부해두세여ㅛ ㅎ
| Text( | ||
| text=rating, | ||
| text = rating, | ||
| style = PawKeyTheme.typography.head22B.copy( |
There was a problem hiding this comment.
이거 여전히 copy 사용 중인데 아직 타이포그래피 적용이 안되서 그런 건가요?
| var isLocationMenuVisible by mutableStateOf(false) | ||
| private set | ||
|
|
||
| fun toggleLocationMenu() { | ||
| isLocationMenuVisible = !isLocationMenuVisible | ||
| } | ||
|
|
||
| fun hideLocationMenu() { | ||
| isLocationMenuVisible = false | ||
| } |
There was a problem hiding this comment.
자 이 부분 state 로 관리해봅시다 현재 dummy 쪽을 보고 공부하시면 좋을 것 같습니다~
|
|
||
| fun navigateCourse(navOptions: NavOptions? = null) { | ||
| navController.navigatePet(navOptions = navOptions) | ||
| } | ||
|
|
There was a problem hiding this comment.
MainTapRoute로 모두 MainNavigator가 graph를 연결 중이라 navigateNext용으로 navigate를 만들 필요가 없습니다. 삭제하셔도 무방합니다.
| paddingValues = paddingValues, | ||
| navigateUp = navigator.navController::navigateUp, | ||
| navigateNext = navigator.navController::navigateUp, | ||
| navigateNext = navigator::navigateCourse, |
3 participants
name: pull_request_template
about: pr 생성용 템플릿입니다~
title: ''
labels: ''
assignees: ''
ISSUE
❗ WORK DESCRIPTIONAdd commentMore actions
📸 SCREENSHOT
📢 TO REVIEWERS