Conversation
|
""" 워크스루이 풀 리퀘스트는 공지사항 기능의 탐색 및 UI 구현에 중점을 둔 변경 사항을 포함합니다. 새로운 네비게이션 경로, 의존성 주입 모듈, 뷰 모델, 그리고 공지사항 상세 및 목록 화면을 위한 컴포저블 함수들이 추가되었습니다. 이러한 변경은 애플리케이션의 공지사항 기능을 확장하고 구조화하는 데 초점을 맞추고 있습니다. 변경 사항
제안된 라벨
관련 가능성 있는 PR
토끼의 시 🐰
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (9)
feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/viewmodel/NoticesViewModel.kt (2)
5-11: 함수 네이밍 개선 제안
현재setIsRecent()메서드는isRecent값을 직접 대입하기보다는 토글하는 동작을 수행하고 있으므로, 보다 직관적인 메서드명(toggleIsRecent()등)으로 변경하면 가독성과 유지보수에 도움이 될 것으로 보입니다.
23-23: SideEffect 인터페이스 정의
NoticesSideEffect가 현재 내부에 구현된 내용은 없지만, 확장 가능성을 위해 선언한 것으로 보입니다. 향후 사용 시 명시적인 목적(네비게이션, 메시지 표시 등)을 코멘트로 남겨주시면 협업에 도움이 될 것입니다.feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/viewmodel/NoticeDetailsViewModel.kt (4)
5-5: BaseViewModel 타입 매개변수 뒤 공백 추가 권장
클래스 선언의 타입 매개변수 사이에 공백이 없어 가독성이 떨어질 수 있습니다. 아래와 같이 수정하는 방안을 제안드립니다.-internal class NoticeDetailsViewModel : BaseViewModel<NoticeDetailsState,NoticeDetailsSideEffect>(NoticeDetailsState.getDefaultState()) { +internal class NoticeDetailsViewModel : BaseViewModel<NoticeDetailsState, NoticeDetailsSideEffect>(NoticeDetailsState.getDefaultState()) {🧰 Tools
🪛 detekt (1.23.7)
[warning] 5-7: The class or object NoticeDetailsViewModel is empty.
(detekt.empty-blocks.EmptyClassBlock)
🪛 GitHub Actions: Kotlin Multiplatform CI
[warning] 5-5: Missing spacing after comma
[warning] 5-5: Unnecessary empty block ({})
5-7: 클래스 본문이 비어 있음
NoticeDetailsViewModel이 현재 빈 블록 상태로 남아있습니다. ViewModel에서 처리할 로직 또는 State 변경 로직이 없다면, 불필요한 빈 블록을 제거하거나 코멘트를 남겨 목적을 명시해 주시기 바랍니다.🧰 Tools
🪛 detekt (1.23.7)
[warning] 5-7: The class or object NoticeDetailsViewModel is empty.
(detekt.empty-blocks.EmptyClassBlock)
🪛 GitHub Actions: Kotlin Multiplatform CI
[warning] 5-5: Missing spacing after comma
[warning] 5-5: Unnecessary empty block ({})
[warning] 6-6: Unexpected blank line(s) before }
6-6: 불필요한 공백 라인 존재
메서드 블록이 비어있는 상태에서 불필요한 빈 줄이 존재합니다. 가능하면 제거해 좀 더 깔끔하게 정리해볼 것을 제안드립니다.5| internal class NoticeDetailsViewModel : BaseViewModel<NoticeDetailsState, NoticeDetailsSideEffect>(NoticeDetailsState.getDefaultState()) { -6| 7| }🧰 Tools
🪛 GitHub Actions: Kotlin Multiplatform CI
[warning] 6-6: Unexpected blank line(s) before }
14-14: 매개변수 리스트에 트레일링 콤마 추가 권장
코틀린 스타일 가이드에 따라 인자 목록이 여러 줄에 걸쳐 있을 때, 마지막 인자 뒤에 쉼표를 붙여주면 변경 사항 트래킹에 유리합니다.13| fun getDefaultState() = NoticeDetailsState( -14| s = "" +14| s = "", 15| )🧰 Tools
🪛 GitHub Actions: Kotlin Multiplatform CI
[warning] 14-14: Missing trailing comma before )
feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/ui/NoticeDetailsScreen.kt (2)
22-22: 사용되지 않는 import 제거 권장
파이프라인 경고에 따르면import team.aliens.dms.kmp.feature.notice.viewmodel.NoticesState가 사용되지 않고 있습니다.- import team.aliens.dms.kmp.feature.notice.viewmodel.NoticesState🧰 Tools
🪛 GitHub Actions: Kotlin Multiplatform CI
[warning] 22-22: Unused import
49-49: 불필요한 공백 라인 존재
함수 종료 전, 불필요한 공백 라인이 있다는 파이프라인 경고가 보고되었습니다. 가독성을 위해 공백 라인을 정리해 주세요.🧰 Tools
🪛 GitHub Actions: Kotlin Multiplatform CI
[warning] 49-49: Unexpected blank line(s) before }
feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/ui/NoticesScreen.kt (1)
98-131: 'NoticeItem' 클릭 이벤트 구현
onNoticeDetailsClick(1)형태로 하드코딩된 ID가 테스트 용도로 사용된 것으로 보입니다. 이후 실제 공지 ID에 맞춰 동적으로 변환해야 할 수 있습니다.- 파이프라인 경고(105-105)대로
{ onNoticeDetailsClick(1) }처럼 중괄호와 함수 사이 띄어쓰기를 권장합니다.- .clickable(onClick = {onNoticeDetailsClick(1)}) + .clickable(onClick = { onNoticeDetailsClick(1) })🧰 Tools
🪛 GitHub Actions: Kotlin Multiplatform CI
[warning] 105-105: Missing spacing after { and before }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
composeApp/src/commonMain/kotlin/team/aliens/dms/kmp/DmsNavigator.kt(2 hunks)composeApp/src/commonMain/kotlin/team/aliens/dms/kmp/di/FeatureModule.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)composeApp/src/commonMain/kotlin/team/aliens/dms/kmp/ui/BottomMenu.kt(2 hunks)core/common/src/commonMain/kotlin/team/aliens/dms/kmp/core/common/ui/PaddingDefaults.kt(1 hunks)feature/notice/build.gradle.kts(1 hunks)feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/di/NoticeModule.kt(1 hunks)feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/navigation/NoticeDetailsNavigation.kt(1 hunks)feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/navigation/NoticeNavigation.kt(0 hunks)feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/navigation/NoticesNavigation.kt(1 hunks)feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/ui/NoticeDetailsScreen.kt(1 hunks)feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/ui/NoticeScreen.kt(0 hunks)feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/ui/NoticesScreen.kt(1 hunks)feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/viewmodel/NoticeDetailsViewModel.kt(1 hunks)feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/viewmodel/NoticesViewModel.kt(1 hunks)
💤 Files with no reviewable changes (2)
- feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/navigation/NoticeNavigation.kt
- feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/ui/NoticeScreen.kt
🧰 Additional context used
🪛 detekt (1.23.7)
feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/viewmodel/NoticeDetailsViewModel.kt
[warning] 5-7: The class or object NoticeDetailsViewModel is empty.
(detekt.empty-blocks.EmptyClassBlock)
🪛 GitHub Actions: Kotlin Multiplatform CI
feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/viewmodel/NoticeDetailsViewModel.kt
[warning] 5-5: Missing spacing after comma
[warning] 5-5: Unnecessary empty block ({})
[warning] 6-6: Unexpected blank line(s) before }
[warning] 14-14: Missing trailing comma before )
feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/ui/NoticeDetailsScreen.kt
[warning] 22-22: Unused import
[warning] 49-49: Unexpected blank line(s) before }
feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/ui/NoticesScreen.kt
[warning] 105-105: Missing spacing after { and before }
🔇 Additional comments (27)
composeApp/src/commonMain/kotlin/team/aliens/dms/kmp/ui/BottomMenu.kt (2)
8-8: 새로운 상수 임포트 반영을 환영합니다.
이전보다 명칭이 더 명확하여 코드 가독성이 개선된 것으로 보입니다.
31-31: 네이밍 변경에 따른 라우트 수정이 적절해 보입니다.
프로젝트 전반의 네이밍 규칙 및 내비게이션 구조와 일관성을 유지하고 있습니다.feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/viewmodel/NoticesViewModel.kt (1)
13-21: 기본 상태 초기화 로직 점검
NoticesState내부isRecent기본값을true로 지정하는 것은 직관적이며, 동작 시 예기치 않은 상태를 막는 데 유용해 보입니다. 특히 공지 목록이 최신순으로 우선 정렬되는 것을 의도했다면 적절한 설정입니다.feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/di/NoticeModule.kt (3)
1-2: 패키지 구성 적절함
해당 파일이team.aliens.dms.kmp.feature.notice.di패키지에 배치되어 있어, 공지 기능 관련 의존성 주입 클래스임을 명확히 알 수 있습니다.
3-7: 의존성 Import 구조 적절
Koin 및 ViewModel 관련 클래스가 잘 임포트되어 있으며, 불필요한 의존성 가져오기가 없어 간결하게 유지됩니다.
8-11: Koin 모듈 설정 적절
noticeModule내부에서viewModelOf(...)함수를 통해NoticesViewModel과NoticeDetailsViewModel을 의존성 주입으로 처리하는 방식이 적절합니다. Koin DSL을 간결하게 활용했다는 점에서 유지보수에도 용이합니다.composeApp/src/commonMain/kotlin/team/aliens/dms/kmp/root/RootNavigation.kt (2)
9-11: 주입 함수 시그니처 변경
UI 상의 공지 상세 클릭 이벤트를 처리하기 위한 람다 파라미터가 추가되었습니다. 이로써 관련 이벤트 핸들링 로직을 상위에서 주입할 수 있어 확장성과 재사용성이 높아집니다.
13-13: 콜백 함수 전달 구조 적절
Root컴포저블에onNoticeDetailClick콜백을 동일한 이름으로 전달하여, UI와 이벤트 흐름이 간결하게 연결되고 명시성이 잘 드러납니다.feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/navigation/NoticesNavigation.kt (3)
1-2: 파일 및 패키지 구조 적절
notice기능 내에서 별도의navigation패키지를 분리하여, 라우팅 관련 로직을 한 곳에 모아둔 구성 방식이 깔끔합니다.
3-7: 간단한 import와 상수 정의
NavGraphBuilder와composable함수를 가져오고,NAVIGATION_NOTICES상수를 분리하여 선언함으로써 유지보수와 재활용 시 가독성이 높아졌습니다.
9-17: Notices 화면 내비게이션 설정
notices확장 함수를 통해composable(NAVIGATION_NOTICES)구성 요소를 선언하고,Notices컴포저블에onNoticeDetailsClick을 인자로 전달합니다. 이를 통해 구현부와 내비게이션이 명료하게 분리되어 있어 확장 기능 추가 시 관리가 수월해집니다.composeApp/src/commonMain/kotlin/team/aliens/dms/kmp/di/FeatureModule.kt (2)
5-5: 새로운 모듈 import
noticeModule을 가져오는 import 문이 추가되어, 공지 기능과 관련된 의존성을 손쉽게 등록할 수 있는 기반을 마련했습니다.
16-16: 모듈 리스트 확장
기존에 등록된splashModule,signInModule,signUpModule,homeModule에 이어noticeModule을 포함하여, 공지 기능 관련 뷰모델 주입이 이루어질 수 있도록 확장되었습니다.composeApp/src/commonMain/kotlin/team/aliens/dms/kmp/navigation/main/MainNavigation.kt (1)
19-20: 내비게이션 경로 정상 동작 확인
root(onNoticeDetailsClick = navigator::navigateToNoticeDetails)와noticeDetails()로 정상적인 이동이 가능한지, 특히noticeDetails가 예상된 인자를 제대로 받아 UI가 렌더링되는지 테스트를 권장드립니다.새로 추가된 경로 관련 로직이 다른 화면들과 충돌하지 않는지 확인하기 위해 아래 스크립트를 고려해 주세요:
✅ Verification successful
내비게이션 구현이 정상적으로 확인되었습니다
noticeId파라미터가 올바르게 전달되고 있습니다- 라우트 패턴과 네비게이션 로직이 일관되게 구현되어 있습니다
- 다른 화면들과의 충돌 가능성이 없습니다
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # noticeDetails 라우트 검색 rg -A 5 "noticeDetails"Length of output: 2344
Script:
#!/bin/bash # Check root navigation and navigator implementations rg -A 5 "class.*Navigator" rg -A 10 "fun.*navigateToNoticeDetails" ast-grep --pattern 'fun root($$$) { $$$ }'Length of output: 2150
core/common/src/commonMain/kotlin/team/aliens/dms/kmp/core/common/ui/PaddingDefaults.kt (1)
18-18: 수직 패딩 함수 추가 적절성
verticalPadding함수를 통해 세로 방향의 패딩을 간편하게 적용할 수 있어, UI 구성 시 가독성과 재사용성이 향상됩니다. 현재로서는 추가적인 문제점이 없어 보입니다.feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/navigation/NoticeDetailsNavigation.kt (1)
25-27: 라우트 문자열 직렬화 확인
매개변수로 전달되는noticeId가 문자열로 정상 변환되어 URL 충돌이나 잘못된 경로가 발생하지 않는지 최종적으로 확인해 주세요.아래 스크립트를 통해,
navigateToNoticeDetails호출을 검색해 인자와 형 변환을 전반적으로 확인할 수 있습니다:✅ Verification successful
라우트 문자열 직렬화 안전 확인 완료
구현이 완벽하게 안전합니다:
NavType.LongType으로 타입 안전성이 보장됨- Kotlin의 문자열 보간법으로
Long타입이 안전하게 변환됨- 경로 패턴이 일관되게 정의되어 있음
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash rg -A 3 "navigateToNoticeDetails"Length of output: 1998
Script:
#!/bin/bash # Check the route constant definition and route pattern matching rg -A 3 "NAVIGATION_NOTICE_DETAILS"Length of output: 1493
composeApp/src/commonMain/kotlin/team/aliens/dms/kmp/root/RootScreen.kt (4)
16-16: 공지 관련 네비게이션 import 추가 확인
notices네비게이션 추가로 인해 import가 새로 생겼습니다. 정상적으로 사용되고 있어 보이며 불필요한 의존성은 없어 보입니다.
20-25: 'Root' 함수에 onNoticeDetailClick 매개변수 추가
사용자 이벤트(공지 상세 진입) 처리를 위해 콜백을 주입하는 방식은 확장성과 유지보수 측면에서 좋아 보입니다.
29-31: 'RootScreen' 함수에 onNoticeDetailClick 매개변수 추가
상위 컴포저블의 매개변수를 그대로 내려받아 사용하는 구조가 명확하며, 공지 상세 진입 경로를 일관성 있게 유지합니다.
46-46: 공지 네비게이션에 콜백 연결 확인
notices(onNoticeDetailsClick = onNoticeDetailClick)로 정확하게 연결되어 있습니다. 네이밍도 직관적이라 유지보수에 유리합니다.composeApp/src/commonMain/kotlin/team/aliens/dms/kmp/DmsNavigator.kt (2)
7-7: 새로운 네비게이션 함수 import
navigateToNoticeDetails사용을 위한 import 문이 정상적으로 추가되었습니다.
69-71: 공지 상세 화면 이동 함수 추가
navigateToNoticeDetails(noticeId: Long)함수가 간결하고 명료하게 작성되었습니다. 비즈니스 로직이 없이 단순 라우팅만 담당하므로 유지보수가 용이해 보입니다.feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/ui/NoticesScreen.kt (3)
30-42: 'Notices' 컴포저블에서 onNoticeDetailsClick 콜백 주입 확인
DI를 통해NoticesViewModel을 주입받고 상태를 연동하며, 공지 상세 화면으로 이동하기 위한 콜백을 연결하는 흐름이 적절합니다.
44-80: 'NoticesScreen' 컴포저블 구조 검토
DmsTopAppBar사용으로 상단 바를 명시적으로 구현하였고, 정렬 순서 변경 버튼(DmsButton)을 통해state.isRecent값을 토글하는 로직이 직관적입니다.NoticeItems를 세부 컴포저블로 분리하여 가독성과 유지보수를 모두 높였습니다.
82-96: 'NoticeItems' 컴포저블에서 LazyColumn 활용
- 고정된 항목 5개를 간단히 표시하고 있으나, 추후 실제 데이터 목록 연동 시 간단히 교체 가능해 보입니다.
onNoticeDetailsClick을 리스트 항목 단위로 내려 주어 재사용성과 확장성을 고려한 구조입니다.feature/notice/build.gradle.kts (2)
46-48: Koin 종속성 추가가 적절해 보입니다.Koin을 사용하여 의존성 주입을 구현한 것이 좋은 선택입니다. ViewModel 주입을 위한 필수 종속성들이 모두 포함되어 있습니다.
50-51: 프로젝트 모듈 종속성이 적절히 구성되었습니다.공지사항 UI 구현을 위해 필요한
designSystem과common모듈을 적절히 참조하고 있습니다.다음 스크립트를 실행하여 프로젝트 모듈의 존재 여부를 확인하겠습니다:
✅ Verification successful
모듈 종속성 구성이 올바르게 되어있음을 확인했습니다.
core/design-system과core/common모듈이 존재하며 다른 기능 모듈들과 동일한 패턴으로 올바르게 참조되고 있습니다.- 공지사항 UI 구현에 필요한 디자인 시스템 컴포넌트들과 공통 유틸리티를 적절히 활용할 수 있습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of referenced project modules # Check for core modules fd -t d "core/(designSystem|common)" -d 2Length of output: 43
Script:
#!/bin/bash # Find all Gradle build files to understand project structure echo "=== Gradle Build Files ===" fd -e "gradle.kts" --type f echo -e "\n=== Searching for design system and common module references ===" rg -i "designsystem|common" -g "*.{gradle.kts,kt,xml}"Length of output: 38983
| fun NavGraphBuilder.noticeDetails() { | ||
| composable( | ||
| route = "$NAVIGATION_NOTICE_DETAILS/{$NOTICE_ID}", | ||
| arguments = listOf( | ||
| navArgument(NOTICE_ID) { type = NavType.LongType }, | ||
| ), | ||
| ) { | ||
| val noticeId = it.arguments?.getLong(NOTICE_ID) ?: 0L | ||
| NoticeDetails(noticeId = noticeId) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
noticeId 인자 처리 방식 검토
noticeId가 없는 경우 0L로 대체하고 있으므로, 실제로 0L이 유효 ID가 아니라면 예외 처리를 고려하는 것이 좋습니다. 알맞은 예외 처리나 로딩 실패 UI 표시 등을 추가해 사용자의 혼란을 줄일 수 있습니다.
필요하다면 0L이 반환되는 상황에 대비하여 적절한 예외 처리 방안을 제안하거나, 관련 로직을 개선하는 작업을 도와드릴 수 있습니다. 필요한 경우 알려 주세요.
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/ui/NoticeDetailsScreen.kt(1 hunks)feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/ui/NoticesScreen.kt(1 hunks)feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/viewmodel/NoticeDetailsViewModel.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/viewmodel/NoticeDetailsViewModel.kt
- feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/ui/NoticesScreen.kt
🧰 Additional context used
🪛 GitHub Actions: Kotlin Multiplatform CI
feature/notice/src/commonMain/kotlin/team/aliens/dms/kmp/feature/notice/ui/NoticeDetailsScreen.kt
[error] 48-48: Unexpected blank line(s) before '}' (standard:no-blank-line-before-rbrace)
| ) | ||
| Notice() | ||
| } | ||
|
|
There was a problem hiding this comment.
파이프라인 오류를 수정해주세요
48번 줄의 닫는 중괄호 앞에 있는 빈 줄을 제거해주세요.
Notice()
}
-
}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Actions: Kotlin Multiplatform CI
[error] 48-48: Unexpected blank line(s) before '}' (standard:no-blank-line-before-rbrace)
| @Composable | ||
| internal fun NoticeDetails( | ||
| noticeId: Long, | ||
| ) { | ||
| val viewModel: NoticeDetailsViewModel = koinInject() | ||
| val state by viewModel.state.collectAsState() | ||
|
|
||
| NoticeDetailsScreen(state = state) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
상태 처리 개선이 필요합니다
ViewModel에서 오는 상태 처리가 불완전합니다. 다음 사항들을 고려해주세요:
- 잘못된 noticeId에 대한 에러 처리
- 로딩 상태 표시
@Composable
internal fun NoticeDetails(
noticeId: Long,
) {
val viewModel: NoticeDetailsViewModel = koinInject()
val state by viewModel.state.collectAsState()
+ when {
+ state.isLoading -> LoadingIndicator()
+ state.error != null -> ErrorScreen(error = state.error)
+ else -> NoticeDetailsScreen(state = state)
+ }
- NoticeDetailsScreen(state = state)
}Committable suggestion skipped: line range outside the PR's diff.
| @Composable | ||
| private fun NoticeDetailsScreen( | ||
| state: NoticeDetailsState, | ||
| ) { | ||
| Column( | ||
| modifier = Modifier | ||
| .fillMaxSize() | ||
| .background(DmsTheme.colors.background), | ||
| ) { | ||
| DmsTopAppBar( | ||
| title = "안내", | ||
| onBackPressed = { }, | ||
| ) | ||
| Notice() | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
뒤로가기 처리와 상태 활용이 필요합니다
다음 문제점들이 발견되었습니다:
onBackPressed핸들러가 비어있습니다state매개변수가 화면에서 활용되지 않고 있습니다
@Composable
private fun NoticeDetailsScreen(
state: NoticeDetailsState,
+ onBackPressed: () -> Unit = {},
) {
Column(
modifier = Modifier
.fillMaxSize()
.background(DmsTheme.colors.background),
) {
DmsTopAppBar(
title = "안내",
- onBackPressed = { },
+ onBackPressed = onBackPressed,
)
- Notice()
+ Notice(
+ title = state.title,
+ date = state.date,
+ content = state.content,
+ )
}
}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Actions: Kotlin Multiplatform CI
[error] 48-48: Unexpected blank line(s) before '}' (standard:no-blank-line-before-rbrace)
| @Composable | ||
| private fun Notice( | ||
| modifier: Modifier = Modifier, | ||
| ) { | ||
| Column( | ||
| modifier = modifier | ||
| .fillMaxWidth() | ||
| .padding(24.dp), | ||
| verticalArrangement = Arrangement.spacedBy(12.dp), | ||
| ) { | ||
| DmsText( | ||
| text = "기숙사생들에 알립니다.", | ||
| style = DmsTypography.Header3, | ||
| color = DmsTheme.colors.surfaceBright, | ||
| ) | ||
| DmsText( | ||
| text = "22/21/22 12:1", | ||
| style = DmsTypography.Body1, | ||
| color = DmsTheme.colors.inverseSurface, | ||
| ) | ||
| HorizontalDivider( | ||
| modifier = Modifier.fillMaxWidth(), | ||
| thickness = 1.dp, | ||
| color = DmsTheme.colors.surface, | ||
| ) | ||
| DmsText( | ||
| text = "sdasdasdasd", | ||
| style = DmsTypography.Body2, | ||
| color = DmsTheme.colors.onTertiaryContainer, | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
문자열 리소스화 및 날짜 형식 수정이 필요합니다
다음과 같은 개선이 필요합니다:
- 하드코딩된 문자열을 리소스로 분리
- 잘못된 날짜 형식 수정 ("22/21/22 12:1")
- 동적 컨텐츠를 위한 매개변수 추가
@Composable
private fun Notice(
modifier: Modifier = Modifier,
+ title: String,
+ date: String,
+ content: String,
) {
Column(
modifier = modifier
.fillMaxWidth()
.padding(24.dp),
verticalArrangement = Arrangement.spacedBy(12.dp),
) {
DmsText(
- text = "기숙사생들에 알립니다.",
+ text = title,
style = DmsTypography.Header3,
color = DmsTheme.colors.surfaceBright,
)
DmsText(
- text = "22/21/22 12:1",
+ text = date,
style = DmsTypography.Body1,
color = DmsTheme.colors.inverseSurface,
)
HorizontalDivider(
modifier = Modifier.fillMaxWidth(),
thickness = 1.dp,
color = DmsTheme.colors.surface,
)
DmsText(
- text = "sdasdasdasd",
+ text = content,
style = DmsTypography.Body2,
color = DmsTheme.colors.onTertiaryContainer,
)
}
}📝 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 | |
| private fun Notice( | |
| modifier: Modifier = Modifier, | |
| ) { | |
| Column( | |
| modifier = modifier | |
| .fillMaxWidth() | |
| .padding(24.dp), | |
| verticalArrangement = Arrangement.spacedBy(12.dp), | |
| ) { | |
| DmsText( | |
| text = "기숙사생들에 알립니다.", | |
| style = DmsTypography.Header3, | |
| color = DmsTheme.colors.surfaceBright, | |
| ) | |
| DmsText( | |
| text = "22/21/22 12:1", | |
| style = DmsTypography.Body1, | |
| color = DmsTheme.colors.inverseSurface, | |
| ) | |
| HorizontalDivider( | |
| modifier = Modifier.fillMaxWidth(), | |
| thickness = 1.dp, | |
| color = DmsTheme.colors.surface, | |
| ) | |
| DmsText( | |
| text = "sdasdasdasd", | |
| style = DmsTypography.Body2, | |
| color = DmsTheme.colors.onTertiaryContainer, | |
| ) | |
| } | |
| } | |
| @Composable | |
| private fun Notice( | |
| modifier: Modifier = Modifier, | |
| title: String, | |
| date: String, | |
| content: String, | |
| ) { | |
| Column( | |
| modifier = modifier | |
| .fillMaxWidth() | |
| .padding(24.dp), | |
| verticalArrangement = Arrangement.spacedBy(12.dp), | |
| ) { | |
| DmsText( | |
| text = title, | |
| style = DmsTypography.Header3, | |
| color = DmsTheme.colors.surfaceBright, | |
| ) | |
| DmsText( | |
| text = date, | |
| style = DmsTypography.Body1, | |
| color = DmsTheme.colors.inverseSurface, | |
| ) | |
| HorizontalDivider( | |
| modifier = Modifier.fillMaxWidth(), | |
| thickness = 1.dp, | |
| color = DmsTheme.colors.surface, | |
| ) | |
| DmsText( | |
| text = content, | |
| style = DmsTypography.Body2, | |
| color = DmsTheme.colors.onTertiaryContainer, | |
| ) | |
| } | |
| } |
개요
작업사항
추가 로 할 말
Summary by CodeRabbit
새로운 기능
의존성 관리
탐색 개선