Skip to content

Conversation

@oungsi2000
Copy link
Contributor

@oungsi2000 oungsi2000 commented Dec 8, 2025

#️⃣ 이슈 번호

#14


🛠️ 작업 내용

  1. Ktlint를 전역적으로 적용하였습니다. 때문에 파일 변경사항이 굉장히 많습니다.
  2. Compose Lint를 적용하였습니다. Lint Violation에 걸려 몇 가지의 컴포저블 함수를 수정했습니다.
  3. FestabookColor, FestabookTyphography를 CompositionLocal로 전달하도록 하였습니다.
  4. Local~.current 에 @ReadOnlyComposable 어노테이션을 붙였습니다.

🙇🏻 중점 리뷰 요청

확인 사항

  1. 현재 Twitter의 Compose Lint 의 릴리즈 날짜가 2022년으로 지원이 사실상 중단되었습니다.
    이로 인해 https://mrmans0n.github.io/compose-rules/latest/ 의 Compose Lint를 사용하였습니다.
    버전별로, 라이브러리 제작사 별로 ruleset이 조금씩 다릅니다 이 점을 감안해주시면 감사하겠습니다.

  2. FestabookColor, FestabookTyphography는 충돌을 줄이고자,CompositionLocal의 getter 함수에 기존 FestabookColor, FestabookTyphography 네이밍을 차용했습니다.

  3. ViewModelTest가 동작하도록 몇 가지 요소를 수정하였습니다.

  4. @Inject어노테이션 관련해서 무수히 많은 컴파일러 경고를 줄이고자 몇 가지 부분을 수정했습니다
    자세한 내용은 슬랙을 참고해주세요
    https://festabook.slack.com/archives/C099EFY59RA/p1765181074465119
    https://festabook.slack.com/archives/C09RFB02A9W/p1765183498967429?thread_ts=1765098976.624099&cid=C09RFB02A9W


📸 이미지 첨부 (Optional)

Summary by CodeRabbit

Release Notes

  • New Features

    • Multi-tab News interface with Notice, FAQ, and Lost Items sections in a single view.
    • New time selection menu for place filters, replacing previous spinner-based selection.
    • Centralized design system with composable theming tokens for consistent UI styling.
  • UI Improvements

    • Redesigned News section with improved navigation and layout.
    • Enhanced theme consistency across the app using unified color, shape, and spacing tokens.
  • Build & Quality

    • Added automated code style checking to improve code quality.
    • Optimized build caching for faster compilation times.
    • New release automation workflow for Android app distribution.
  • Chores

    • Removed unused example test files.
    • Cleaned up unused imports and dependencies.

✏️ Tip: You can customize this high-level summary in your review settings.

전반적인 코드 베이스에서 Metro DI와 관련된 생성자 주입 스타일을 통일하고 일부 코드를 정리했습니다.

- **생성자 주입 스타일 통일:**
    - `@Inject constructor(...)` 형식으로 작성된 모든 클래스의 생성자 주입 코드를 `@Inject class ... (...)` 형식으로 변경하여 어노테이션 위치를 클래스 선언부로 통일했습니다. 이는 코드 가독성을 높이고 일관된 스타일을 유지하기 위함입니다.
    - 대상 파일: `DataSource`, `Repository`, `ViewModel`, `Fragment`, `Interceptor`, `Factory` 등 다수의 클래스

- **`FragmentUtil.kt` 코드 정리:**
    - 더 이상 사용되지 않는 `getObject(key: String)` 확장 함수를 삭제했습니다.
    - API 33(Android 13, UPSIDE_DOWN_CAKE) 미만 버전에서 `getSerializableExtra`, `getParcelableExtra` 등을 호출할 때 발생하는 `DEPRECATION` 경고를 억제하기 위해 `@Suppress("DEPRECATION")` 어노테이션을 추가했습니다.
기존에 `@Inject` 어노테이션을 사용하여 필드로 주입받던 `ViewModelProvider.Factory` 및 기타 의존성들을 프래그먼트의 생성자를 통해 주입하도록 리팩토링했습니다. 이를 통해 의존성 주입 시점을 명확히 하고, 불변성(immutability)을 확보하여 코드의 안정성을 높였습니다.

