Skip to content

Conversation

@juhwankim-dev
Copy link
Member

개요

힌트 화면을 Compose로 리팩터링한다.

변경사항

  • Compose 설정. 프로젝트에서 컴포즈를 사용한 적이 없으므로 의존성을 추가한다.
  • 디자인 컨벤션에 맞는 텍스트 스타일, 컬러 등 추가
  • Composable 컴포넌트 작성
  • Compose용 BaseFagment 생성
  • HintFragment가 Compose를 사용하도록 수정

@juhwankim-dev juhwankim-dev self-assigned this Dec 27, 2025
@github-actions
Copy link

🤖 AI Code Review (Gemini)

안녕하세요, NR-128 Pull Request에 대한 코드 리뷰를 시작하겠습니다.
힌트 화면을 XML에서 Compose로 마이그레이션하는 작업은 프로젝트의 기술 스택 전환에 있어 중요한 단계입니다. 전반적으로 Compose 도입 및 기존 로직 마이그레이션이 잘 이루어졌습니다. 특히 Compose 디자인 시스템을 위한 NRColor, NRTypo, NRToolbar, NRLoading 등의 컴포넌트를 새롭게 구축한 점은 매우 긍정적입니다.

다만, 프로젝트의 핵심 아키텍처 원칙 중 하나인 Orbit MVI 사용에 대한 변경사항이 있어 이에 대한 논의가 필요합니다.


🔴 Critical (치명적)

1. Orbit MVI 상태 관리 방식 변경

  • 파일: presentation/src/main/java/com/nextroom/nextroom/presentation/base/ComposeBaseViewModelFragment.kt, presentation/src/main/java/com/nextroom/nextroom/presentation/ui/hint/HintViewModel.kt
  • 문제: CLAUDE.md에 명시된 프로젝트의 상태 관리 방식은 Orbit MVI를 사용하는 BaseViewModel<STATE, SIDE_EFFECT>입니다. 하지만 이 PR에서 추가된 ComposeBaseViewModelFragmentNewBaseViewModel을 사용하며, HintViewModel은 Orbit의 ContainerHost를 상속하지 않고 MutableStateFlowMutableSharedFlow를 직접 사용하여 상태를 관리합니다. 이는 프로젝트의 핵심 아키텍처 원칙(Orbit MVI)과 상이하며, Compose 화면에 대한 새로운 상태 관리 패턴을 도입하는 것으로 보입니다.
  • 영향:
    • 아키텍처 일관성 저해: 기존 XML 기반 화면과 Compose 기반 화면 간에 상태 관리 방식이 달라져 아키텍처 일관성이 깨집니다.
    • 문서화 필요: 만약 Compose 화면에 대해 Orbit MVI 대신 StateFlow/SharedFlow를 직접 사용하는 것이 새로운 표준이라면, CLAUDE.md를 업데이트하여 이 변경사항을 명확히 문서화해야 합니다.
    • 학습 곡선: 개발자들이 두 가지 다른 상태 관리 패턴을 이해하고 적용해야 하므로 학습 곡선이 증가할 수 있습니다.
  • 조치 제안:
    • 옵션 1 (Orbit 유지): Compose에서도 Orbit MVI를 계속 사용하도록 HintViewModel을 리팩터링합니다. Orbit은 Compose와의 통합을 지원합니다.
    • 옵션 2 (새로운 표준 도입): Compose 화면에서는 StateFlow/SharedFlow를 직접 사용하는 것을 공식적인 상태 관리 표준으로 채택하고, CLAUDE.md를 업데이트합니다. 이 경우, NewBaseViewModel이 어떤 역할을 하는지, BaseViewModel과의 관계는 어떻게 되는지 명확히 정의해야 합니다.
    • 논의 필요: 이 변경이 의도된 것인지, 아니면 Compose 도입 과정에서 발생한 것인지 팀 내 논의가 필요합니다.

🟡 Warning (경고)

