-
Notifications
You must be signed in to change notification settings - Fork 1
[REFACTOR] : 뷰잇 Compose 마이그레이션 및 성능 벤치마크 도입 #171
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
…into feature/viewit-compose
…into feature/viewit-compose
Walkthrough이번 변경에서는 "ViewIt" 기능의 Jetpack Compose 기반 UI를 새롭게 도입하고, 기존 View 기반 구현을 Compose로 전환하였습니다. 벤치마크 측정을 위한 별도 모듈과 테스트 프레임워크가 추가되었으며, 도메인, 디자인 시스템, UI 유틸리티 등 여러 코어 모듈에 새로운 함수와 컴포넌트가 대거 도입되었습니다. 또한, Paging, 이미지 로딩, 권한 타입 판별 등 다양한 기능이 Compose 기반으로 통합되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Fragment
participant ComposeUI
participant ViewModel
participant Repository
User->>Fragment: 앱 화면 진입
Fragment->>ComposeUI: ViewItRoute() 호출
ComposeUI->>ViewModel: 인텐트 처리 (예: 좋아요 클릭, 새로고침 등)
ViewModel->>Repository: 데이터 요청/변경 (Paging, 좋아요, 삭제 등)
Repository-->>ViewModel: 결과 반환
ViewModel-->>ComposeUI: State/PagingData/SideEffect 전달
ComposeUI-->>User: UI 업데이트 및 피드백
sequenceDiagram
participant BenchmarkTest
participant BenchmarkFramework
participant AppActivity
participant Device
BenchmarkTest->>BenchmarkFramework: runPerformanceTest() 호출
BenchmarkFramework->>Device: 앱 종료, 홈 이동, 대기
BenchmarkFramework->>AppActivity: 타겟 액티비티 실행
BenchmarkFramework->>Device: UI 조건 대기 (WaitCondition)
BenchmarkFramework->>AppActivity: 시나리오(클릭/스크롤 등) 수행
BenchmarkFramework->>BenchmarkTest: 결과/메트릭 반환
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
…e/WABLE-ANDROID into feature/viewit-compose # Conflicts: # benchmark/src/main/java/com/teamwable/benchmark/tests/ViewItBenchmarkTest.kt
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: 13
🧹 Nitpick comments (17)
feature/viewit/src/main/res/layout/fragment_view_it_compose.xml (1)
1-24: XML-내 ComposeView 사용은 과도기 대응으로 보여집니다장기적으로는
Fragment→@Composable전환 후 XML 없이setContent {}로 바로 Compose 트리 구성하는 편이 유지보수·성능 모두에 이점이 큽니다. 추후 전면 Compose 마이그레이션을 고려해 주세요.app/build.gradle.kts (2)
10-12: Task 이름 문자열 매칭으로 Google-Services 플러그인을 건너뛰는 방식은 취약합니다
gradle.startParameter.taskNames에 의존하면 IDE Build, AGP 내부 task 이름 변동,:app:assembleBenchmarkRelease등 복합 태스크에서 오동작할 수 있습니다.
추천:plugins { id("com.google.gms.google-services") apply false } androidComponents { beforeVariants { variant -> if (variant.buildType == "benchmark") { variant.enablePlugin("com.google.gms.google-services", false) } } }AGP 8+ 의 플러그인 적용 제어 API를 사용하면 변형별(onVariant) 정확 제어가 가능합니다.
61-69:benchmark빌드타입에 Proguard / R8 설정과profileable병합 누락 여부 확인
initWith(release)로 난독화 옵션이 그대로 복사되었으나isMinifyEnabled속성이 release 쪽에서false로 돼 있어, 벤치마크 apk 가 디버그와 동일하게 최적화 없이 빌드됩니다.
실측 성능 비교라면 실제 릴리스와 동일한 R8 / Proguard 최적화 레벨을 맞추는 편이 신뢰도 높습니다.또한 앞서 지적한
<profileable>태그를 main manifest 에서 제거할 경우,benchmark소스세트(예:src/benchmark/AndroidManifest.xml) 에 추가하는 것을 잊지 마세요.feature/viewit/src/main/java/com/teamwable/viewit/ui/ViewItContract.kt (1)
5-12: Recomposition 최소화를 위해@Stable/@Immutable어노테이션 추가
ViewItActions는 Compose 트리 전역에서 반복 전달됩니다. 기본 인라인 람다({})는 각 recomposition 때마다 새로운 인스턴스를 만들며, 이는 unnecessary recomposition 을 유발할 수 있습니다.-import com.teamwable.model.viewit.ViewIt +import androidx.compose.runtime.Stable +import com.teamwable.model.viewit.ViewIt + +@Stable data class ViewItActions(또는
@Immutable을 사용해도 무방합니다.benchmark/src/main/java/com/teamwable/benchmark/targets/BenchmarkTargets.kt (1)
7-24: 중복 코드 감소를 위한 팩토리 메서드 제안두 타겟의 필드 값만 일부 다르므로 객체 두 개를 하드코딩하기보다 팩토리 함수를 통해 생성하면 유지보수성이 향상됩니다.
feature/viewit/src/main/java/com/teamwable/viewit/component/ProfileImage.kt (2)
21-29: 로컬 프로필 이미지에도contentScale지정 필요원형 클립 이후 기본 scale 방식이 달라져 이미지가 짤릴 수 있습니다.
Image에도contentScale = ContentScale.Crop을 명시해 일관성을 유지하세요.
30-49:modifier재사용 시 다중clip/background체이닝 주의같은
modifier인스턴스를 Box와 GlideImage 모두에 전달하면서 추가로clip/background를 덧붙이면 의도치 않은 두 번의clip호출이 발생합니다. placeholder 용 Box에는Modifier.fillMaxSize()등 별도 인스턴스를 사용하는 편이 안전합니다.benchmark/build.gradle.kts (1)
10-17: JDK 타깃을 17로 상향하는 것을 고려해 보세요.Compose 1.6+·AGP 8.x 조합에서는
jvmTarget = "17"권장이 일반화되었습니다.
향후 Compose Compiler 경고 제거 및 최신 언어 기능(Sealed 인터페이스 등) 활용을 위해 빌드 타깃을 17로 올리는 것이 유리합니다.- compileOptions { - sourceCompatibility = JavaVersion.VERSION_1_8 - targetCompatibility = JavaVersion.VERSION_1_8 - } - - kotlinOptions { - jvmTarget = "1.8" - } + compileOptions { + sourceCompatibility = JavaVersion.VERSION_17 + targetCompatibility = JavaVersion.VERSION_17 + } + + kotlinOptions { + jvmTarget = "17" + }feature/viewit/src/main/java/com/teamwable/viewit/benchmark/BenchmarkXmlEntryActivity.kt (1)
27-37: 빈 콜백 블록 제거 및 재사용 가능한 객체로 치환 권장
detekt.empty-blocks경고가 지적하듯, 빈 메서드 블록이 네 곳 존재합니다.
벤치마크 용도로 실제 로직이 필요 없다면 재사용 가능한NoOpViewItClickListener(또는object : ViewItClickListener { ... }싱글톤)로 치환해 코드·DEX 크기를 줄이고 가독성을 높일 수 있습니다.- val viewitAdapter = ViewItAdapter(object : ViewItClickListener { - override fun onItemClick(link: String) {} - override fun onLikeBtnClick(viewHolder: ViewItViewHolder, id: Long, isLiked: Boolean) {} - override fun onPostAuthorProfileClick(id: Long) {} - override fun onKebabBtnClick(viewIt: ViewIt) {} - }) + val viewitAdapter = ViewItAdapter(NoOpViewItClickListener) + +object NoOpViewItClickListener : ViewItClickListener { + override fun onItemClick(link: String) = Unit + override fun onLikeBtnClick(viewHolder: ViewItViewHolder, id: Long, isLiked: Boolean) = Unit + override fun onPostAuthorProfileClick(id: Long) = Unit + override fun onKebabBtnClick(viewIt: ViewIt) = Unit +}core/designsystem/src/main/java/com/teamwable/designsystem/component/paging/WablePagingSpinner.kt (1)
82-88: 하드코딩된 흰색 대신 테마 색상 사용 권장
containerColor = Color.White는 다크모드 시 시각적 이질감을 유발할 수 있습니다.
MaterialTheme.colorScheme.background(또는surface) 로 교체하면 다크/라이트 모드 모두 자연스럽게 대응됩니다.- containerColor = Color.White, + containerColor = MaterialTheme.colorScheme.background,core/ui/src/main/java/com/teamwable/ui/extensions/FlowExt.kt (1)
23-30:awaitRefreshComplete를 확장 함수로 노출하면 사용성이 향상됩니다현재는 매 호출마다
awaitRefreshComplete(data)형태로 인자를 전달해야 합니다.
확장 함수로 변경하면pagingItems.awaitRefreshComplete()로 자연스럽게 읽히며 IDE 자동 완성이 용이합니다.-suspend fun awaitRefreshComplete(data: LazyPagingItems<*>) { - snapshotFlow { data.loadState.refresh } +suspend fun LazyPagingItems<*>.awaitRefreshComplete() { + snapshotFlow { loadState.refresh }core/designsystem/src/main/java/com/teamwable/designsystem/component/button/WableFloatingButton.kt (1)
117-146: 접근성 속성 및 시각적 피드백 부재
noRippleClickable을 사용해 리플 효과와Role.Button세맨틱이 사라졌습니다.
스크린리더·터치 피드백을 위해Modifier.clickable(indication = null, role = Role.Button, ...)등으로 세맨틱을 명시하거나IconButton에 직접onClick을 위임하는 방식을 고려해 주세요.feature/viewit/src/main/java/com/teamwable/viewit/ui/ViewItIntent.kt (1)
21-22:Triple대신 명확한 데이터 클래스로 교체 권장
BanViewIt에Triple<Long, String, Long>을 사용하면 필드 의미가 코드에서 즉시 드러나지 않습니다.
전용 데이터 클래스를 정의해 가독성과 타입 안정성을 높여 주세요.feature/viewit/src/main/java/com/teamwable/viewit/component/ViewitItem.kt (3)
51-60: 프로필 영역 클릭 처리 일관성 부족
ProfileImage와 닉네임Text에 각각 클릭 리스너를 중복으로 넣으면 터치 목표가 분리돼 UX가 혼란스러울 수 있습니다.
한 부모 컨테이너에 단일 클릭 리스너를 두어 전체 프로필 영역을 커버하도록 개선해 보세요.
126-147: GlideImage 실패·로딩 UI 누락 가능성
loading·failure슬롯에서contentDescription이 없어 스크린리더가 이미지 상태를 알 수 없습니다.
Image에 명시적contentDescription을 제공하거나null이유를 주석으로 설명해 주세요.
200-213: 좋아요 아이콘에 접근성 세맨틱 미부여
noRippleDebounceClickable사용 시 버튼 역할(Role.Button) 세맨틱이 빠집니다.
Modifier.clickable(role = Role.Button, indication = null, ... )형태로 보강해 시각적·보조기기 피드백을 확보해 주세요.feature/viewit/src/main/java/com/teamwable/viewit/viewit/ViewItViewModel.kt (1)
40-58: combine 블록 재계산 비용 고려
combine안에서PagingData를 매번filter → map으로 변환하므로removed/ban/like-Flow가 갱신될 때마다 기존 페이징 데이터가 반복 변환됩니다. 데이터량이 많으면 불필요한 비용이 커질 수 있으므로
pagingData.cachedIn뒤가 아닌 앞에서 변환을 적용하거나transformablePageAPI를 사용해 항목 단위 변경만 반영하는 방법을 검토해 보세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
app/build.gradle.kts(2 hunks)app/src/main/AndroidManifest.xml(3 hunks)benchmark/.gitignore(1 hunks)benchmark/build.gradle.kts(1 hunks)benchmark/src/main/java/com/teamwable/benchmark/BenchmarkFramework.kt(1 hunks)benchmark/src/main/java/com/teamwable/benchmark/targets/BenchmarkTargets.kt(1 hunks)benchmark/src/main/java/com/teamwable/benchmark/tests/ViewItBenchmarkTest.kt(1 hunks)build.gradle.kts(1 hunks)core/designsystem/src/main/java/com/teamwable/designsystem/component/button/WableFloatingButton.kt(1 hunks)core/designsystem/src/main/java/com/teamwable/designsystem/component/paging/WablePagingSpinner.kt(3 hunks)core/designsystem/src/main/java/com/teamwable/designsystem/component/screen/FeedBlindScreen.kt(1 hunks)core/designsystem/src/main/res/values/strings.xml(1 hunks)core/domain/src/main/java/com/teamwable/domain/usecase/GetAuthTypeUseCase.kt(1 hunks)core/model/src/main/java/com/teamwable/model/home/LikeState.kt(1 hunks)core/ui/build.gradle.kts(1 hunks)core/ui/src/main/java/com/teamwable/ui/extensions/FlowExt.kt(1 hunks)core/ui/src/main/java/com/teamwable/ui/extensions/FragmentExt.kt(1 hunks)core/ui/src/main/java/com/teamwable/ui/type/BottomSheetType.kt(1 hunks)feature/viewit/build.gradle.kts(2 hunks)feature/viewit/src/main/AndroidManifest.xml(1 hunks)feature/viewit/src/main/java/com/teamwable/viewit/benchmark/BenchmarkComposeEntryActivity.kt(1 hunks)feature/viewit/src/main/java/com/teamwable/viewit/benchmark/BenchmarkXmlEntryActivity.kt(1 hunks)feature/viewit/src/main/java/com/teamwable/viewit/component/ProfileImage.kt(1 hunks)feature/viewit/src/main/java/com/teamwable/viewit/component/ViewitItem.kt(1 hunks)feature/viewit/src/main/java/com/teamwable/viewit/ui/ViewItContract.kt(1 hunks)feature/viewit/src/main/java/com/teamwable/viewit/ui/ViewItIntent.kt(1 hunks)feature/viewit/src/main/java/com/teamwable/viewit/ui/ViewItScreen.kt(1 hunks)feature/viewit/src/main/java/com/teamwable/viewit/ui/ViewItSideEffect.kt(1 hunks)feature/viewit/src/main/java/com/teamwable/viewit/viewit/ViewItFragment.kt(1 hunks)feature/viewit/src/main/java/com/teamwable/viewit/viewit/ViewItViewModel.kt(1 hunks)feature/viewit/src/main/res/layout/fragment_view_it_compose.xml(1 hunks)gradle/libs.versions.toml(3 hunks)settings.gradle.kts(1 hunks)
🧰 Additional context used
🪛 detekt (1.23.8)
feature/viewit/src/main/java/com/teamwable/viewit/benchmark/BenchmarkXmlEntryActivity.kt
[warning] 29-29: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
[warning] 31-31: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
[warning] 33-33: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
[warning] 35-35: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR Checker
🔇 Additional comments (14)
benchmark/.gitignore (1)
1-1: 빌드 아티팩트 무시 확인
/build패턴이 Benchmark 모듈의 빌드 디렉토리를 올바르게 무시합니다.build.gradle.kts (1)
29-29: Android Test 플러그인 alias 추가
루트 스크립트에libs.plugins.android.test를apply false로 선언해 Benchmark 모듈에서 해당 플러그인을 선택 적용할 수 있게 되었습니다.settings.gradle.kts (1)
46-46: Benchmark 모듈 포함 설정
include(":benchmark")가 추가되어 프로젝트에 Benchmark 모듈이 정상 포함됩니다.gradle/libs.versions.toml (3)
30-31: 벤치마크 관련 버전 참조 추가
uiautomator = "2.2.0"및benchmarkMacroJunit4 = "1.4.0-beta02"가 버전 관리에 올바르게 선언되었습니다.
113-114: UI Automator 및 Macrobenchmark 라이브러리 alias 추가
androidx-uiautomator와androidx-benchmark-macro-junit4항목이libraries섹션에 정확히 매핑되었습니다.
204-204: Android Test 플러그인 alias 등록
플러그인 블록에android-test = { id = "com.android.test", version.ref = "agp" }가 추가되어, 루트 스크립트의 alias와 버전 테이블이 일치합니다.core/ui/build.gradle.kts (1)
16-16: Paging Compose 의존성 추가
Compose 기반 페이징 기능을 위해libs.androidx.paging.compose가 잘 추가되었습니다.core/ui/src/main/java/com/teamwable/ui/type/BottomSheetType.kt (1)
15-20: Enum 매핑 함수 구현 적절
when식으로 모든 케이스를 명시하여 컴파일러가 exhaustiveness 를 보장합니다. 간결하고 가독성도 좋아 👍feature/viewit/build.gradle.kts (1)
5-30: Compose 플러그인·라이브러리 버전 호환성 확인 필요
com.teamwable.wable.compose.feature플러그인에서 이미 compose-bom 을 가져온다면,androidx.paging.compose등 추가 라이브러리의 버전이 bom 과 일치하는지 확인해 주세요. 버전 불일치 시 빌드 경고 또는 런타임 크래시가 발생할 수 있습니다.core/domain/src/main/java/com/teamwable/domain/usecase/GetAuthTypeUseCase.kt (1)
11-12: NULL 가능성 검증 필요
getMemberId()또는getIsAdmin()Flow가null을 내보낼 경우 NPE 혹은toLong()NumberFormatException위험이 있습니다. 저장소 반환 타입을Result또는 nullable 로 감싸고 적절한 fallback 을 마련했는지 확인해 주세요.feature/viewit/src/main/java/com/teamwable/viewit/benchmark/BenchmarkComposeEntryActivity.kt (1)
30-49: 모의 데이터 속성 타입 불일치 가능성 확인
likedNumber가String으로 설정돼 있습니다. 실제 모델이Int또는Long일 경우 타입 변환 시 예외가 발생하므로 확인 바랍니다.benchmark/src/main/java/com/teamwable/benchmark/tests/ViewItBenchmarkTest.kt (1)
21-24: 반복 횟수 3회는 통계적 신뢰도가 낮을 수 있습니다매크로벤치마크 결과의 표준편차를 낮추려면 최소 5~10회 측정을 권장합니다. 시간 제약이 없다면
iterations = 7수준으로 늘려 보세요.feature/viewit/src/main/java/com/teamwable/viewit/ui/ViewItSideEffect.kt (1)
8-29:data object키워드는 Kotlin 1.9 이상에서만 지원됩니다.현재 빌드 환경이 1.9 미만이라면 컴파일 오류가 발생하므로 Gradle Kotlin 버전을 확인해 주세요.
feature/viewit/src/main/java/com/teamwable/viewit/ui/ViewItScreen.kt (1)
64-83:animateScrollToItem(0, -100)의 음수 오프셋 확인 필요음수 offset은 Compose 버전에 따라 무시되거나 예외를 유발할 수 있습니다.
테스트 기기에서 정상 동작을 확인하고, 필요한 경우0이상의 offset 또는snapToItemIndex패턴으로 변경해 주세요.
core/designsystem/src/main/java/com/teamwable/designsystem/component/screen/FeedBlindScreen.kt
Show resolved
Hide resolved
benchmark/src/main/java/com/teamwable/benchmark/targets/BenchmarkTargets.kt
Outdated
Show resolved
Hide resolved
feature/viewit/src/main/java/com/teamwable/viewit/benchmark/BenchmarkComposeEntryActivity.kt
Show resolved
Hide resolved
feature/viewit/src/main/java/com/teamwable/viewit/viewit/ViewItFragment.kt
Outdated
Show resolved
Hide resolved
feature/viewit/src/main/java/com/teamwable/viewit/viewit/ViewItFragment.kt
Outdated
Show resolved
Hide resolved
feature/viewit/src/main/java/com/teamwable/viewit/viewit/ViewItFragment.kt
Outdated
Show resolved
Hide resolved
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 (2)
feature/viewit/src/main/java/com/teamwable/viewit/ui/ViewItScreen.kt (2)
133-142:painterResource를remember로 감싸지 않아 재합성마다 디코딩 발생
painterResource는 매 합성 시 리소스 디코딩이 일어나므로, 스크롤 등으로WableFloatingButton이 자주 재합성되면 불필요한 비용이 증가합니다.- WableFloatingButton( - modifier = modifier.padding(20.dp), - onClick = actions.onClickPosting, - icon = painterResource(id = com.teamwable.common.R.drawable.ic_home_posting), + val postingIcon = remember { painterResource(id = com.teamwable.common.R.drawable.ic_home_posting) } + WableFloatingButton( + modifier = modifier.padding(20.dp), + onClick = actions.onClickPosting, + icon = postingIcon,
192-199: 스피너 아이템의contentType계산 불필요하게 Paging API 호출
itemContentType { … }는 인덱스별 콘텐츠 타입을 계산하는 Paging 확장입니다.
스피너는 상수 타입이므로 Paging의 헬퍼를 호출할 필요 없이 문자열 상수를 직접 넘기는 편이 명확하고 미세하게 빠릅니다.- contentType = viewIts.itemContentType { ContentType.Spinner.name }, + contentType = ContentType.Spinner.name,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
benchmark/src/main/java/com/teamwable/benchmark/tests/ViewItBenchmarkTest.kt(1 hunks)core/ui/src/main/java/com/teamwable/ui/extensions/FlowExt.kt(1 hunks)feature/viewit/src/main/java/com/teamwable/viewit/ui/ViewItScreen.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- core/ui/src/main/java/com/teamwable/ui/extensions/FlowExt.kt
- benchmark/src/main/java/com/teamwable/benchmark/tests/ViewItBenchmarkTest.kt
🔇 Additional comments (1)
feature/viewit/src/main/java/com/teamwable/viewit/ui/ViewItScreen.kt (1)
178-185: 널 가드 로직은 좋지만, 아이템 손실 시 UX 고려 필요
viewIts[index] ?: return@items로 로드 실패 시 아이템·디바이더 모두 그리지 않도록 처리한 것은 안전합니다.
다만 빈 영역이 생기면 사용자 입장에선 “비어있음”으로 보일 수 있으니,
프로그래스 아이콘 추가 또는LoadState.Error케이스를 별도 아이템으로 노출하는 방안을 검토해 주세요.
feature/viewit/src/main/java/com/teamwable/viewit/ui/ViewItScreen.kt
Outdated
Show resolved
Hide resolved
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: 3
♻️ Duplicate comments (1)
benchmark/src/main/java/com/teamwable/benchmark/BenchmarkFramework.kt (1)
146-180: ScrollTest 파라미터 사용 불일치 해결 필요과거 리뷰에서 지적된 문제가 부분적으로만 해결되었습니다:
scrollCount파라미터는 이제 올바르게 사용됨 (161줄)scrollRatio파라미터가 두 번째 스와이프에서만 사용됨 (171-172줄)- 첫 번째 스와이프에서는 여전히 하드코딩된 값 사용 (162-164줄)
- 과거 코멘트에서 언급된
flingCount파라미터가 완전히 제거됨첫 번째 스와이프도
scrollRatio파라미터를 사용하도록 수정하고, 일관성을 위해 fling 동작을 별도로 구현하는 것을 고려해보세요:repeat(scrollCount) { device.swipe( - centerX, displayHeight * 2 / 3, - centerX, displayHeight / 3, + centerX, (displayHeight * (1 - scrollRatio / 2)).toInt(), + centerX, (displayHeight * (scrollRatio / 2)).toInt(), 50, ) device.waitForIdle() Thread.sleep(300) device.swipe( centerX, (displayHeight * (1 - scrollRatio / 2)).toInt(), centerX, (displayHeight * (scrollRatio / 2)).toInt(), 50, ) device.waitForIdle() Thread.sleep(300) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
.github/workflows/develop_PR_builder.yml(1 hunks)app/src/main/AndroidManifest.xml(3 hunks)benchmark/src/main/AndroidManifest.xml(1 hunks)benchmark/src/main/java/com/teamwable/benchmark/BenchmarkFramework.kt(1 hunks)benchmark/src/main/java/com/teamwable/benchmark/targets/BenchmarkTargets.kt(1 hunks)core/designsystem/src/main/res/values-ko/strings.xml(1 hunks)core/designsystem/src/main/res/values/strings.xml(1 hunks)core/ui/src/main/java/com/teamwable/ui/extensions/FragmentExt.kt(1 hunks)feature/viewit/src/main/AndroidManifest.xml(1 hunks)feature/viewit/src/main/java/com/teamwable/viewit/benchmark/BenchmarkComposeEntryActivity.kt(1 hunks)feature/viewit/src/main/java/com/teamwable/viewit/ui/ViewItContract.kt(1 hunks)feature/viewit/src/main/java/com/teamwable/viewit/ui/ViewItIntent.kt(1 hunks)feature/viewit/src/main/java/com/teamwable/viewit/ui/ViewItScreen.kt(1 hunks)feature/viewit/src/main/java/com/teamwable/viewit/viewit/ViewItFragment.kt(1 hunks)feature/viewit/src/main/java/com/teamwable/viewit/viewit/ViewItViewModel.kt(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- .github/workflows/develop_PR_builder.yml
- benchmark/src/main/AndroidManifest.xml
- core/designsystem/src/main/res/values-ko/strings.xml
🚧 Files skipped from review as they are similar to previous changes (10)
- app/src/main/AndroidManifest.xml
- core/designsystem/src/main/res/values/strings.xml
- benchmark/src/main/java/com/teamwable/benchmark/targets/BenchmarkTargets.kt
- core/ui/src/main/java/com/teamwable/ui/extensions/FragmentExt.kt
- feature/viewit/src/main/java/com/teamwable/viewit/ui/ViewItContract.kt
- feature/viewit/src/main/java/com/teamwable/viewit/viewit/ViewItFragment.kt
- feature/viewit/src/main/AndroidManifest.xml
- feature/viewit/src/main/java/com/teamwable/viewit/ui/ViewItIntent.kt
- feature/viewit/src/main/java/com/teamwable/viewit/benchmark/BenchmarkComposeEntryActivity.kt
- feature/viewit/src/main/java/com/teamwable/viewit/ui/ViewItScreen.kt
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR Checker
🔇 Additional comments (13)
feature/viewit/src/main/java/com/teamwable/viewit/viewit/ViewItViewModel.kt (9)
8-8: 새로운 아키텍처 패턴을 위한 임포트 추가 승인BaseViewModel, GetAuthTypeUseCase, UI 확장 함수들과 새로운 Intent/State/SideEffect 클래스들의 임포트가 적절하게 추가되었습니다. 새로운 MVI 아키텍처 패턴을 지원하기 위한 필요한 의존성들이 잘 정리되어 있습니다.
Also applies to: 11-11, 14-17, 20-22
34-37: BaseViewModel 확장 및 제네릭 타입 지정 승인GetAuthTypeUseCase 의존성 추가와 BaseViewModel<ViewItIntent, ViewItUiState, ViewItSideEffect> 확장이 올바르게 구현되었습니다. 초기 상태도 적절히 설정되어 있습니다.
41-59: 복합 플로우 로직의 반응형 상태 관리 승인여러 상태 플로우(제거된 항목, 금지된 항목, 좋아요 상태)를 combine하여 페이징 데이터를 동적으로 필터링하고 변환하는 로직이 잘 구현되었습니다. 로컬 상태 변경사항이 UI에 즉시 반영되는 반응형 패턴이 효과적으로 적용되었습니다.
61-74: Intent 기반 이벤트 처리 패턴 승인when 표현식을 사용한 Intent 디스패칭 로직이 명확하고 체계적으로 구현되었습니다. 각 Intent 타입에 대해 적절한 핸들러 메서드로 분기 처리되어 있어 코드 가독성과 유지보수성이 향상되었습니다.
84-90: GetAuthTypeUseCase 활용한 프로필 타입 판별 승인기존의 명시적인 authId 추적 대신 GetAuthTypeUseCase를 통한 동적 권한 확인 패턴이 잘 적용되었습니다. 사용자 타입에 따른 적절한 네비게이션 처리가 구현되어 있습니다.
92-102: 케밥 버튼 클릭 시 동적 바텀시트 처리 승인사용자 권한에 따라 다른 바텀시트 타입을 동적으로 결정하는 로직이 효과적으로 구현되었습니다. 상태 업데이트와 사이드 이펙트 발생이 올바르게 분리되어 있습니다.
108-115: CRUD 작업의 에러 핸들링 및 상태 관리 승인삭제, 신고, 차단 기능에서 성공/실패 시나리오가 적절히 처리되고 있습니다. 로컬 상태 업데이트와 사용자 피드백(스낵바)이 일관된 패턴으로 구현되어 있어 사용자 경험이 향상되었습니다.
Also applies to: 117-124, 126-136
138-141: 바텀시트 해제 로직 승인상태 초기화와 사이드 이펙트 발생을 통한 바텀시트 해제 로직이 깔끔하게 구현되었습니다. 펜딩 상태도 적절히 정리되어 있습니다.
143-151: ViewIt 포스팅 시 사용자 피드백 개선 승인포스팅 진행 중과 완료 시 각각 다른 스낵바를 표시하여 사용자에게 명확한 피드백을 제공하는 것이 우수합니다. 성공 시 자동 새로고침도 적절히 처리되어 있습니다.
benchmark/src/main/java/com/teamwable/benchmark/BenchmarkFramework.kt (4)
14-40: 잘 설계된 벤치마크 대상 추상화
BenchmarkTarget인터페이스가 명확하고 확장 가능한 구조로 설계되었습니다. 문서화도 훌륭하며 사용 예시까지 포함되어 있어 개발자가 쉽게 이해할 수 있습니다.
42-58: UI 동기화를 위한 견고한 대기 조건 시스템
WaitCondition과WaitType의 조합이 다양한 UI 상태를 효과적으로 처리할 수 있도록 설계되었습니다. 기본 타임아웃 값(5초)도 적절합니다.
95-111: BasicInteraction 시나리오 구현이 올바름파라미터들이 올바르게 사용되고 있으며, 적절한 대기 시간과 함께 사용자 상호작용을 시뮬레이션합니다.
183-198: 성능 메트릭 조합이 효과적으로 구성됨
@OptIn(ExperimentalMetricApi::class)주석과 함께 프레임 타이밍과 메모리 사용량을 적절히 조합했습니다. 메트릭별로 분리된 옵션도 유용합니다.
feature/viewit/src/main/java/com/teamwable/viewit/viewit/ViewItViewModel.kt
Show resolved
Hide resolved
chanubc
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.
1!ㄷㄷ 저보다 더 잘하시는거 같은데요? 고생많으셨습니다!!
| Image( | ||
| painter = painterResource(id = profileType.profileDrawableRes), |
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.
만약에 vector drawable이 가능하다면
imageVector = toImageVector(id = com.teamwable.common.R.drawable.ic_community_check),
이런식으로 활용하는 것이 성능향상(리컴포지션)에 유리합니다!
toImageVector는 만들어 놓은 확장함수에요
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.
해당 부분에서 성능향상에 유리한 구체적인 이유가 궁금합니다!!
관련 공식문서를 확인해보니 ImageVector에 직접 접근하는 방식(toImageVector 확장함수 활용)이 한번 로드하여 여러 곳에서 재사용 가능하기 때문에 메모리 효율성이 좋다고 나와있는데, 이 부분 맞나요?
혹시 다른 성능상 이점을 고려한건지 궁금합니다!
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.
https://tech.wonderwall.kr/articles/recomposition/
관련 레퍼런스입니다!
painter의 경우 stable이라 판단하지 않는다고 하네요!
반면 imageVector는 stable이라 판단하기에 리컴포지션 count가 올라가지 않는 것으로 보입니다.
근데 profileType이 svg면 vector로 가능인데 png면 painter 써야 될거에요
| data class ViewItActions( | ||
| val onClickProfile: (Long) -> Unit = {}, | ||
| val onClickKebab: (ViewIt) -> Unit = {}, | ||
| val onClickLink: (String) -> Unit = {}, | ||
| val onClickLike: (ViewIt) -> Unit = {}, | ||
| val onClickPosting: () -> Unit = {}, | ||
| val onRefresh: () -> Unit = {}, | ||
| ) |
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.
결국에는 사용자의 intent(action)을 하위에서 제어하느냐, 상위에서 제어하느냐의 문제인것 같습니다! 전 상위에서 제어하는게 흐름 보기 더 편하더라구요. 이건 개인의 자유일것 같아요!
혹시 해당 action을 따로 data class로 분리한 이유를 알 수 있을까요?
intenta만 하위로 넘기고자 하면
actions: (CommunityIntent) -> Unit = {},이런식으로 intent를 람다 파라미터로 넘기는 방법도 있습니다!!
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.
저도 이 부분에 대해 많이 고민했었는데요!
- Intent와 action을 분리한 이유는 Intent는 사용자의 의도고 Action은 그 의도를 처리하기 위한 구체적인 동작이라고 생각해서 명확히는 다른 개념이라고 판단했습니다. 아래처럼 하나의 Action이 여러 Intent를 발생시킬 수 있으니까요!
val actions = CommunityActions(
onClickLike = { postId ->
viewModel.onIntent(CommunityIntent.ClickLike(postId))
viewModel.onIntent(CommunityIntent.ShowLikeAnimation(postId))
viewModel.onIntent(CommunityIntent.TrackLikeEvent(postId))
}
)다만, 현재 프로젝트 구조에서 Intent와 Action이 거의 1:1 대응 되는 상황이고 분리가 과도하지 않나? 하는 생각도 들었습니다. 또 말씀하신 것처럼 Intent를 하위에서 제어하는 것이 더 간결하지 않을까?하는 고민도 들었어요. (Intent와 action을 상위에서 제어하는 게 흐름 보기엔 더 편하긴 한 것 같아요! SRP에 더 적합한 것 같기도 하구요)
이런 고민들을 했었는데, 이 부분에 대해서는 명확히 결론이 안난거 같아요 😢 읽어보시고 의견 공유해주세요!
- action을 따로 data class로 분리한 이유는 Intent를 상위에서 제어하게 되면서 콜백들이 아래 코드처럼 파라미터로 계속 전달되어 컴포저블이 길어지는 문제를 해결하고 싶었습니다!
//before
@Composable
fun ViewItScreen(
onClickProfile: (Long) -> Unit = {},
onClickLike: (Long) -> Unit = {},
onClickKebab: (Long) -> Unit = {},
onClickComment: (Long) -> Unit = {},
// ...
)
// after
@Composable
fun CommunityScreen(
actions: ViewItActions
)| suspend fun awaitRefreshComplete(data: LazyPagingItems<*>) = runCatching { | ||
| snapshotFlow { data.loadState.refresh } | ||
| .map { it is LoadState.NotLoading } | ||
| .distinctUntilChanged() | ||
| .filter { it } | ||
| .timeout(5.seconds) | ||
| .first() | ||
| } |
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.
👍
| @Composable | ||
| fun rememberViewItActions( | ||
| viewModel: ViewItViewModel = hiltViewModel(), | ||
| ): ViewItActions { | ||
| return remember(viewModel) { | ||
| ViewItActions( | ||
| onClickProfile = { viewModel.onIntent(ViewItIntent.ClickProfile(it)) }, | ||
| onClickKebab = { viewModel.onIntent(ViewItIntent.ClickKebabBtn(it)) }, | ||
| onClickLink = { viewModel.onIntent(ViewItIntent.ClickLink(it)) }, | ||
| onClickLike = { viewModel.onIntent(ViewItIntent.ClickLikeBtn(it)) }, | ||
| onClickPosting = { viewModel.onIntent(ViewItIntent.ClickPosting) }, | ||
| onRefresh = { viewModel.onIntent(ViewItIntent.PullToRefresh) }, | ||
| ) | ||
| } | ||
| } |
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.
ViewItIntent를 그대로 사용해도 될것 같다는 느낌입니다..!
| PullToRefreshBox( | ||
| isRefreshing = isRefreshing, | ||
| onRefresh = actions.onRefresh, | ||
| state = refreshState, | ||
| indicator = { |
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.
slot api를 활용하여 해당 컴포넌트를 더 재사용 가능하게 사용 할 수 있을것 같습니다!
이렇게 되면 내부에서 상태(isLoading, isRfreshing)을 state hoisting 할 수 있을것 같아서요!
@Composable
fun PagingStateHandler(
lazyPagingItems: LazyPagingItems<*>,
loadingContent: @Composable () -> Unit = {},
emptyContent: @Composable () -> Unit = {},
errorContent: @Composable (Throwable) -> Unit = {},
content: @Composable () -> Unit,
) {
when {
lazyPagingItems.loadState.refresh is LoadState.Loading -> loadingContent()
lazyPagingItems.loadState.refresh is LoadState.Error -> {
val error = (lazyPagingItems.loadState.refresh as LoadState.Error).error
errorContent(error)
}
lazyPagingItems.itemCount == 0 &&
lazyPagingItems.loadState.refresh is LoadState.NotLoading -> emptyContent()
else -> content()
}
}전 타 플젝에서 이렇게 활용했었는데 한번 참고해보아도 좋을것 같아요~
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.
너무 좋은 아이디어입니다 👍 반영할게요~~
…into feature/viewit-compose
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: 0
🧹 Nitpick comments (2)
core/designsystem/src/main/java/com/teamwable/designsystem/component/screen/PagingScreen.kt (2)
31-50: 로직이 올바르며 성능 최적화를 제안합니다.페이징 상태 처리 로직이 정확하게 구현되어 있습니다. 각 LoadState 케이스를 적절히 처리하고 있습니다.
다음과 같이
loadState.refresh를 캐싱하여 성능을 개선할 수 있습니다:+ val refreshState = lazyPagingItems.loadState.refresh when { - lazyPagingItems.loadState.refresh is LoadState.Loading -> loadingContent() - lazyPagingItems.loadState.refresh is LoadState.Error -> { - val error = (lazyPagingItems.loadState.refresh as LoadState.Error).error + refreshState is LoadState.Loading -> loadingContent() + refreshState is LoadState.Error -> { + val error = refreshState.error errorContent(error) } - lazyPagingItems.itemCount == 0 && lazyPagingItems.loadState.refresh is LoadState.NotLoading -> emptyContent() + lazyPagingItems.itemCount == 0 && refreshState is LoadState.NotLoading -> emptyContent() else -> content() }
71-120: 전체적인 구조가 우수하며 modifier 사용에 주의가 필요합니다.컴포넌트의 관심사 분리가 잘 되어 있고, 커스터마이징 옵션과 기본값 설정이 적절합니다.
PagingStateHandler와의 위임 패턴도 깔끔하게 구현되었습니다.라인 83에서
modifier파라미터를 정렬용도로 재사용하고 있는데, 이는 잠재적인 레이아웃 충돌을 일으킬 수 있습니다:WableCustomRefreshIndicator( state = state, isRefreshing = isRefreshing, - modifier = modifier.align(Alignment.TopCenter), + modifier = Modifier.align(Alignment.TopCenter), )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
core/designsystem/build.gradle.kts(1 hunks)core/designsystem/src/main/java/com/teamwable/designsystem/component/screen/PagingScreen.kt(1 hunks)core/ui/build.gradle.kts(1 hunks)feature/viewit/src/main/java/com/teamwable/viewit/ui/ViewItScreen.kt(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- core/designsystem/build.gradle.kts
🚧 Files skipped from review as they are similar to previous changes (2)
- core/ui/build.gradle.kts
- feature/viewit/src/main/java/com/teamwable/viewit/ui/ViewItScreen.kt
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: PR Checker
🔇 Additional comments (2)
core/designsystem/src/main/java/com/teamwable/designsystem/component/screen/PagingScreen.kt (2)
92-93: isRefreshing 로직 검토가 필요합니다.현재
isRefreshing로직이itemCount != 0 && loadState.refresh is LoadState.Loading으로 되어 있어, 초기 로딩 시에는 새로고침 인디케이터가 표시되지 않고 후속 새로고침에서만 표시됩니다.이것이 의도된 UX인지 확인이 필요합니다. 만약 초기 로딩에서도 pull-to-refresh 동작을 보여주려면 다음과 같이 수정할 수 있습니다:
- val isRefreshing = lazyPagingItems.itemCount != 0 && - lazyPagingItems.loadState.refresh is LoadState.Loading + val isRefreshing = lazyPagingItems.loadState.refresh is LoadState.Loading
1-14: 추가 검증이 필요합니다. 아래 스크립트를 실행해LoadingScreen정의 위치를 찾아주세요:#!/bin/bash # 1) 파일명으로 LoadingScreen.kt가 있는지 확인 fd LoadingScreen.kt # 2) 전체 코드베이스에서 LoadingScreen 함수 정의 검색 rg -n "@Composable.*fun LoadingScreen" --type kt # 3) 함수명이 정확히 일치하는지 범용 검색 rg -n "fun LoadingScreen" --type kt
✅ 𝗖𝗵𝗲𝗰𝗸-𝗟𝗶𝘀𝘁
📌 𝗜𝘀𝘀𝘂𝗲𝘀
📎𝗪𝗼𝗿𝗸 𝗗𝗲𝘀𝗰𝗿𝗶𝗽𝘁𝗶𝗼𝗻
ViewIt 화면 Compose 마이그레이션
성능 벤치마크 시스템 도입
📷 𝗦𝗰𝗿𝗲𝗲𝗻𝘀𝗵𝗼𝘁
뷰잇
KakaoTalk_20250618_220833148.mp4
성능 비교

🔗 더 자세히 보기
💬 𝗧𝗼 𝗥𝗲𝘃𝗶𝗲𝘄𝗲𝗿𝘀
MVI 패턴에서의 Actions 구조
: Actions 긴 콜백 체인으로 인해 보일러플레이트 코드가 발생한다고 생각하여 data class로 묶어봤습니다. 더 깔끔한 처리 방식이 있다면 제안해주세요!
아직 Compose 배울게 많습니다. 😢 컴포넌트 분리 및 상태 관리에 대해 개선해야 하는 부분이 있으면 제안해주세요!
Summary by CodeRabbit
새로운 기능
성능 벤치마킹
버그 수정 및 개선
문서 및 리소스
빌드/환경