- **대상 프래그먼트:**
    - `HomeFragment`
    - `PlaceCategoryFragment`
    - `PlaceDetailPreviewFragment`
    - `PlaceDetailPreviewSecondaryFragment`
    - `PlaceListFragment`
    - `PlaceMapFragment`
    - `ScheduleFragment`
    - `SettingFragment`

- **`ExploreViewModel.kt` 수정:**
    - `debounce` 사용을 위해 필요한 `@FlowPreview` 어노테이션을 클래스 레벨에 추가했습니다.
Compose 환경의 테마 시스템(`FestabookTheme`)을 확장하여, 기존에 객체(`object`)로 정의되었던 `FestabookColor`와 `FestabookTypography`를 `CompositionLocalProvider`를 통해 주입하도록 구조를 변경했습니다. 이를 통해 테마 값에 대한 접근 방식을 통일하고 재사용성을 높였습니다.

- **`FestabookTheme.kt` 수정:**
    - `CompositionLocalProvider`에 `LocalColor`와 `LocalTypography`를 추가하여, 하위 컴포저블에서 `FestabookColor`와 `FestabookTypography`를 통해 테마의 색상과 타이포그래피에 접근할 수 있도록 설정했습니다.

- **`FestabookColor.kt` 리팩토링:**
    - 기존의 `object FestabookColor`를 `data class FestabookColorPalette`로 변경했습니다.
    - `staticCompositionLocalOf`를 사용하여 `LocalColor`를 정의하고, `@Composable` `get()` 프로퍼티인 `FestabookColor`를 통해 테마 색상에 접근하도록 수정했습니다.

- **`FestabookTypography.kt` 리팩토링:**
    - 기존의 `val FestabookTypography`를 `FestabookTypographies`로 이름을 변경했습니다.
    - `staticCompositionLocalOf`로 `LocalTypography`를 정의하고, `@Composable` `get()` 프로퍼티인 `FestabookTypography`를 추가하여 테마 타이포그래피에 접근할 수 있도록 개선했습니다.

- **`FestabookSpacing.kt` & `FestabookShapes.kt` 수정:**
    - `get()` 프로퍼티에 `@ReadOnlyComposable` 어노테이션을 추가하여 컴포지션 최적화를 개선했습니다.

- **`themes.xml` 수정:**
    - `android:windowBackground` 값을 `@color/gray050`에서 하드코딩된 `#FAFAFA`로 변경하여, 새로운 테마 시스템의 색상 값(`white`)과 일치시켰습니다.
ktlint의 Compose 관련 규칙을 프로젝트에 새로 적용하고, 이에 따라 발견된 코드 스타일 및 컨벤션 문제를 수정했습니다.

- **ktlint Compose 규칙 적용:**
    - `gradle/libs.versions.toml`과 `app/build.gradle.kts`에 `ktlint-compose` 의존성을 추가했습니다.
    - `.editorconfig` 파일에 `compose_allowed_composition_locals` 설정을 추가하여 `LocalColor`, `LocalTypography` 등의 사용을 허용했습니다.

- **코드 스타일 및 컨벤션 수정:**
    - `@Composable` 함수의 네이밍 컨벤션에 맞춰 `@Preview` 함수의 이름을 `PascalCase`에서 `private` 접근자의 `camelCase`로 변경했습니다. (`CoilImage.kt`)
    - `TimeTagMenu.kt`에서 의미가 불분명했던 콜백 파라미터명 `onSizeDetermined`를 `onSizeDetermine`으로 더 명확하게 수정했습니다.
    - `PullToRefreshContainer.kt` 컴포저블의 파라미터 순서를 Modifier가 마지막에 오도록 조정했습니다.
    - `LostItemScreen.kt`에서 `EmptyStateScreen` 호출 시 불필요하게 전달되던 Modifier를 제거했습니다.
@oungsi2000 oungsi2000 self-assigned this Dec 8, 2025
@coderabbitai
Copy link

coderabbitai bot commented Dec 8, 2025

Caution

Review failed

Failed to post review comments

Walkthrough

This pull request refactors dependency injection patterns across data sources and repositories, introduces a new Compose-based theming system with composition locals, rewrites the news feature UI from XML fragments to Compose, converts state management from LiveData to StateFlow for news and place map features, and updates fragment constructor injection patterns. Additionally, it adds ktlint support and simplifies Android CI/CD workflows.

Changes