1. activity-compose 버전 명시의 불필요성

  • 파일: gradle/libs.versions.toml (L113)
  • 문제: androidx-activity-compose 라이브러리의 버전이 androidx-compose-bom에서 참조하는 버전과 동일함에도 불구하고 version = "1.8.2"로 명시되어 있습니다. compose-bom = "2024.02.00"activity-compose 1.8.2를 포함하고 있습니다.
  • 영향: 불필요한 버전 명시로 인해 BOM의 이점을 충분히 활용하지 못하며, 향후 BOM 업데이트 시 수동으로 버전을 맞춰야 하는 번거로움이 발생할 수 있습니다.
  • 조치 제안: version = "1.8.2" 부분을 제거하여 BOM이 버전을 관리하도록 합니다.
    --- a/gradle/libs.versions.toml
    +++ b/gradle/libs.versions.toml
    @@ -110,7 +110,7 @@
     androidx-compose-ui-graphics = { module = "androidx.compose.ui:ui-graphics" }
     androidx-compose-foundation = { module = "androidx.compose.foundation:foundation" }
     androidx-compose-material3 = { module = "androidx.compose.material3:material3" }
     androidx-compose-ui-tooling = { module = "androidx.compose.ui:ui-tooling" }
     androidx-compose-ui-tooling-preview = { module = "androidx.compose.ui:ui-tooling-preview" }
    -androidx-activity-compose = { module = "androidx.activity:activity-compose", version = "1.8.2" }
    +androidx-activity-compose = { module = "androidx.activity:activity-compose" }
     androidx-lifecycle-viewmodel-compose = { module = "androidx.lifecycle:lifecycle-viewmodel-compose", version.ref = "lifecycle" }
     androidx-lifecycle-runtime-compose = { module = "androidx.lifecycle:lifecycle-runtime-compose", version.ref = "lifecycle" }
     coil-compose = { module = "io.coil-kt:coil-compose", version.ref = "coil" }

2. NRTypo의 기본 색상 설정

  • 파일: presentation/src/main/java/com/nextroom/nextroom/presentation/common/compose/NRTypo.kt
  • 문제: NRTypo에 정의된 모든 TextStylecolor = NRColor.White가 기본으로 설정되어 있습니다.
  • 영향: 대부분의 텍스트가 흰색일 경우 편리하지만, 다른 색상의 텍스트를 사용해야 할 때마다 color 파라미터를 명시적으로 재정의해야 합니다. 일반적으로 TextStyle은 폰트 패밀리, 굵기, 크기, 줄 간격 등 타이포그래피 속성만 정의하고, 색상은 MaterialThemeLocalContentColor를 따르거나 사용처에서 지정하는 것이 더 유연합니다.
  • 조치 제안:
    • NRTypo에서 color 속성을 제거하고, MaterialThemeLocalContentColor를 따르도록 하거나, 각 Composable에서 color를 명시적으로 지정하는 것을 고려해볼 수 있습니다.
    • 만약 앱의 디자인 시스템이 대부분 흰색 텍스트를 사용하도록 의도된 것이라면 현재 방식도 허용 가능하지만, 향후 색상 변경이나 테마 확장에 제약이 될 수 있음을 인지해야 합니다.

💡 Suggestion (제안)

1. Compose 테마 (NRTheme) 도입 고려

  • 파일: presentation/src/main/java/com/nextroom/nextroom/presentation/common/compose/NRColor.kt, presentation/src/main/java/com/nextroom/nextroom/presentation/common/compose/NRTypo.kt
  • 내용: NRColorNRTypo를 통해 디자인 토큰을 잘 정의하셨습니다. 다음 단계로 MaterialTheme을 래핑하는 NRTheme Composable을 만들어 이 색상과 타이포그래피를 적용하는 것을 고려해보세요.
  • 이점:
    • 중앙 집중화: 앱의 모든 Compose UI가 일관된 테마를 따르도록 강제할 수 있습니다.
    • 유연성: 향후 다크 모드 지원이나 테마 변경이 필요할 때 NRTheme 내부만 수정하면 됩니다.
    • Material Design 시스템 활용: MaterialTheme.colorSchemeMaterialTheme.typography를 통해 정의된 값을 쉽게 참조할 수 있게 됩니다.
  • 예시:
    // presentation/src/main/java/com/nextroom/nextroom/presentation/common/compose/theme/Theme.kt (새 파일)
    @Composable
    fun NextRoomTheme(
        content: @Composable () -> Unit
    ) {
        val colors = lightColorScheme(
            primary = NRColor.ColorPrimary,
            surface = NRColor.ColorSurface,
            onSurface = NRColor.OnSurface,
            // ... NRColor의 다른 색상들을 MaterialTheme에 매핑
        )
        val typography = Typography(
            // ... NRTypo의 스타일들을 MaterialTheme에 매핑
            headlineLarge = NRTypo.Title.size24SemiBold,
            bodyMedium = NRTypo.Body.size14Regular,
            // ...
        )
    
        MaterialTheme(
            colorScheme = colors,
            typography = typography,
            content = content
        )
    }
    그리고 HintFragmentsetContent에서 NextRoomTheme으로 래핑합니다.
    setContent {
        NextRoomTheme { // 추가
            val state by viewModel.uiState.collectAsState()
            Column(modifier = Modifier.fillMaxSize()) {
                // ...
            }
        } // 추가
    }

2. 파일 끝 개행 문자 추가

  • 파일: presentation/src/main/java/com/nextroom/nextroom/presentation/base/ComposeBaseFragment.kt, presentation/src/main/java/com/nextroom/nextroom/presentation/base/ComposeBaseViewModelFragment.kt, presentation/src/main/java/com/nextroom/nextroom/presentation/common/compose/NRColor.kt, presentation/src/main/java/com/nextroom/nextroom/presentation/common/compose/NRLoading.kt, presentation/src/main/java/com/nextroom/nextroom/presentation/common/compose/NRToolbar.kt, presentation/src/main/java/com/nextroom/nextroom/presentation/common/compose/NRTypo.kt, presentation/src/main/java/com/nextroom/nextroom/presentation/extension/Modifier.kt, presentation/src/main/java/com/nextroom/nextroom/presentation/ui/hint/compose/ImagePager.kt
  • 내용: 새로 추가된 파일들 중 여러 파일에 파일의 마지막에 개행 문자가 누락되어 있습니다.
  • 조치 제안: 각 파일의 마지막에 개행 문자를 추가하여 코드 스타일 가이드를 준수합니다.

3. HintScreenExperimentalFoundationApi 어노테이션 위치

  • 파일: presentation/src/main/java/com/nextroom/nextroom/presentation/ui/hint/compose/HintScreen.kt (L30)
  • 내용: HintScreen 함수에 @OptIn(ExperimentalFoundationApi::class)가 적용되어 있습니다. LazyColumn 자체는 foundation 패키지에 있지만 ExperimentalFoundationApi를 요구하지 않습니다. 이 어노테이션은 ImagePager 내부의 HorizontalPager 때문에 필요한 것으로 보입니다.
  • 조치 제안: ImagePagerExperimentalFoundationApi를 사용하므로, HintScreen에서는 이 어노테이션을 제거하고 ImagePager를 호출하는 부분에만 @OptIn을 적용하거나, ImagePager 컴포저블 자체에 @OptIn을 적용하는 것이 더 명확합니다. 현재 ImagePager.kt 파일에 이미 @OptIn이 적용되어 있으므로 HintScreen.kt에서는 제거해도 무방합니다.

✅ Positive Aspects (긍정적인 측면)

  • Compose 도입 및 기본 컴포넌트 구축: 프로젝트에 Jetpack Compose를 성공적으로 도입하고, NRColor, NRTypo, NRToolbar, NRLoading 등 재사용 가능한 기본 컴포넌트들을 잘 구축하여 향후 Compose 개발의 기반을 마련했습니다.
  • 기존 로직의 Compose 마이그레이션: HintFragment의 복잡한 UI 로직(이미지 뷰페이저, 스크롤, 구독 상태에 따른 이미지 처리 등)을 Compose로 깔끔하게 마이그레이션했습니다.
  • 상태 및 이벤트 처리 분리: HintViewModel에서 uiStateuiEvent를 분리하여 상태와 단발성 이벤트를 명확하게 처리하는 패턴을 도입했습니다.
  • throttleClick 유틸리티: Compose ModifierthrottleClick 확장 함수를 추가하여 중복 클릭 방지 로직을 재사용 가능하게 만든 점이 좋습니다.
  • ComposeBaseFragmentComposeBaseViewModelFragment: 기존 Fragment 계층 구조를 Compose에 맞게 재구성하여 일관된 개발 경험을 제공하려는 노력이 돋보입니다.
  • Preview 기능 활용: 새로 추가된 Compose 컴포넌트들에 @Preview를 적극적으로 사용하여 UI 개발 및 검증에 용이하게 한 점이 훌륭합니다.
  • HintState 개선: lastSecondsHintState에서 분리하여 HintTimerToolbar에서 직접 Flow를 구독하도록 한 것은 상태 객체의 단일 책임 원칙을 강화하고 관심사를 분리한 좋은 리팩터링입니다.

전반적으로 Compose 전환 작업이 매우 잘 이루어졌으며, 새로운 기술 스택 도입에 대한 깊은 이해를 보여줍니다. Critical 이슈로 언급된 아키텍처 일관성 문제에 대한 논의와 결정이 이루어진다면, 이 PR은 프로젝트에 큰 기여를 할 것입니다.


This review was automatically generated using Gemini AI. Please use your judgment when addressing the feedback.

@juhwankim-dev
Copy link
Member Author

Orbit MVI는 앞으로 걷어낼 예정이므로 해당 코멘트는 패스합니다.
CLAUDE.md는 수정하여 develop 브랜치에 바로 푸시 예정

@juhwankim-dev juhwankim-dev merged commit 581fd6e into develop Dec 27, 2025
1 check passed
@juhwankim-dev juhwankim-dev deleted the feature/NR-128 branch December 27, 2025 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants