-
Notifications
You must be signed in to change notification settings - Fork 0
feat - 홈화면 작업 #63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat - 홈화면 작업 #63
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements the female user version of the matching feature with comprehensive navigation and UI updates. The changes introduce a splash screen with video, a complete home screen with carousel-based matching display, and placeholder screens for chat, profile, and settings.
Key changes include:
- Video splash screen implementation with custom fullscreen video view
- Complete matching flow with carousel UI and detail views
- Bottom navigation with home, chat, and profile tabs
- Logout functionality with proper cleanup
Reviewed changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/main/res/values/themes.xml | Adds splash screen theme with black background |
| app/src/main/res/layout/activity_splash.xml | Layout for splash screen with fullscreen video view |
| app/src/main/res/drawable/*.xml | Navigation icons (filled and outline versions) for user, chat, settings, and home |
| app/src/main/java/com/apptive/japkor/widget/FullscreenVideoView.kt | Custom VideoView that scales video to fill screen while maintaining aspect ratio |
| app/src/main/java/com/apptive/japkor/utils/required_info/RequiredInfoMapper.kt | Updates religion mapping (SHINTO → TOISM) |
| app/src/main/java/com/apptive/japkor/ui/main/setting/SettingScreen.kt | Settings screen with logout button |
| app/src/main/java/com/apptive/japkor/ui/main/mypage/MypageScreen.kt | Placeholder profile page |
| app/src/main/java/com/apptive/japkor/ui/main/home/HomeViewModel.kt | Business logic for fetching and selecting matchings |
| app/src/main/java/com/apptive/japkor/ui/main/home/HomeScreen.kt | Matching carousel UI with detail view |
| app/src/main/java/com/apptive/japkor/ui/main/chat/ChattingScreen.kt | Placeholder chat screen |
| app/src/main/java/com/apptive/japkor/ui/main/MainRouteScreen.kt | Main navigation coordinator with bottom navigation |
| app/src/main/java/com/apptive/japkor/ui/login/LoginScreen.kt | Routes CONNECTING users to home screen |
| app/src/main/java/com/apptive/japkor/ui/localization/Localization.kt | Adds Japanese translations for logout dialog |
| app/src/main/java/com/apptive/japkor/navigation/AppNavHost.kt | Registers home route in navigation graph |
| app/src/main/java/com/apptive/japkor/data/model/Matching.kt | Data model for matching response |
| app/src/main/java/com/apptive/japkor/data/local/DataStoreManager.kt | Adds clearUserInfo method for logout |
| app/src/main/java/com/apptive/japkor/data/api/ServiceFactory.kt | Registers matching service |
| app/src/main/java/com/apptive/japkor/data/api/MatchingService.kt | API interface for matching operations |
| app/src/main/java/com/apptive/japkor/SplashActivity.kt | Activity that plays splash video and navigates to main |
| app/src/main/java/com/apptive/japkor/LoginCallbackActivity.kt | Routes CONNECTING users to home screen |
| app/src/main/AndroidManifest.xml | Sets SplashActivity as launcher activity |
| app/build.gradle.kts | Configures mp4 files as non-compressed resources |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Button( | ||
| onClick = { currentMatching?.let { onShowDetails(it) } }, | ||
| enabled = currentMatching != null, | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .height(50.dp), | ||
| colors = ButtonDefaults.buttonColors( | ||
| containerColor = CustomColor.primary600, | ||
| disabledContainerColor = CustomColor.primary300 | ||
| ), | ||
| shape = RoundedCornerShape(16.dp) | ||
| ) { | ||
| CustomText( | ||
| text = "이 분 프로필이 궁금해요", | ||
| type = CustomTextType.body, | ||
| color = Color.White | ||
| ) | ||
| } |
Copilot
AI
Jan 4, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The button is enabled based on currentMatching being non-null, but currentMatching is derived from matchings.getOrNull(pagerState.currentPage). If the pager is animating or the state is inconsistent, this could result in the button being enabled when it shouldn't be. Consider checking both conditions explicitly.
| if (hasNavigated) return | ||
| hasNavigated = true | ||
| val intent = Intent(this, MainActivity::class.java) | ||
| intent.putExtras(this.intent) |
Copilot
AI
Jan 4, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using Intent.putExtras with this.intent could propagate any extras from the original intent. If the SplashActivity is launched with specific extras, they will be passed to MainActivity. Consider explicitly handling which extras should be forwarded or document this behavior.
| intent.putExtras(this.intent) |
| "기독교" -> "CHRISTIANITY" | ||
| "천주교" -> "CATHOLICISM" | ||
| "신토" -> "SHINTO" | ||
| "신토" -> "TOISM" |
Copilot
AI
Jan 4, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mapping for "신토" (Shinto) has been changed from "SHINTO" to "TOISM", which appears to be a typo. "TOISM" is not a recognized term. This should likely remain as "SHINTO" or be "SHINTOISM" if following a consistent naming pattern for religions.
| "신토" -> "TOISM" | |
| "신토" -> "SHINTO" |
| videoView.stopPlayback() | ||
| navigateToMain() | ||
| } | ||
| videoView.setOnErrorListener { _, _, _ -> |
Copilot
AI
Jan 4, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error handler for the video view returns true, which indicates the error was handled. However, the video is not properly cleaned up before navigating. Consider calling stopPlayback() before navigating to ensure resources are released properly.
| videoView.setOnErrorListener { _, _, _ -> | |
| videoView.setOnErrorListener { _, _, _ -> | |
| videoView.stopPlayback() |
| private fun navigateToMain() { | ||
| if (hasNavigated) return | ||
| hasNavigated = true | ||
| val intent = Intent(this, MainActivity::class.java) | ||
| intent.putExtras(this.intent) | ||
| intent.addFlags(Intent.FLAG_ACTIVITY_NO_ANIMATION) | ||
| startActivity(intent) | ||
| overridePendingTransition(0, 0) | ||
| finish() |
Copilot
AI
Jan 4, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The navigateToMain() function can be called from multiple sources (completion listener and error listener) without proper synchronization. Although hasNavigated flag is checked, this is not thread-safe and could lead to race conditions if both callbacks are triggered simultaneously.
| val data = response.body().orEmpty() | ||
| if (data.isEmpty()) { | ||
| _uiState.value = HomeUiState(isWaiting = true) | ||
| } else { | ||
| _uiState.value = HomeUiState(matchings = data) |
Copilot
AI
Jan 4, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The response.body().orEmpty() call assumes that a null body should be treated as an empty list. However, a null body from a successful response typically indicates a server error or unexpected response format. Consider explicitly handling this case with an error message rather than treating it as an empty matching list.
| val data = response.body().orEmpty() | |
| if (data.isEmpty()) { | |
| _uiState.value = HomeUiState(isWaiting = true) | |
| } else { | |
| _uiState.value = HomeUiState(matchings = data) | |
| val body = response.body() | |
| if (body == null) { | |
| Log.e(TAG, "getFemaleMatchings failed: response body is null on success") | |
| _uiState.value = HomeUiState(isWaiting = true) | |
| _events.tryEmit(HomeUiEvent.ShowToast("매칭 목록을 불러오지 못했습니다.")) | |
| } else { | |
| if (body.isEmpty()) { | |
| _uiState.value = HomeUiState(isWaiting = true) | |
| } else { | |
| _uiState.value = HomeUiState(matchings = body) | |
| } |
| val scaledWidth = (viewHeight * videoRatio).toInt() | ||
| setMeasuredDimension(scaledWidth, viewHeight) | ||
| } else { | ||
| val scaledHeight = (viewWidth / videoRatio).toInt() |
Copilot
AI
Jan 4, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The onMeasure method calculates scaled dimensions but doesn't account for potential integer overflow when multiplying viewHeight by videoRatio or dividing viewWidth by videoRatio. For very large dimensions or extreme aspect ratios, this could cause crashes or incorrect layout.
| val scaledWidth = (viewHeight * videoRatio).toInt() | |
| setMeasuredDimension(scaledWidth, viewHeight) | |
| } else { | |
| val scaledHeight = (viewWidth / videoRatio).toInt() | |
| val scaledWidthDouble = viewHeight.toDouble() * videoRatio.toDouble() | |
| val scaledWidth = scaledWidthDouble | |
| .coerceAtLeast(1.0) | |
| .coerceAtMost(Int.MAX_VALUE.toDouble()) | |
| .toInt() | |
| setMeasuredDimension(scaledWidth, viewHeight) | |
| } else { | |
| val scaledHeightDouble = viewWidth.toDouble() / videoRatio.toDouble() | |
| val scaledHeight = scaledHeightDouble | |
| .coerceAtLeast(1.0) | |
| .coerceAtMost(Int.MAX_VALUE.toDouble()) | |
| .toInt() |
| suspend fun clearUserInfo() { | ||
| context.dataStore.edit { prefs -> | ||
| prefs.remove(KEY_MEMBER_ID) | ||
| prefs.remove(KEY_NAME) | ||
| prefs.remove(KEY_TOKEN) | ||
| prefs.remove(KEY_STATUS) | ||
| } | ||
| } |
Copilot
AI
Jan 4, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The clearUserInfo() method removes specific user-related keys but leaves other potential keys in the DataStore. This could lead to incomplete logout if other user-specific data exists. Consider using the existing clear() method or documenting why selective removal is necessary.
| import retrofit2.Call | ||
| import retrofit2.http.GET | ||
| import retrofit2.http.POST | ||
| import retrofit2.http.Path | ||
|
|
||
| interface MatchingService { | ||
| @GET("members/matchings/female") | ||
| fun getFemaleMatchings(): Call<List<MatchingResponse>> | ||
|
|
||
| @POST("members/matchings/{matchingId}/select") | ||
| fun selectMatching(@Path("matchingId") matchingId: Long): Call<Void> |
Copilot
AI
Jan 4, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The API service methods return Call objects but are being used with awaitResponse(). This is mixing Retrofit's callback-based API with Kotlin coroutines. Consider changing the interface to use suspend functions that return Response directly for cleaner and more idiomatic coroutine usage.
| import retrofit2.Call | |
| import retrofit2.http.GET | |
| import retrofit2.http.POST | |
| import retrofit2.http.Path | |
| interface MatchingService { | |
| @GET("members/matchings/female") | |
| fun getFemaleMatchings(): Call<List<MatchingResponse>> | |
| @POST("members/matchings/{matchingId}/select") | |
| fun selectMatching(@Path("matchingId") matchingId: Long): Call<Void> | |
| import retrofit2.Response | |
| import retrofit2.http.GET | |
| import retrofit2.http.POST | |
| import retrofit2.http.Path | |
| interface MatchingService { | |
| @GET("members/matchings/female") | |
| suspend fun getFemaleMatchings(): Response<List<MatchingResponse>> | |
| @POST("members/matchings/{matchingId}/select") | |
| suspend fun selectMatching(@Path("matchingId") matchingId: Long): Response<Void> |
| } | ||
|
|
||
| LaunchedEffect(matchings.size) { | ||
| if (matchings.isNotEmpty() && pagerState.currentPage >= matchings.size) { |
Copilot
AI
Jan 4, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pager state is created with a dynamic pageCount based on matchings.size. When matchings changes, the pagerState needs to be recreated or the pageCount needs to be updated. The current implementation has a LaunchedEffect to scroll to page 0 if currentPage is out of bounds, but this doesn't handle the case where pageCount changes while the user is viewing a valid page.
| if (matchings.isNotEmpty() && pagerState.currentPage >= matchings.size) { | |
| if (matchings.isNotEmpty()) { |
fkal829
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다~~~👍
에러 찾은 거 진짜 ㄴㅇㅅ
작업내용
기타 작업
홈화면 작업
참고
main/chat/ChatScreen.kt,main/mypage/MypageScreen.kt,main/setting/SettingScreen.kt작업해주시면 됩니다~