Cohort / File(s) Summary
Configuration & Build Setup
.coderabbit.yaml, .editorconfig, app/build.gradle.kts
Removed CodeRabbit config file; updated ktlint composition local settings; added ktlint plugin and ruleset dependency to Gradle.
GitHub Workflows
.github/workflows/android-ci.yml, .github/workflows/android-release-drafter.yml, .github/workflows/release-drafter.yml
Standardized checkout action; added conditional test jobs (develop vs. main branch); introduced release automation workflow with AAB build, version extraction, and main-to-develop sync; removed legacy release-drafter workflow.
Dependency Injection: Data Sources
app/src/main/java/com/daedan/festabook/data/datasource/local/*, app/src/main/java/com/daedan/festabook/data/datasource/remote/**/*
Moved @Inject annotation from primary constructor to class declaration across 12 data source implementation files (local and remote), preserving constructor signatures and DI semantics.
Dependency Injection: Repositories
app/src/main/java/com/daedan/festabook/data/repository/*
Moved @Inject annotation from primary constructor to class declaration across 11 repository implementation files; added FestivalLocalDataSource dependency to FestivalNotificationRepositoryImpl.
Dependency Injection: DI Infrastructure & Services
app/src/main/java/com/daedan/festabook/di/*, app/src/main/java/com/daedan/festabook/data/service/api/FestaBookAuthInterceptor.kt, app/src/main/java/com/daedan/festabook/logging/*
Moved @Inject annotation from constructor to class declaration in fragment factory, view model factory, auth interceptor, and logging classes.
Theme System: New Composition Locals
app/src/main/java/com/daedan/festabook/presentation/theme/FestabookColor.kt, app/src/main/java/com/daedan/festabook/presentation/theme/FestabookShapes.kt, app/src/main/java/com/daedan/festabook/presentation/theme/FestabookSpacing.kt, app/src/main/java/com/daedan/festabook/presentation/theme/FestabookTypography.kt, app/src/main/java/com/daedan/festabook/presentation/theme/FestabookTheme.kt
Refactored color system to data class FestabookColorPalette with LocalColor composition local; introduced FestabookShapes with LocalShapes; introduced FestabookSpacing with LocalSpacing; converted typography to composition-local-backed getter; created unified FestabookTheme composable.
News Feature UI Rewrite: Fragment & ViewModel
app/src/main/java/com/daedan/festabook/presentation/news/NewsFragment.kt, app/src/main/java/com/daedan/festabook/presentation/news/NewsViewModel.kt
Replaced XML-driven TabLayout + adapters with Compose-based NewsScreen; converted state from mutable Compose state to StateFlow-backed properties; renamed public ViewModel methods (toggleNoticeExpandedtoggleNotice, etc.).
News Feature: Child Fragments Removed
app/src/main/java/com/daedan/festabook/presentation/news/faq/FAQFragment.kt, app/src/main/java/com/daedan/festabook/presentation/news/lost/LostItemFragment.kt, app/src/main/java/com/daedan/festabook/presentation/news/notice/NoticeFragment.kt, app/src/main/java/com/daedan/festabook/presentation/news/adapter/NewsPagerAdapter.kt
Removed fragment-based tab navigation; replaced with integrated Compose pages in unified NewsScreen.
News Feature: New Compose Components
app/src/main/java/com/daedan/festabook/presentation/news/component/NewsScreen.kt, app/src/main/java/com/daedan/festabook/presentation/news/component/NewsTabRow.kt, app/src/main/java/com/daedan/festabook/presentation/news/component/NewsTabPage.kt
Added new composables for unified news UI: NewsScreen (orchestrates tabs and state), NewsTabRow (tab selector), NewsTabPage (horizontal pager over notice/FAQ/lost items).
News Feature: Utilities & Styling
app/src/main/java/com/daedan/festabook/presentation/news/component/NewsItem.kt, app/src/main/java/com/daedan/festabook/presentation/news/faq/component/FAQScreen.kt, app/src/main/java/com/daedan/festabook/presentation/news/lost/component/LostItem.kt, app/src/main/java/com/daedan/festabook/presentation/news/lost/component/LostItemScreen.kt, app/src/main/java/com/daedan/festabook/presentation/news/notice/component/NoticeScreen.kt
Updated spacing/padding to use festabookSpacing tokens; replaced hard-coded dimensions with themed values; added LoadingStateScreen for initial load states.
News Feature: Adapters & Listeners
app/src/main/java/com/daedan/festabook/presentation/news/faq/adapter/FAQViewHolder.kt, app/src/main/java/com/daedan/festabook/presentation/news/lost/adapter/LostGuideItemViewHolder.kt, app/src/main/java/com/daedan/festabook/presentation/news/notice/adapter/NoticeViewHolder.kt, app/src/main/java/com/daedan/festabook/presentation/news/notice/adapter/NewsClickListener.kt
Removed click listener invocations from view holders; cleaned up unused imports.
Place Map Feature: StateFlow Migration
app/src/main/java/com/daedan/festabook/presentation/placeMap/PlaceMapViewModel.kt
Converted _timeTags from MutableLiveData to MutableStateFlow; introduced selectedTimeTagFlow as derived StateFlow; moved @Inject to class declaration.
Place Map Feature: Compose Time Tag Menu
app/src/main/java/com/daedan/festabook/presentation/placeMap/placeMap/PlaceMapFragment.kt, app/src/main/java/com/daedan/festabook/presentation/placeMap/timeTagSpinner/component/TimeTagMenu.kt, app/src/main/java/com/daedan/festabook/presentation/placeMap/timeTagSpinner/adapter/TimeTagSpinnerAdapter.kt
Replaced Android Spinner-based time tag menu with Compose-based TimeTagMenu component; removed legacy TimeTagSpinnerAdapter.
Fragment Constructor Injection
app/src/main/java/com/daedan/festabook/presentation/placeMap/placeCategory/PlaceCategoryFragment.kt, app/src/main/java/com/daedan/festabook/presentation/placeMap/placeDetailPreview/PlaceDetailPreviewFragment.kt, app/src/main/java/com/daedan/festabook/presentation/placeMap/placeDetailPreview/PlaceDetailPreviewSecondaryFragment.kt, app/src/main/java/com/daedan/festabook/presentation/placeMap/placeList/PlaceListFragment.kt, app/src/main/java/com/daedan/festabook/presentation/setting/SettingFragment.kt, app/src/main/java/com/daedan/festabook/presentation/schedule/ScheduleFragment.kt
Converted defaultViewModelProviderFactory from field injection to constructor parameter in six fragments.
ViewModel Refactoring
app/src/main/java/com/daedan/festabook/presentation/home/HomeFragment.kt, app/src/main/java/com/daedan/festabook/presentation/home/HomeViewModel.kt, app/src/main/java/com/daedan/festabook/presentation/main/MainActivity.kt, app/src/main/java/com/daedan/festabook/presentation/main/MainViewModel.kt
Moved @Inject to class declarations; added NewsViewModel support to MainActivity; removed notice expansion APIs from MainViewModel; updated HomeFragment constructor injection.
UI Components & Utilities
app/src/main/java/com/daedan/festabook/presentation/common/component/CardBackground.kt, app/src/main/java/com/daedan/festabook/presentation/common/component/CoilImage.kt, app/src/main/java/com/daedan/festabook/presentation/common/component/Header.kt, app/src/main/java/com/daedan/festabook/presentation/common/AnimationUtil.kt, app/src/main/java/com/daedan/festabook/presentation/common/FragmentUtil.kt, app/src/main/java/com/daedan/festabook/presentation/common/ImageUtil.kt, app/src/main/java/com/daedan/festabook/presentation/common/PermissionUtil.kt
Updated CardBackground to use themed shape; added new Header composable; removed Bundle.getObject extension; private preview functions; added deprecation suppressions.
PlaceDetail Updates
app/src/main/java/com/daedan/festabook/presentation/placeDetail/PlaceDetailActivity.kt, app/src/main/java/com/daedan/festabook/presentation/placeDetail/PlaceDetailViewModel.kt, app/src/main/java/com/daedan/festabook/presentation/placeDetail/adapter/PlaceImageViewPagerAdapter.kt, app/src/main/java/com/daedan/festabook/presentation/placeDetail/adapter/PlaceImageViewPagerViewHolder.kt, app/src/main/java/com/daedan/festabook/presentation/placeDetail/logging/*
Removed unused imports; added trailing commas; minor formatting adjustments.
Other ViewModels
app/src/main/java/com/daedan/festabook/presentation/placeMap/placeList/PlaceListViewModel.kt, app/src/main/java/com/daedan/festabook/presentation/setting/SettingViewModel.kt, app/src/main/java/com/daedan/festabook/presentation/splash/SplashViewModel.kt
Moved @Inject annotation from constructor to class declaration.
Layout & Resources
app/src/main/res/layout/fragment_place_map.xml, app/src/main/res/values/themes.xml
Replaced Spinner-based time tag menu with ComposeView; updated theme background color from @color/gray050 to hardcoded #FAFAFA.
Domain & Misc
app/src/main/java/com/daedan/festabook/domain/model/PlaceCategory.kt, app/src/main/java/com/daedan/festabook/util/FestabookGlobalExceptionHandler.kt, app/src/main/java/com/daedan/festabook/presentation/error/ErrorActivity.kt, app/src/main/java/com/daedan/festabook/presentation/explore/ExploreActivity.kt, app/src/main/java/com/daedan/festabook/presentation/explore/ExploreViewModel.kt, app/src/main/java/com/daedan/festabook/presentation/placeDetail/component/PullToRefreshContainer.kt
Added trailing comma to enum; removed unused imports; formatting adjustments; added @OptIn(FlowPreview::class) to ExploreViewModel.
Tests
app/src/test/java/com/daedan/festabook/**/*, app/src/androidTest/java/com/daedan/festabook/ExampleInstrumentedTest.kt
Removed example tests; updated test fixtures and view model tests to match new API names and StateFlow access patterns; variable naming consistency.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~65 minutes

Areas requiring extra attention during review:

  • News Feature Rewrite (NewsFragment, NewsViewModel, NewsScreen, related components): Major architectural shift from Fragment-based XML/adapter UI to unified Compose-based screen; StateFlow conversion and method signature changes. Cross-verify that state flows correctly between tabs and that all click callbacks are properly wired through the new architecture.
  • Theme System Refactoring (FestabookColor, FestabookShapes, FestabookSpacing, FestabookTypography, FestabookTheme): New composition-local-backed system requires verification that all usages throughout the codebase are correctly updated and that theming applies consistently to all Compose and XML layouts.
  • Fragment Constructor Injection Pattern (six fragments): Verify that dependency injection still works correctly after conversion from field injection to constructor parameters, particularly in complex inheritance hierarchies.
  • DI Migration Scale (30+ data sources/repositories): While repetitive, ensure the @Inject annotation movement is consistent and doesn't introduce regressions in DI binding or instantiation.
  • Place Map StateFlow Migration (PlaceMapViewModel): Verify that the conversion from LiveData to StateFlow works correctly with existing observers and that the new selectedTimeTagFlow is properly subscribed.
  • Test Updates (NewsViewModelTest, PlaceMapViewModelTest): Confirm that test assertions correctly use StateFlow .value accessor and that renamed ViewModel methods are properly tested.

Possibly related issues

  • Ktlint 전역 적용 및 기타 작업 진행 #14: This PR directly addresses the issue by introducing a comprehensive ktlint configuration, implementing the new Compose theming system with composition locals (LocalColor, LocalShapes, LocalSpacing, LocalTypography) and their respective read-only composable getters (FestabookColor, festabookShapes, festabookSpacing), and ensuring all Compose components and screens utilize the new theming infrastructure.

Poem

🐰 A rabbit hops through code with glee,
@Inject moves from constructor's spree,
Compose themes shimmer, colors flow,
NewsScreen blooms where fragments used to grow,
StateFlow dances, LocalColor glows!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 1.85% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The PR title is partially related to the changeset but not the main point. While Ktlint application is mentioned, the title is overly broad and vague in describing the actual scope. Consider a more specific title that highlights the primary changes, such as 'Apply global Ktlint and refactor theme composition locals' or 'Apply Ktlint formatting and update theme architecture'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/14

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@oungsi2000 oungsi2000 changed the base branch from main to develop December 8, 2025 10:52
Copy link
Contributor

@etama123 etama123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

밀러 고생하셨습니다 ~~

Copy link
Contributor

@parkjiminnnn parkjiminnnn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생했어요 밀러!

@oungsi2000 oungsi2000 merged commit b2c369b into develop Dec 16, 2025
7 of 8 checks passed
@oungsi2000 oungsi2000 deleted the chore/14 branch December 16, 2025 14:02
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.

4 participants