Skip to content

Conversation

@oungsi2000
Copy link
Contributor

#️⃣ 이슈 번호

#17


🛠️ 작업 내용

  • PlaceDetailPreviewFragment, PlaceDetailSecondPrevewFragment Compose 마이그레이션 하였습니다.

🙇🏻 중점 리뷰 요청

현재 버그가 하나 있습니다

재현 방법

  1. 기기의 개발자 모드에서 활동 유지 안함 설정
  2. PlaceMapFragment -> 장소 클릭 -> PlaceDetailPreviewFragment 표시
  3. PlaceDetailActivity 실행
  4. 뒤로 가기

그러면 기존의 PlaceDetailPreviewFragment가 사라지고, 빈 화면이 나타납니다.

원인으로는 Compose 환경의 가시성 설정 로직과, 기존 Fragment의 가시성 설정 로직이 서로 충돌하여 벌어지는 것으로 추정됩니다.
StateFlow는 초기값이 있어 collect 시 즉시 값을 방출하지만 LiveData는 초기값이 없어 지연됩니다.
이런 특성 때문에 현재 버그가 일어나는 것으로 추정됩니다.

이 부분은 다음 작업인, 하위 프래그먼트 제거 시 반영하도록 하겠습니다.
다음 작업 관련 이슈

📸 이미지 첨부 (Optional)

지도에서 특정 장소를 선택했을 때 하단에 표시되는 장소 상세 정보 미리보기 카드(`PlaceDetailPreviewScreen`)를 새로 구현했습니다. 이 카드는 선택된 장소의 핵심 정보를 요약하여 보여줍니다.

- **`PlaceDetailPreviewScreen.kt` 신규 컴포저블 추가:**
    - 지도에서 선택된 장소(`SelectedPlaceUiState`)의 상세 정보를 표시하는 카드 UI를 구현했습니다.
    - 장소의 카테고리, 이름, 운영 시간, 위치, 주최자, 대표 이미지를 표시합니다.
    - 카드가 나타날 때 부드러운 애니메이션(`fadeIn`, `slideInVertically`) 효과를 적용했습니다.
    - 장소 상세 정보의 상태(`Loading`, `Success`, `Error`, `Empty`)에 따라 분기 처리하도록 설계했습니다.

- **`URLText.kt` 신규 컴포저블 추가:**
    - `Text` 컴포저블을 확장하여, 문자열 내의 URL을 자동으로 감지하고 클릭 가능한 링크로 만들어주는 `URLText`를 구현했습니다.
    - 링크는 밑줄 스타일과 함께 지정된 색상(`gray500`)으로 표시됩니다.

- **`strings.xml` 리소스 추가:**
    - 미리보기 카드에 사용될 아이콘(운영 시간, 주최자)의 접근성을 위한 콘텐츠 설명 문자열을 추가했습니다.
기존의 `View` 시스템 기반으로 구현되었던 장소 상세 정보 미리보기 UI(`PlaceDetailPreviewFragment`)를 Jetpack Compose 기반의 `PlaceDetailPreviewScreen`으로 전면 마이그레이션했습니다. 이를 통해 XML 레이아웃 의존성을 제거하고, 선언적 UI 방식으로 전환하여 코드의 가독성과 유지보수성을 향상시켰습니다.

- **`PlaceDetailPreviewFragment.kt` 리팩토링:**
    - `onCreateView`에서 `ComposeView`를 반환하도록 변경하고, 내부에 `PlaceDetailPreviewScreen` 컴포저블을 설정했습니다.
    - 기존의 `View` 바인딩, `showBottomAnimation`, UI 업데이트 로직(`updateSelectedPlaceUi`) 등을 모두 제거했습니다.
    - `ViewModel`의 `LiveData`를 `StateFlow`로 변환하여 `collectAsStateWithLifecycle`로 상태를 구독하도록 수정했습니다.
    - 화면 클릭, 에러 처리, 비어있는 상태 처리 로직을 `PlaceDetailPreviewScreen`의 콜백으로 위임했습니다.
    - `onBackPressedCallback`의 활성화 상태를 `LaunchedEffect` 내에서 관리하도록 변경했습니다.

- **`PlaceDetailPreviewScreen.kt` (컴포저블) 수정:**
    - 기존의 `AnimatedVisibility`를 제거하고, `Animatable`과 `graphicsLayer`를 사용한 커스텀 애니메이션을 구현하여 UI가 나타날 때 아래에서 위로 올라오며 서서히 나타나는 효과를 적용했습니다.
    - 장소 설명(`description`) 텍스트에 최대 두 줄 제한과 `Ellipsis`(줄임표)를 적용했습니다.

- **`PlaceMapViewModel.kt` 수정:**
    - 기존의 `selectedPlace` `LiveData`를 `selectedPlaceFlow`라는 `StateFlow`로 변환하여 Compose 환경에서 상태를 효율적으로 관찰할 수 있도록 했습니다.
기존 `PlaceDetailPreviewScreen`에 포함되어 있던 입장/퇴장 애니메이션 로직을 `PreviewAnimatableBox` 컴포저블로 분리하여 재사용성을 높였습니다. 또한, 장소 이름과 카테고리 아이콘만 간결하게 표시하는 `PlaceDetailPreviewSecondaryScreen`을 새로 추가했습니다.

- **`PreviewAnimatableBox.kt` 신규 추가:**
    - `visible` 상태에 따라 Y축 이동(`translationY`) 및 투명도(`alpha`) 애니메이션을 처리하는 `Box` 래퍼 컴포저블을 구현했습니다.
    - 배경, 테두리, 모양 등 UI 속성을 파라미터로 받아 커스터마이징할 수 있습니다.

- **`PlaceDetailPreviewScreen.kt` 리팩토링:**
    - 기존에 직접 구현되어 있던 `Animatable`, `LaunchedEffect`를 사용한 애니메이션 코드를 제거했습니다.
    - 새로 추가된 `PreviewAnimatableBox`를 사용하여 UI와 애니메이션 로직을 분리하고 코드를 간소화했습니다.

- **`PlaceDetailPreviewSecondaryScreen.kt` 신규 추가:**
    - 지도 위에서 선택된 장소의 아이콘과 이름만 표시하는 간단한 미리보기 화면을 구현했습니다.
    - 이 화면 역시 `PreviewAnimatableBox`를 사용하여 애니메이션 효과를 적용합니다.

- **`strings.xml` 수정:**
    - 카테고리 마커 아이콘에 대한 접근성을 위해 `content_description_iv_category_marker` 문자열 리소스를 추가했습니다.
…pose 마이그레이션

기존 View 시스템 기반의 `PlaceDetailPreviewSecondaryFragment`를 Jetpack Compose로 마이그레이션하여 UI 구현 방식을 변경했습니다.

- **`PlaceDetailPreviewSecondaryFragment.kt` 수정:**
    - `onViewCreated` 및 ViewBinding 관련 로직을 제거하고, `onCreateView`에서 `ComposeView`를 반환하도록 변경했습니다.
    - `PlaceDetailPreviewSecondaryScreen` 컴포저블을 사용하여 UI를 구성하고, `viewModel.selectedPlaceFlow`를 `collectAsStateWithLifecycle`로 구독하여 상태를 전달했습니다.
    - 뒤로 가기 콜백(`OnBackPressedCallback`) 활성화 로직을 `LaunchedEffect` 내에서 처리하도록 변경했습니다.
    - 클릭 시 발생하는 로그 기록 로직(`PlacePreviewClick`)을 컴포저블의 `onClick` 콜백으로 이동시켰습니다.

- **`PlaceDetailPreviewSecondaryScreen.kt` 수정:**
    - 컴포넌트 전체에 클릭 이벤트를 지원하기 위해 `onClick` 파라미터를 추가하고 `clickable` Modifier를 적용했습니다.
    - `PreviewAnimatableBox`에 `fillMaxWidth` Modifier와 `shape`(`radius2`) 설정을 추가하여 UI 레이아웃을 개선했습니다.
@oungsi2000 oungsi2000 requested a review from etama123 December 20, 2025 04:42
@oungsi2000 oungsi2000 self-assigned this Dec 20, 2025
@coderabbitai
Copy link

coderabbitai bot commented Dec 20, 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/17

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.

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.

고생하셨습니다 밀러~
코드에 신경을 많이 써주신 것 같네요!
궁금한 점 살짝 코멘트 남겨봤습니다. 확인 부탁드려요~

Comment on lines +70 to +86
LaunchedEffect(placeDetailUiState) {
backPressedCallback.isEnabled = true
}

Box(
modifier = Modifier.fillMaxSize(),
contentAlignment = Alignment.BottomCenter,
) {
PlaceDetailPreviewScreen(
placeUiState = placeDetailUiState,
visible = visible,
modifier =
Modifier
.padding(
vertical = festabookSpacing.paddingBody4,
horizontal = festabookSpacing.paddingScreenGutter,
),
Copy link
Contributor

Choose a reason for hiding this comment

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

요 작업들을 컴포저블이 아닌 Fragment에서 하신 이유가 궁금합니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LaunchedEffect(placeDetailUiState) {
    backPressedCallback.isEnabled = true
}

이 부분 같은 경우는 backPressedCallback이 액티비티에 종속적이기 떄문에
컴포저블에서 수행하면 람다를 넘겨줘서 처리해줘야 합니다.
저는 이 부분이 불필요한 람다라고 생각이 되어서 프래그먼트에서 처리해 주었는데요.

다시 찾아보니까 BackHandler 라는것이 있어 더 간편하게 구현할 수 있더라구요!
반영했습니다 !

)
},
onError = { selectedPlace ->
showErrorSnackBar(selectedPlace.throwable)
Copy link
Contributor

Choose a reason for hiding this comment

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

제 마이그레이션 코드에도 error 상태 처리가 아직 안되어 있는데 나중에 공통 컴포저블 ErrorSnackBar를 만들어서 같이 사용하면 좋을 것 같네요~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

일단 점진적 마이그레이션이기 때문에 기존의 에러 상태 처리 로직을 구현했습니다 !
ErrorSnackBar가 만들어진다면 즉시 반영할게요 !

Comment on lines +67 to +68
LaunchedEffect(placeDetailUiState) {
backPressedCallback.isEnabled = true
Copy link
Contributor

Choose a reason for hiding this comment

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

uiState가 변경될 때마다 true로 바뀌었다가 Empty면 콜백으로 다시 false하는 구조는 리컴포지션 때 중복이라 비효율적인 것 같은데 아래 코드처럼 바뀌는건 어때요?

LaunchedEffect(placeDetailUiState) {
                    backPressedCallback.isEnabled =
                        placeDetailUiState !is SelectedPlaceUiState.Empty
                }

추가로 Ui가 성공상태일 때만 그리게 되어있어서 Empty도 성공상태일 때만 사용자가 제어할 것 같은데 Success 의 상태로 isEmpty 상태를 가지는 건 어떠신가요?
아니면 Empty와 Success 상태를 분리하신 이유가 궁금합니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#19 (comment)

BackHandler를 사용해서 해당 코드 제거했습니다 !

현재는 uiState가 변경될 때마다 처리하는 것이 아닌, visible 상태를 사용하도록 변경했어요 !

@oungsi2000 oungsi2000 merged commit 3e3d13c into develop Dec 28, 2025
10 of 11 checks passed
@oungsi2000
Copy link
Contributor Author

@parkjiminnnn
해당 내용은 다음 PR에 반영하겠습니다 !

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