Skip to content

Conversation

@comst19
Copy link
Collaborator

@comst19 comst19 commented Jan 15, 2026

1. ⭐️ 변경된 내용

  • Contact 화면 입장 시 무조건 Lottie 표출하도록 수정

2. 🖼️ 스크린샷(선택)

3. 💡 알게된 부분

4. 📌 이 부분은 꼭 봐주세요!

  • 임시로 무조건 Lottie 표출하도록 함
  • 안 쓰는 코드는 리팩토링에서 제거 필요

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Updated matching animation display timing to use a consistent delay before appearing
    • Improved performance of contact data loading by streamlining backend operations

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

@comst19 comst19 requested review from kkh725 and tgyuuAn January 15, 2026 14:45
@comst19 comst19 self-assigned this Jan 15, 2026
@comst19 comst19 added 리뷰 원해요🔥 피어의 리뷰를 기다리는 ing.. 🔥 ㅁㅅ민수 labels Jan 15, 2026
@coderabbitai
Copy link

coderabbitai bot commented Jan 15, 2026

📝 Walkthrough

Walkthrough

The changes refactor animation timing logic in ContactScreen from state-based conditional triggers to a fixed 3-second delay using LaunchedEffect. The isFirstMatching state field is removed entirely from the data class and ViewModel, and opponent contact loading is simplified by removing separate coroutine coordination.

Changes

Cohort / File(s) Summary
Animation logic refactoring
feature/matching/src/main/java/com/puzzle/matching/graph/contact/ContactScreen.kt
Replaced state-based animation trigger with time-driven LaunchedEffect(Unit) that delays 3 seconds; introduces isMatchingAnimationEnd state to control Lottie animation visibility; animation now displays for the full delay period then hides.
State and ViewModel simplification
feature/matching/src/main/java/com/puzzle/matching/graph/contact/ContactViewModel.kt, feature/matching/src/main/java/com/puzzle/matching/graph/contact/contract/ContactState.kt
Removed isFirstMatching field from ContactState data class; eliminated separate opponent contacts loading coroutine and its synchronization logic; consolidated loading into single suspendRunCatching call; removed unused navigation import.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • Piece-Puzzly/Piece-Android#129: Modifies the same three files in ContactScreen, ContactViewModel, and ContactState as part of a Mavericks→Hilt migration refactor.

Suggested labels

리팩토링 🧰

Suggested reviewers

  • tgyuuAn
  • kkh725

Poem

🐰 A timer counts down, no state needed now,
The animation dances for three seconds, bow!
Simplified state, cleaner the way,
First matching checks fade, time leads the play! 🎬✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: ensuring the Lottie animation always displays when entering the Contact screen.
Description check ✅ Passed The description follows the required template and covers the key change. However, sections 2 and 3 are empty, and section 4 notes this is temporary with cleanup needed later.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

🧹 Recent nitpick comments
feature/matching/src/main/java/com/puzzle/matching/graph/contact/ContactScreen.kt (1)

86-92: Consider extracting magic number and note configuration change behavior.

The 3000L delay is a magic number that could be extracted as a named constant for clarity.

Note: On configuration changes (e.g., rotation), isMatchingAnimationEnd will reset to false and the animation will replay. If this is unintended, consider hoisting this state to the ViewModel using rememberSaveable or a similar approach.

♻️ Suggested improvement
+private const val MATCHING_ANIMATION_DURATION_MS = 3000L
+
 `@Composable`
 private fun ContactScreen(
     state: ContactState,
     onCloseClick: () -> Unit,
     onContactClick: (Contact) -> Unit,
 ) {
-    var isMatchingAnimationEnd by remember { mutableStateOf(false) }
+    var isMatchingAnimationEnd by rememberSaveable { mutableStateOf(false) }
     val analyticsHelper = LocalAnalyticsHelper.current

     LaunchedEffect(Unit) {
-        delay(3000L)
+        delay(MATCHING_ANIMATION_DURATION_MS)
         isMatchingAnimationEnd = true
     }

Note: If using rememberSaveable, you'll need to import androidx.compose.runtime.saveable.rememberSaveable.


📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4fd56d6 and 6536d1e.

📒 Files selected for processing (3)
  • feature/matching/src/main/java/com/puzzle/matching/graph/contact/ContactScreen.kt
  • feature/matching/src/main/java/com/puzzle/matching/graph/contact/ContactViewModel.kt
  • feature/matching/src/main/java/com/puzzle/matching/graph/contact/contract/ContactState.kt
💤 Files with no reviewable changes (1)
  • feature/matching/src/main/java/com/puzzle/matching/graph/contact/contract/ContactState.kt
🧰 Additional context used
🧬 Code graph analysis (1)
feature/matching/src/main/java/com/puzzle/matching/graph/contact/ContactViewModel.kt (2)
core/common/src/main/java/com/puzzle/common/ResultUtil.kt (1)
  • suspendRunCatching (5-13)
core/common-ui/src/main/java/com/puzzle/common/base/BaseViewModel.kt (1)
  • setState (39-40)
🔇 Additional comments (2)
feature/matching/src/main/java/com/puzzle/matching/graph/contact/ContactViewModel.kt (1)

43-58: LGTM! Clean simplification of the contact loading logic.

Removing the parallel coroutine coordination and isFirstMatching check aligns well with the time-driven animation approach in ContactScreen. The single suspendRunCatching call is easier to follow while preserving error handling.

feature/matching/src/main/java/com/puzzle/matching/graph/contact/ContactScreen.kt (1)

149-157: LGTM!

The Lottie composition is properly memoized via rememberLottieComposition, and the conditional rendering based on isMatchingAnimationEnd correctly gates the animation display.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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
Collaborator

@kkh725 kkh725 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다.

delay(3000L)
}
LaunchedEffect(Unit) {
delay(3000L)
Copy link
Collaborator

Choose a reason for hiding this comment

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

3000L을 상수로 분리하면 좋을 것 같아요 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ㅁㅅ민수 리뷰 원해요🔥 피어의 리뷰를 기다리는 ing.. 🔥

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants