Skip to content

Conversation

@oungsi2000
Copy link
Contributor

@oungsi2000 oungsi2000 commented Dec 1, 2025

#️⃣ 이슈 번호

#5


🛠️ 작업 내용

TimeTagSpinner를 Compose로 마이그레이션 하였습니다.


🙇🏻 중점 리뷰 요청

점진적으로 Compose 마이그레이션을 진행하려고 하고 있습니다.

현재 FragmentContainer 자체만으로 작업량이 상당하기 때문에 fragment_place_map의 각 View들을 각각 별도의 ComposeView로 나눈 후, 추후 한번에 합치도록 하겠습니다.

최종 형태는 다음과 같을 것 같습니다.

@Composable
fun PlaceMapScreen(
    onMapReady: (NaverMap) -> Unit,
    timeTags: List<TimeTag>,
    onTimeTagSelected: (TimeTag) -> Unit,
) {
    NaverMapContent(
        modifier = Modifier.fillMaxSize(),
        onMapReady = onMapReady,
    ) {
        Column(
            modifier = Modifier.wrapContentSize(),
        ) {
            //TimeTagMenu, CategoryChips, PlaceListScreen ... 등등
        }
    }
}

📸 이미지 첨부 (Optional)

mobizen_20251202_000401.mp4

@oungsi2000 oungsi2000 self-assigned this Dec 1, 2025
@oungsi2000 oungsi2000 added the Feat label Dec 1, 2025
@coderabbitai
Copy link

coderabbitai bot commented Dec 1, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/4

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 title TimeTagSpinner Compose 마이그레이션 [Feat] TimeTagSpinner Compose 마이그레이션 Dec 1, 2025
지도 화면에서 사용될 시간대 선택 드롭다운 메뉴인 `TimeTagMenu` 컴포저블을 새로 추가했습니다.

- **주요 기능:**
  - `ExposedDropdownMenuBox`를 사용하여 커스텀 드롭다운 메뉴를 구현했습니다.
  - 사용자가 시간 태그(`TimeTag`)를 선택하면 `onTimeTagClick` 콜백을 통해 선택된 값을 전달합니다.
  - 선택된 `TimeTag`의 이름(`title`)을 버튼 텍스트로 표시합니다.
  - 메뉴 아이템 클릭 시, 리플(ripple) 효과가 끝난 후 메뉴가 닫히도록 `waitForRipple` 유틸리티 함수를 사용했습니다.

- **컴포저블 구성:**
  - `TimeTagMenu`: 드롭다운 메뉴의 전체적인 레이아웃과 상태를 관리합니다.
  - `TimeTagButton`: 드롭다운을 열고 현재 선택된 시간대를 표시하는 버튼입니다.
  - `@Preview`를 포함하여 컴포저블의 시각적 확인이 가능하도록 했습니다.
`PlaceMapFragment`의 `ComposeView` 내부 Modifier 체이닝에서 `background` 함수의 괄호 위치를 수정하여 코드 포맷을 정리했습니다.
`TimeTagMenu` 컴포저블의 상태 관리 방식을 리팩토링했습니다. 기존에는 외부 컴포저블(`PlaceMapFragment`)에서 `title` 상태를 관리하고 `TimeTagMenu`에 전달했지만, 이제 `TimeTagMenu`가 내부적으로 `title` 상태를 관리하도록 변경했습니다.

- **`TimeTagMenu.kt` 수정:**
  - `title` 파라미터를 `initialTitle`로 변경하여 초기값만 받도록 수정했습니다.
  - `remember { mutableStateOf(initialTitle) }`를 사용하여 `title` 상태를 내부적으로 관리합니다.
  - 메뉴 아이템 클릭 시, `onTimeTagClick` 콜백을 호출하기 전에 내부 `title` 상태를 먼저 업데이트하도록 로직을 변경했습니다.

- **`PlaceMapFragment.kt` 수정:**
  - `TimeTagMenu`를 호출하는 부분에서 불필요해진 `title` 상태 관리 로직(`var title by remember ...`)을 제거했습니다.
  - `onTimeTagClick` 람다를 단순화하여 선택된 `timeTag`를 `onTimeTagSelected` 함수에 바로 전달하도록 수정했습니다.
지도 화면(`PlaceMapFragment`)에서 시간 태그 메뉴를 표시하는 `ComposeView`의 ID를 `cv_timeTag`에서 `cv_place_map`으로 변경하고, 관련 코드 구조를 개선했습니다.

- **`fragment_place_map.xml` 수정:**
    - `ComposeView`의 ID를 `cv_timeTag`에서 `cv_place_map`으로 변경하여 이름의 범용성을 높였습니다.
    - `FragmentContainerView`의 제약 조건이 새 ID를 참조하도록 업데이트했습니다.

- **`PlaceMapFragment.kt` 리팩토링:**
    - `setUpObserver()` 메서드에 있던 `ComposeView` 설정 로직을 `setupComposeView()`라는 별도의 메서드로 분리했습니다.
    - `onCreateView()`에서 `setupComposeView()`를 호출하도록 추가하여 코드의 역할과 가독성을 개선했습니다.
    - 변경된 `ComposeView` ID(`cv_place_map`)를 참조하도록 코드를 수정했습니다.
`TimeTag` 선택 메뉴가 Compose `DropdownMenu`로 교체됨에 따라, 더 이상 사용되지 않는 기존의 `TimeTagSpinnerAdapter` 파일을 삭제했습니다. 이 클래스는 `Spinner`를 커스터마이징하는 데 사용되었습니다.
지도 화면(`PlaceMapFragment`)과 시간대 선택 메뉴(`TimeTagMenu`) 컴포저블에 `FestabookTheme`를 적용하고, 디자인 시스템에 정의된 값을 사용하도록 코드를 리팩토링했습니다.

- **`PlaceMapFragment.kt`**
  - `cvPlaceMap`의 `setContent` 블록 전체를 `FestabookTheme`로 감싸 일관된 테마를 적용했습니다.

- **`TimeTagMenu.kt`**
  - `DropdownMenu`의 테두리 모양(`shape`)과 `DropdownMenuItem`의 텍스트 스타일(`style`)에 `festabookShapes`와 `FestabookTypography`를 사용하도록 수정했습니다.
  - 여백과 간격 등 하드코딩된 `dp` 값을 `festabookSpacing`에 정의된 값으로 대체했습니다.
  - `@Preview`에서도 `FestabookTheme`를 적용하여 실제 앱 환경과 동일한 디자인을 확인할 수 있도록 개선했습니다.
private val _timeTags = MutableLiveData<List<TimeTag>>()
val timeTags: LiveData<List<TimeTag>> = _timeTags
private val _timeTags = MutableStateFlow<List<TimeTag>>(emptyList())
val timeTags: StateFlow<List<TimeTag>> = _timeTags.asStateFlow()
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

title = title,
onSizeDetermined = { dropdownWidth = it },
)
DropdownMenu(
Copy link
Contributor

Choose a reason for hiding this comment

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

오 compose에서는 dropdown으로 바로 만들 수 있네요?! 👀

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.

밀러 마이그레이션 수고 많으셨습니다!! 궁금한 점 몇 개 코멘트 남겼는데 확인 부탁드립니당.
다음에 바로 approve 하겠습니다!

val timeTags by viewModel.timeTags.collectAsStateWithLifecycle(
viewLifecycleOwner,
)
if (!timeTags.isEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

isNotEmpty()를 써도 좋을 것 같아용

Copy link
Contributor

@parkjiminnnn parkjiminnnn Dec 2, 2025

Choose a reason for hiding this comment

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

collectAsStateWithLifecycle(viewLifecycleOwner) 이렇게 viewLifecycleOwner를 직접 명시해주는게 메모리 누수 측면에서 더 좋은가요?
명시 안해주었을 때 어떤 상황에서 문제가 발생하나요?(단순 질문)
제 코드엔 안되어 있어서 적용하려구요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

앗 코멘트 남기는걸 까먹었네요.
처음에는 fragment의 생명주기임을 명시하려고 했지만
viewLifecycleOwner와 LocalLifeCycleOwner.current가 동일한 인스턴스라서 그냥 제거했습니다. !

modifier: Modifier = Modifier,
onTimeTagClick: (TimeTag) -> Unit = {},
) {
var title by remember { mutableStateOf(initialTitle) }
Copy link
Contributor

Choose a reason for hiding this comment

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

title은 서버에서 오는 데이터기 때문에 사용자 클릭에 따라 값이 변경되면 onClick관련 메서드를 viewModel에서 만들어서 넘겨주는 것도 좋아보이는데 어떠신가요? 사소해보이긴 하는데 밀러의 의견이 궁금합니다! (진짜 궁금)

ViewModel이 한층 더 뚱뚱해진다는 단점이 있지만 stateHolder = ViewModel이라는 관점에서 봤을 때 나쁘지 않아보이고 컴포저블 함수가 StateLess하게 된다는 점도 있어보입니다!

Copy link
Contributor Author

@oungsi2000 oungsi2000 Dec 3, 2025

Choose a reason for hiding this comment

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

저는 title은 클라이언트 측에서 정해주는 데이터라고 생각했습니다.
또한 TimeTagMenu 컴포저블을 호출했을 때, 개발자가 기대하는 동작 중 하나는 내가 따로 설정하지 않아도 선택된 타임태그의 이름을 표시한다 라고 생각해서 title State를 TimeTagMenu 내부로 넣었습니다 !

하지만 곰곰이 생각해보니, timeTags를 서버에서 가져올 때, selectedTimeTag 또한 같이 정해줍니다.
그런데 TimeTagMenu에서 UI용 상태를 하나 뚫게 되면, viewModel의 selectedTimeTag와 UI의 상태가 불일치될 여지가 있네요.
단일 진실 공급원을 위반합니다.

때문에 제이의 말씀대로 viewModel에서 처리하는게 더 낫다고 생각이 들었어요.

임시 StateFlow를 생성하여 title을 외부 컴포저블로
반영했습니다 ~!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

onClick관련 메서드를 viewModel에서 만들어서 넘겨주는 것도 좋아보이는데 어떠신가요?

viewModel.onDaySelected로 통합했습니다.

text = {
Text(
text = item.name,
style = FestabookTypography.bodyLarge,
Copy link
Contributor

Choose a reason for hiding this comment

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

MaterialTheme.typography.bodyLarge로 바꾸면 적용 테마에 따라 자동으로 바꿔줄 수 있을 것 같아용

Copy link
Contributor Author

Choose a reason for hiding this comment

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

festabookTyphography 를 하나 만들어도 좋을 것 같아요 !


Icon(
painter = painterResource(id = R.drawable.ic_chevron_down),
contentDescription = "드롭다운",
Copy link
Contributor

Choose a reason for hiding this comment

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

👀

Comment on lines 145 to 146
text = title,
style = FestabookTypography.displaySmall,
Copy link
Contributor

Choose a reason for hiding this comment

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

여기도요!

Comment on lines 87 to 92
).background(color = FestabookColor.white)
.border(
width = 2.dp,
color = FestabookColor.gray300,
shape = festabookShapes.radius2,
),
Copy link
Contributor

Choose a reason for hiding this comment

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

제가 CardBackgound함수 만들어놨는데 그거 사용해주셔도 좋을 것 같아요!!

Comment on lines +156 to +162
private suspend inline fun waitForRipple(
timeMillis: Long = 100,
after: () -> Unit = {},
) {
delay(timeMillis)
after()
}
Copy link
Contributor

Choose a reason for hiding this comment

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

요친구 역할을 알 수 있을까요?(진짜 모름..)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

delay를 걸어주지 않으면 TimeTag를 클릭했을 때 너무 빨리 사라져서 클릭 리플 효과가 발생하지 않더라구요 !
XML View에서는 리플 효과가 있어서 Compose에서도 반영하려고 저 코드를 넣었습니다.
(전반적으로 인터랙션이 없어서 굉장히 밋밋했어요. )

기존에 `Fragment`에서 콜백 인터페이스(`OnTimeTagSelectedListener`)를 통해 처리하던 시간대(`TimeTag`) 선택 로직을 `ViewModel`이 직접 상태를 관리하고 Compose UI가 이를 구독하는 단방향 데이터 흐름(UDF) 방식으로 리팩토링했습니다.

- **`PlaceMapViewModel.kt` 수정:**
    - `LiveData`로 관리되던 `selectedTimeTag`를 `StateFlow`(`selectedTimeTagFlow`)로 변환하여 Compose 환경에서 더 효율적으로 상태를 관찰할 수 있도록 했습니다.
    - 시간대 선택 시 장소 선택을 해제하는 로직(`unselectPlace()`)을 `onDaySelected` 메서드 내부로 이동시켜 관련 로직을 통합했습니다.

- **`PlaceMapFragment.kt` 리팩토링:**
    - `OnTimeTagSelectedListener` 인터페이스와 관련 콜백 메서드(`onTimeTagSelected`, `onNothingSelected`)를 제거했습니다.
    - `TimeTagMenu` 컴포저블의 `onTimeTagClick` 람다 내에서 `ViewModel`의 `onDaySelected`를 직접 호출하도록 변경하여 `Fragment`의 역할을 줄였습니다.
    - `ViewModel`의 `StateFlow`를 구독하여 `TimeTagMenu`의 제목(`title`)을 동적으로 업데이트하도록 개선했습니다.

- **`TimeTagMenu.kt` 수정:**
    - `initialTitle` 파라미터를 `title`로 변경하고, 컴포저블 내부에서 `title`을 관리하던 `remember` 상태를 제거했습니다.
    - 이제 `TimeTagMenu`는 외부에서 전달받은 `title`을 그대로 표시하는 상태 비저장(Stateless) 컴포저블로 변경되었습니다.
시간대 선택 메뉴(`TimeTagMenu.kt`)의 코드를 리팩토링하고 디자인 시스템을 적용하여 재사용성을 높이고 일관성을 개선했습니다.

- **`TimeTagMenu.kt` 수정:**
    - `cardBackground` 커스텀 Modifier를 사용하여 배경색, 테두리, 모양을 한 번에 설정하도록 코드를 간소화했습니다.
    - 기존 `FestabookTypography` 대신 `MaterialTheme.typography`를 사용하도록 변경하여 테마 시스템과의 일관성을 강화했습니다.
    - 하드코딩된 아이콘의 `contentDescription`을 `stringResource`를 사용하도록 수정하여 다국어 지원 및 접근성을 개선했습니다.
    - 더 이상 사용하지 않는 `border` import를 제거했습니다.
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 6ea5b4f into develop Dec 3, 2025
6 of 7 checks passed
@oungsi2000 oungsi2000 deleted the feat/4 branch December 3, 2025 11:43
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