-
Notifications
You must be signed in to change notification settings - Fork 0
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
[feat] :: 잔류 신청 퍼블리싱 #105
The head ref may contain hidden characters: "feature/104-feat-\uC794\uB958-\uC2E0\uCCAD-\uD37C\uBE14\uB9AC\uC2F1"
[feat] :: 잔류 신청 퍼블리싱 #105
Conversation
워크스루이 풀 리퀘스트는 DMS 애플리케이션에 잔류 신청 및 외출 신청 기능을 추가합니다. 새로운 모듈 변경 사항
가능한 관련 이슈
시퀀스 다이어그램sequenceDiagram
participant User
participant Navigator
participant RemainScreen
participant OutingScreen
User->>Navigator: 잔류/외출 신청 선택
Navigator->>RemainScreen: navigateToRemainApplication()
Navigator->>OutingScreen: navigateToOutingApplication()
토끼의 시 🐰
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 8
🧹 Nitpick comments (2)
core/design-system/src/commonMain/kotlin/team/aliens/dms/kmp/core/designsystem/tag/DmsTag.kt (1)
37-42
: 색상 설정 중복을 개선해 주세요
DmsText
의 color 속성이contentColor
파라미터와 중복되어 있습니다. Surface의contentColor
가 자동으로 전파되므로DmsText
의 명시적 색상 설정은 불필요합니다.다음과 같이 수정을 제안합니다:
DmsText( modifier = Modifier.padding(contentPadding), text = text, style = DmsTypography.Caption, - color = DmsTheme.colors.inversePrimary, )
core/design-system/src/commonMain/kotlin/team/aliens/dms/kmp/core/designsystem/float/DmsFloatingNotice.kt (1)
40-43
: 접근성 개선이 필요합니다아이콘의
contentDescription
이 null로 설정되어 있어 스크린 리더 사용자의 접근성이 제한됩니다. 적절한 설명을 추가해 주세요.다음과 같이 수정을 제안합니다:
Icon( painter = painterResource(iconResource), - contentDescription = null, + contentDescription = "알림 아이콘", )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
composeApp/build.gradle.kts
(1 hunks)composeApp/src/commonMain/kotlin/team/aliens/dms/kmp/DmsNavigator.kt
(2 hunks)composeApp/src/commonMain/kotlin/team/aliens/dms/kmp/navigation/main/MainNavigation.kt
(2 hunks)composeApp/src/commonMain/kotlin/team/aliens/dms/kmp/root/RootNavigation.kt
(1 hunks)composeApp/src/commonMain/kotlin/team/aliens/dms/kmp/root/RootScreen.kt
(2 hunks)core/design-system/src/commonMain/kotlin/team/aliens/dms/kmp/core/designsystem/float/DmsFloatingNotice.kt
(1 hunks)core/design-system/src/commonMain/kotlin/team/aliens/dms/kmp/core/designsystem/tag/DmsTag.kt
(1 hunks)feature/application/src/commonMain/kotlin/team/aliens/dms/kmp/feature/application/navigation/ApplicationNavigation.kt
(1 hunks)feature/application/src/commonMain/kotlin/team/aliens/dms/kmp/feature/application/ui/ApplicationScreen.kt
(3 hunks)feature/outing/build.gradle.kts
(1 hunks)feature/outing/src/commonMain/kotlin/team/aliens/dms/kmp/feature/outing/OutingApplicationScreen.kt
(1 hunks)feature/outing/src/commonMain/kotlin/team/aliens/dms/kmp/feature/outing/navigation/OutingApplicationNavigation.kt
(1 hunks)feature/remain/build.gradle.kts
(1 hunks)feature/remain/src/commonMain/kotlin/tema/aliens/dms/kmp/feature/remain/RemainApplicationScreen.kt
(1 hunks)feature/remain/src/commonMain/kotlin/tema/aliens/dms/kmp/feature/remain/navigation/RemainApplicationNavigation.kt
(1 hunks)settings.gradle.kts
(1 hunks)
👮 Files not reviewed due to content moderation or server errors (4)
- composeApp/src/commonMain/kotlin/team/aliens/dms/kmp/root/RootScreen.kt
- composeApp/src/commonMain/kotlin/team/aliens/dms/kmp/DmsNavigator.kt
- feature/application/src/commonMain/kotlin/team/aliens/dms/kmp/feature/application/ui/ApplicationScreen.kt
- feature/remain/src/commonMain/kotlin/tema/aliens/dms/kmp/feature/remain/RemainApplicationScreen.kt
🧰 Additional context used
🪛 GitHub Actions: Kotlin Multiplatform CI
composeApp/src/commonMain/kotlin/team/aliens/dms/kmp/navigation/main/MainNavigation.kt
[warning] 23-23: Missing trailing comma before ')'
composeApp/src/commonMain/kotlin/team/aliens/dms/kmp/root/RootNavigation.kt
[warning] 18-18: Missing trailing comma before ')'
feature/remain/build.gradle.kts
[warning] 23-23: Trailing spaces detected
[warning] 50-50: Unnecessary long whitespace and missing newline before '}'
feature/outing/src/commonMain/kotlin/team/aliens/dms/kmp/feature/outing/OutingApplicationScreen.kt
[warning] 14-16: Missing trailing comma, unexpected blank line before '}', and empty first line in method block
feature/outing/build.gradle.kts
[warning] 23-23: Trailing spaces detected
feature/remain/src/commonMain/kotlin/tema/aliens/dms/kmp/feature/remain/RemainApplicationScreen.kt
[error] 146-151: Incorrect indentation: found 24 spaces, expected 20 spaces
🪛 detekt (1.23.7)
feature/outing/src/commonMain/kotlin/team/aliens/dms/kmp/feature/outing/OutingApplicationScreen.kt
[warning] 15-17: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
🔇 Additional comments (8)
feature/remain/src/commonMain/kotlin/tema/aliens/dms/kmp/feature/remain/navigation/RemainApplicationNavigation.kt (1)
8-18
: 구현이 깔끔합니다!네비게이션 구현이 Compose Navigation 패턴을 잘 따르고 있으며, 코드가 간결합니다.
feature/outing/src/commonMain/kotlin/team/aliens/dms/kmp/feature/outing/navigation/OutingApplicationNavigation.kt (1)
8-18
: 구현이 일관성 있고 깔끔합니다!RemainApplicationNavigation과 동일한 패턴으로 구현되어 있어 일관성이 좋습니다.
feature/application/src/commonMain/kotlin/team/aliens/dms/kmp/feature/application/navigation/ApplicationNavigation.kt (1)
9-17
: 콜백 구현이 적절합니다!네비게이션 콜백이 잘 추가되었으며, 파라미터 전달이 깔끔하게 구현되었습니다.
settings.gradle.kts (1)
50-51
: 새로운 기능 모듈이 올바르게 추가되었습니다!
outing
과remain
기능 모듈이 프로젝트의 기존 모듈 구조를 따라 적절하게 추가되었습니다.feature/outing/build.gradle.kts (1)
1-67
: 모듈 구성이 적절합니다.Kotlin Multiplatform 설정과 의존성이 프로젝트 표준을 잘 따르고 있습니다.
🧰 Tools
🪛 GitHub Actions: Kotlin Multiplatform CI
[warning] 23-23: Trailing spaces detected
composeApp/build.gradle.kts (1)
74-74
: 의존성이 올바르게 추가되었습니다!
remain
기능 모듈의 의존성이 기존 패턴을 따라 적절하게 추가되었습니다.composeApp/src/commonMain/kotlin/team/aliens/dms/kmp/navigation/main/MainNavigation.kt (1)
22-22
: 외출 신청 네비게이션 구현이 필요합니다
onNavigateOutingApplication
이 빈 람다로 구현되어 있습니다. 외출 신청 기능이 필요한 경우 적절한 네비게이션 구현이 필요합니다.feature/outing/src/commonMain/kotlin/team/aliens/dms/kmp/feature/outing/OutingApplicationScreen.kt (1)
31-31
: 뒤로가기 동작 구현이 필요합니다
onBackPressed
핸들러가 비어있습니다. 적절한 네비게이션 동작을 구현해 주세요.
...mmonMain/kotlin/tema/aliens/dms/kmp/feature/remain/navigation/RemainApplicationNavigation.kt
Show resolved
Hide resolved
Root( | ||
onNavigateRemainApplication = onNavigateRemainApplication, | ||
onNavigateOutingApplication = onNavigateOutingApplication, | ||
onNoticeDetailClick = onNoticeDetailsClick | ||
) |
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.
트레일링 콤마를 추가해주세요
CI 파이프라인 경고에 따라 마지막 파라미터 뒤에 트레일링 콤마를 추가해야 합니다.
Root(
onNavigateRemainApplication = onNavigateRemainApplication,
onNavigateOutingApplication = onNavigateOutingApplication,
- onNoticeDetailClick = onNoticeDetailsClick
+ onNoticeDetailClick = onNoticeDetailsClick,
)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Root( | |
onNavigateRemainApplication = onNavigateRemainApplication, | |
onNavigateOutingApplication = onNavigateOutingApplication, | |
onNoticeDetailClick = onNoticeDetailsClick | |
) | |
Root( | |
onNavigateRemainApplication = onNavigateRemainApplication, | |
onNavigateOutingApplication = onNavigateOutingApplication, | |
onNoticeDetailClick = onNoticeDetailsClick, | |
) |
🧰 Tools
🪛 GitHub Actions: Kotlin Multiplatform CI
[warning] 18-18: Missing trailing comma before ')'
feature/outing/build.gradle.kts
Outdated
} | ||
} | ||
} | ||
|
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.
불필요한 공백 문자를 제거해 주세요.
파이프라인 검사에서 23번 줄에 불필요한 공백이 감지되었습니다.
-
+
🧰 Tools
🪛 GitHub Actions: Kotlin Multiplatform CI
[warning] 23-23: Trailing spaces detected
feature/remain/build.gradle.kts
Outdated
} | ||
|
||
android { | ||
namespace = "tema.aliens.dms.kmp.feature.remain" |
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.
네임스페이스 오타를 수정해 주세요.
네임스페이스에 중요한 오타가 있습니다: "tema"가 "team"이어야 합니다.
- namespace = "tema.aliens.dms.kmp.feature.remain"
+ namespace = "team.aliens.dms.kmp.feature.remain"
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
namespace = "tema.aliens.dms.kmp.feature.remain" | |
namespace = "team.aliens.dms.kmp.feature.remain" |
feature/remain/build.gradle.kts
Outdated
implementation(libs.koin.compose.viewmodel) | ||
|
||
implementation(projects.core.designSystem) | ||
implementation(projects.core.common) } |
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.
코드 포맷팅을 수정해 주세요.
50번 줄의 불필요한 공백과 누락된 줄바꿈을 수정해야 합니다.
- implementation(projects.core.common) }
+ implementation(projects.core.common)
+ }
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
implementation(projects.core.common) } | |
implementation(projects.core.common) | |
} |
🧰 Tools
🪛 GitHub Actions: Kotlin Multiplatform CI
[warning] 50-50: Unnecessary long whitespace and missing newline before '}'
feature/remain/build.gradle.kts
Outdated
} | ||
} | ||
} | ||
|
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.
불필요한 공백 문자를 제거해 주세요.
파이프라인 검사에서 23번 줄에 불필요한 공백이 감지되었습니다.
-
+
🧰 Tools
🪛 GitHub Actions: Kotlin Multiplatform CI
[warning] 23-23: Trailing spaces detected
onNavigateRemainApplication = navigator::navigateToRemainApplication, | ||
onNavigateOutingApplication = {}, | ||
onNoticeDetailsClick = navigator::navigateToNoticeDetails | ||
) |
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.
🛠️ Refactor suggestion
코드 스타일 개선이 필요합니다
파이프라인 실패를 해결하기 위해 trailing comma를 추가해 주세요.
다음과 같이 수정해 주세요:
root(
onNavigateRemainApplication = navigator::navigateToRemainApplication,
- onNavigateOutingApplication = {},
+ onNavigateOutingApplication = {},
onNoticeDetailsClick = navigator::navigateToNoticeDetails
)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
onNavigateRemainApplication = navigator::navigateToRemainApplication, | |
onNavigateOutingApplication = {}, | |
onNoticeDetailsClick = navigator::navigateToNoticeDetails | |
) | |
onNavigateRemainApplication = navigator::navigateToRemainApplication, | |
onNavigateOutingApplication = {}, | |
onNoticeDetailsClick = navigator::navigateToNoticeDetails | |
) |
🧰 Tools
🪛 GitHub Actions: Kotlin Multiplatform CI
[warning] 23-23: Missing trailing comma before ')'
internal fun OutingApplication( | ||
modifier: 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.
OutingApplication 구현이 필요합니다
OutingApplication
컴포저블이 비어있습니다. OutingApplicationScreen
을 호출하도록 구현이 필요합니다.
다음과 같이 구현을 제안합니다:
@Composable
internal fun OutingApplication(
- modifier: Modifier = Modifier
+ modifier: Modifier = Modifier,
) {
-
+ OutingApplicationScreen(
+ modifier = modifier,
+ )
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
internal fun OutingApplication( | |
modifier: Modifier = Modifier | |
) { | |
} | |
internal fun OutingApplication( | |
modifier: Modifier = Modifier, | |
) { | |
OutingApplicationScreen( | |
modifier = modifier, | |
) | |
} |
🧰 Tools
🪛 detekt (1.23.7)
[warning] 15-17: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
🪛 GitHub Actions: Kotlin Multiplatform CI
[warning] 14-16: Missing trailing comma, unexpected blank line before '}', and empty first line in method block
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.
Actionable comments posted: 6
♻️ Duplicate comments (1)
feature/outing/src/commonMain/kotlin/team/aliens/dms/kmp/feature/outing/OutingApplicationScreen.kt (1)
12-17
:⚠️ Potential issueOutingApplication 구현이 누락되었습니다
OutingApplication
컴포저블이 비어있어OutingApplicationScreen
을 표시할 수 없습니다. 이전 리뷰에서 지적된 사항이 아직 해결되지 않았습니다.다음과 같이 수정해주세요:
@Composable internal fun OutingApplication( modifier: Modifier = Modifier, ) { - + OutingApplicationScreen( + modifier = modifier, + ) }🧰 Tools
🪛 detekt (1.23.7)
[warning] 15-17: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
🪛 GitHub Actions: Kotlin Multiplatform CI
[error] 16-16: Unexpected blank line(s) before '}' (standard:no-blank-line-before-rbrace)
[error] 16-16: First line in a method block should not be empty (standard:no-empty-first-line-in-method-block)
🧹 Nitpick comments (2)
feature/outing/src/commonMain/kotlin/team/aliens/dms/kmp/feature/outing/OutingApplicationScreen.kt (1)
27-27
: 레이아웃 배치 방식 검토가 필요합니다
verticalArrangement = Arrangement.Center
는 폼 화면에 적절하지 않을 수 있습니다. 일반적으로 폼은 상단에서 시작하여 아래로 내용이 나열되는 것이 사용자 경험에 더 적합합니다.다음과 같이 수정을 제안드립니다:
- verticalArrangement = Arrangement.Center, + verticalArrangement = Arrangement.Top,feature/remain/build.gradle.kts (1)
35-56
: 의존성 그룹에 주석을 추가하면 좋을 것 같습니다.각 의존성 그룹(Compose, Koin, Core)에 구분 주석을 추가하면 가독성이 향상될 것 같습니다.
다음과 같이 수정해보세요:
sourceSets { commonMain.dependencies { + // Compose dependencies implementation(compose.runtime) implementation(compose.foundation) implementation(compose.material) implementation(compose.material3) implementation(compose.ui) implementation(compose.components.resources) implementation(libs.navigation.compose) + // Koin dependencies implementation(libs.koin.core) implementation(libs.koin.compose) implementation(libs.koin.compose.viewmodel) + // Core module dependencies implementation(projects.core.designSystem) implementation(projects.core.common) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
composeApp/src/commonMain/kotlin/team/aliens/dms/kmp/navigation/main/MainNavigation.kt
(2 hunks)composeApp/src/commonMain/kotlin/team/aliens/dms/kmp/root/RootNavigation.kt
(1 hunks)feature/outing/src/commonMain/kotlin/team/aliens/dms/kmp/feature/outing/OutingApplicationScreen.kt
(1 hunks)feature/remain/build.gradle.kts
(1 hunks)feature/remain/src/commonMain/kotlin/tema/aliens/dms/kmp/feature/remain/RemainApplicationScreen.kt
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- composeApp/src/commonMain/kotlin/team/aliens/dms/kmp/navigation/main/MainNavigation.kt
- composeApp/src/commonMain/kotlin/team/aliens/dms/kmp/root/RootNavigation.kt
🧰 Additional context used
🪛 detekt (1.23.7)
feature/outing/src/commonMain/kotlin/team/aliens/dms/kmp/feature/outing/OutingApplicationScreen.kt
[warning] 15-17: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
🪛 GitHub Actions: Kotlin Multiplatform CI
feature/outing/src/commonMain/kotlin/team/aliens/dms/kmp/feature/outing/OutingApplicationScreen.kt
[error] 16-16: Unexpected blank line(s) before '}' (standard:no-blank-line-before-rbrace)
[error] 16-16: First line in a method block should not be empty (standard:no-empty-first-line-in-method-block)
🔇 Additional comments (4)
feature/outing/src/commonMain/kotlin/team/aliens/dms/kmp/feature/outing/OutingApplicationScreen.kt (1)
29-32
:⚠️ Potential issue뒤로가기 동작 구현이 필요합니다
onBackPressed
핸들러가 비어있어 사용자가 뒤로가기를 할 수 없습니다. 네비게이션 로직을 구현해주세요.추가로, 잔류 신청 폼의 나머지 UI 요소들도 구현이 필요해 보입니다. 현재는 상단 앱바만 구현되어 있습니다.
feature/remain/build.gradle.kts (3)
5-11
: 플러그인 구성이 적절합니다!필요한 모든 플러그인이 버전 카탈로그를 통해 올바르게 구성되어 있습니다.
23-23
: 불필요한 공백 라인을 제거해 주세요.23번 줄의 불필요한 공백을 제거해야 합니다.
59-59
: 네임스페이스 오타를 수정해 주세요.네임스페이스에 오타가 있습니다: "tema"를 "team"으로 수정해야 합니다.
DmsTopAppBar( | ||
title = "잔류 신청", | ||
onBackPressed = {}, | ||
) |
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.
뒤로가기 동작 구현 필요
onBackPressed
핸들러가 비어있습니다. 상위 컴포넌트로부터 네비게이션 콜백을 전달받아 처리해야 합니다.
title = "잔류 신청", | ||
onBackPressed = {}, |
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.
🛠️ Refactor suggestion
문자열 리소스 분리 필요
하드코딩된 문자열들을 리소스로 분리하여 다국어 지원을 용이하게 해야 합니다:
- "잔류 신청"
- "잔류 신청 시간은 화 18:00 ~ 목 18:00 까지 입니다."
Also applies to: 64-65
items(3) { | ||
var selected by remember { mutableStateOf(false) } | ||
RemainOptionCard( | ||
isSelected = selected, | ||
isShowDetail = true, | ||
onClick = { selected = it }, | ||
) | ||
} |
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.
🛠️ Refactor suggestion
옵션 데이터 모델링 개선 필요
현재 구현에서 발견된 문제점들:
- 하드코딩된 아이템 개수 (3)
isShowDetail
파라미터가 항상true
로 전달됨- 옵션 데이터가 모델링되어 있지 않음
아래와 같은 개선이 필요합니다:
- 옵션을 데이터 클래스로 모델링
- 옵션 목록을 상태로 관리
data class RemainOption(
val title: String,
val description: String,
val isCompleted: Boolean,
)
@Composable
private fun RemainApplicationScreen(
options: List<RemainOption>,
selectedOptionId: Int?,
onOptionSelected: (Int) -> Unit,
)
DmsButton( | ||
modifier = Modifier | ||
.fillMaxWidth() | ||
.horizontalPadding(24.dp) | ||
.verticalPadding(16.dp), | ||
text = "변경하기", | ||
buttonType = ButtonType.Contained, | ||
buttonColor = ButtonColor.Primary, | ||
onClick = {}, | ||
) |
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.
버튼 상태 관리 및 이벤트 처리 구현 필요
변경하기 버튼에 대한 개선사항:
- 옵션 선택 여부에 따른 활성화 상태 관리
- 클릭 이벤트 핸들러 구현
DmsButton(
modifier = Modifier
.fillMaxWidth()
.horizontalPadding(24.dp)
.verticalPadding(16.dp),
text = "변경하기",
buttonType = ButtonType.Contained,
buttonColor = ButtonColor.Primary,
- onClick = {},
+ enabled = selectedOptionId != null,
+ onClick = onSubmit,
)
Committable suggestion skipped: line range outside the PR's diff.
@Composable | ||
internal fun RemainApplication() { | ||
RemainApplicationScreen() | ||
} |
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.
🛠️ Refactor suggestion
네비게이션 파라미터 추가 필요
네비게이션 처리를 위해 onBackPressed
파라미터를 추가하고 RemainApplicationScreen
으로 전달하는 것이 좋습니다.
@Composable
-internal fun RemainApplication() {
- RemainApplicationScreen()
+internal fun RemainApplication(
+ onBackPressed: () -> Unit,
+) {
+ RemainApplicationScreen(
+ onBackPressed = onBackPressed,
+ )
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
@Composable | |
internal fun RemainApplication() { | |
RemainApplicationScreen() | |
} | |
@Composable | |
internal fun RemainApplication( | |
onBackPressed: () -> Unit, | |
) { | |
RemainApplicationScreen( | |
onBackPressed = onBackPressed, | |
) | |
} |
@Composable | ||
private fun RemainOptionCard( | ||
modifier: Modifier = Modifier, | ||
isSelected: Boolean, | ||
isShowDetail: Boolean, | ||
onClick: (Boolean) -> Unit, | ||
) { | ||
var showDetail by remember { mutableStateOf(false) } | ||
Column( | ||
modifier = modifier | ||
.fillMaxWidth() | ||
.clickable( | ||
onClick = { onClick(!isSelected) }, | ||
) | ||
.border( | ||
width = 1.dp, | ||
color = DmsTheme.colors.inversePrimary, | ||
shape = RoundedCornerShape(12.dp), | ||
) | ||
.background( | ||
color = DmsTheme.colors.background, | ||
shape = RoundedCornerShape(12.dp), | ||
) | ||
.padding( | ||
horizontal = 20.dp, | ||
vertical = 14.dp, | ||
), | ||
) { | ||
Row( | ||
verticalAlignment = Alignment.CenterVertically, | ||
horizontalArrangement = Arrangement.spacedBy(16.dp), | ||
) { | ||
DmsText( | ||
text = "금요 귀가", | ||
style = DmsTypography.Title2, | ||
color = DmsTheme.colors.surfaceBright, | ||
) | ||
DmsTag(text = "신청 완료") | ||
Spacer(modifier = Modifier.weight(1f)) | ||
DmsIconButton( | ||
resource = DmsIcon.Forward, | ||
onClick = { showDetail = !showDetail }, | ||
) | ||
} | ||
AnimatedVisibility(visible = showDetail) { | ||
DmsText( | ||
modifier = Modifier.topPadding(24.dp), | ||
text = "금요일 일과가 모두 끝나고 8시 30분 이후부터9시 30분까지 귀가하고 일요일 6시 30분 부터 9시 30분까지 귀사해야 합니다. 혹시나 개인 일정으로 부득이하기 금요일이 아닌, 토요일 또는 일요일에 귀가해야 하는 학생들은 사감 선생님께 말씀부탁드립니다.", | ||
style = DmsTypography.Body3, | ||
color = DmsTheme.colors.onBackground, | ||
) | ||
} | ||
} | ||
} |
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.
🛠️ Refactor suggestion
RemainOptionCard 컴포넌트 개선 필요
개선이 필요한 부분들:
showDetail
과isShowDetail
상태 중복- 하드코딩된 제목 ("금요 귀가")과 설명 텍스트
- "신청 완료" 태그의 조건부 표시 로직 누락
RemainOption
데이터 클래스를 활용하여 리팩토링이 필요합니다.
@Composable
private fun RemainOptionCard(
modifier: Modifier = Modifier,
+ option: RemainOption,
isSelected: Boolean,
- isShowDetail: Boolean,
+ showDetail: Boolean,
+ onShowDetailChange: (Boolean) -> Unit,
onClick: (Boolean) -> Unit,
) {
- var showDetail by remember { mutableStateOf(false) }
// ...
- DmsText(text = "금요 귀가")
+ DmsText(text = option.title)
// ...
- DmsTag(text = "신청 완료")
+ if (option.isCompleted) {
+ DmsTag(text = "신청 완료")
+ }
// ...
- onClick = { showDetail = !showDetail },
+ onClick = onShowDetailChange,
// ...
- DmsText(text = "금요일 일과가 모두 끝나고...")
+ DmsText(text = option.description)
}
Committable suggestion skipped: line range outside the PR's diff.
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
feature/outing/src/commonMain/kotlin/team/aliens/dms/kmp/feature/outing/OutingApplicationScreen.kt (1)
21-26
: 레이아웃 배치 방식 검토가 필요합니다현재
verticalArrangement = Arrangement.Center
로 설정되어 있어 모든 컨텐츠가 화면 중앙에 배치됩니다. 일반적인 신청 화면의 경우 컨텐츠가 상단부터 시작되는 것이 더 자연스러울 수 있습니다.다음과 같이 수정하는 것을 고려해보세요:
Column( modifier = modifier .fillMaxSize() .background(DmsTheme.colors.background), - verticalArrangement = Arrangement.Center, + verticalArrangement = Arrangement.Top, )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
feature/outing/build.gradle.kts
(1 hunks)feature/outing/src/commonMain/kotlin/team/aliens/dms/kmp/feature/outing/OutingApplicationScreen.kt
(1 hunks)feature/remain/build.gradle.kts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- feature/outing/build.gradle.kts
- feature/remain/build.gradle.kts
🧰 Additional context used
🪛 detekt (1.23.7)
feature/outing/src/commonMain/kotlin/team/aliens/dms/kmp/feature/outing/OutingApplicationScreen.kt
[warning] 15-15: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build iOS
🔇 Additional comments (1)
feature/outing/src/commonMain/kotlin/team/aliens/dms/kmp/feature/outing/OutingApplicationScreen.kt (1)
12-15
: OutingApplication 구현이 여전히 비어있습니다
OutingApplication
컴포저블이 비어있는 상태입니다.OutingApplicationScreen
을 호출하도록 구현해주세요.다음과 같이 구현하시는 것을 제안드립니다:
@Composable internal fun OutingApplication( modifier: Modifier = Modifier, ) { + OutingApplicationScreen( + modifier = modifier, + ) }🧰 Tools
🪛 detekt (1.23.7)
[warning] 15-15: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
title = "외출 신청", | ||
onBackPressed = { }, |
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.
뒤로가기 동작이 구현되지 않았습니다
onBackPressed
핸들러가 비어있습니다. 사용자가 뒤로가기를 눌렀을 때의 동작을 구현해주세요.
개요
작업사항
추가 로 할 말
Summary by CodeRabbit
릴리즈 노트
새로운 기능
개선 사항
기술적 변경