Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough홈 화면 기능을 새로 구현하고 관련 데이터 모델·리포지토리·뷰모델·Composable UI를 추가했으며, Coil 3로 업그레이드 및 네트워크 모듈 추가, 앱 디버그 로깅 커스텀화, 플래그 이모지 유틸리티와 캘린더 벡터 리소스를 추가했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (UI)
participant Screen as HomeScreen
participant VM as HomeViewModel
participant Auth as AuthRepository
participant Repo as HomeRepository
participant State as HomeState
User->>Screen: Composable 렌더링 요청
Screen->>VM: 상태 구독 / 이벤트 전달
activate VM
VM->>Auth: initSession()
Auth-->>VM: Success/Failure
par Load contents
VM->>Repo: getMyTravel()
Repo-->>VM: InProgressTravel
VM->>Repo: getPopularTravels()
Repo-->>VM: List<TravelSummary>
VM->>Repo: getRecommendedTravels()
Repo-->>VM: List<TravelSummary>
end
VM->>State: HomeState 업데이트
deactivate VM
VM-->>Screen: 업데이트된 HomeState 푸시
Screen->>User: 섹션별 렌더링 및 표시
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
jihee-dev
force-pushed
the
feature/NDGL-61
branch
from
8d385de to
2a40676
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@core/util/src/main/java/com/yapp/ndgl/core/util/FlagEmojiUtil.kt`:
- Around line 6-9: The toFlagEmoji() function uses locale-sensitive uppercase()
and lacks input validation; update String.toFlagEmoji to first validate the
input is exactly 2 characters and each char is in the A–Z range, then call
uppercase(Locale.ROOT) (or Locale.US) before mapping to codepoints using
FLAG_EMOJI_OFFSET, and throw or return a clear error for invalid inputs;
reference: String.toFlagEmoji and FLAG_EMOJI_OFFSET.
In
`@data/travel/src/main/java/com/yapp/ndgl/data/travel/repository/HomeRepository.kt`:
- Around line 11-23: The sample travel in getMyTravel uses hardcoded past dates;
update it to use relative dates so the "in-progress" travel stays current:
compute val start = LocalDate.now(), val end = start.plusDays(3) (or desired
duration), set startDate = start, endDate = end and adjust dayCount =
ChronoUnit.DAYS.between(start, end).toInt() + 1 (or appropriate calculation).
Keep the rest of the InProgressTravel fields (currentPlace, title, thumbnailUrl)
unchanged and ensure imports/reference to LocalDate/ChronoUnit remain correct.
In `@feature/home/src/main/java/com/yapp/ndgl/feature/home/main/HomeViewModel.kt`:
- Around line 113-117: Validate the incoming tab index before updating state in
HomeViewModel.handleIntent for HomeIntent.SelectPopularTravelTab: compute a
maxIndex (e.g., derive from your tab list size like popularTabs.size - 1 or the
enum count) and either guard or clamp the value (use intent.index.coerceIn(0,
maxIndex) or return/LOG when out of range) and then call reduce {
copy(popularTravelSelectedTabIndex = safeIndex) } instead of writing
intent.index directly; reference the HomeViewModel.handleIntent method,
HomeIntent.SelectPopularTravelTab, and the reduce {
copy(popularTravelSelectedTabIndex = ... ) } site when making the change.
- Around line 42-110: Each loader (loadMyTravel, loadPopularTravel,
loadRecommendedTravel) silently swallows failures; add onFailure handlers to the
suspendRunCatching chains that log the throwable (use the same logger used in
initSession or a process/global logger) and update state via reduce to a
fallback/empty/error variant (e.g., set myTravel to an Idle/Empty/Error
HomeState.MyTravel, popularTravelTabs/popularTravelsByTab to empty defaults, and
recommendedContents to an empty list) so UI can show a fallback; keep the
existing onSuccess logic and ensure you reference suspendRunCatching {
homeRepository.getMyTravel() } / getPopularTravels() / getRecommendedTravels()
and call reduce(...) inside the onFailure to record the error and fallback
state.
In `@feature/home/src/main/res/values/strings.xml`:
- Line 9: The string resource home_my_travel_card_d_day_minus currently formats
as "D%d" which omits the minus sign; update its value to include the minus sign
(e.g., "D-%d") so values render as "D-3" for negative days; change the string
value for home_my_travel_card_d_day_minus accordingly.
In `@gradle/libs.versions.toml`:
- Around line 45-47: The project declares Coil as coil = "3.1.0" which is built
against Kotlin 2.1.x and is incompatible with your current Kotlin 2.0.21; either
upgrade the project's Kotlin version to a 2.1.x release (update the Kotlin
Gradle plugin version used by the build where Kotlin is defined) so it matches
Coil 3.x, or change the dependency to a Coil 2.x artifact (replace coil =
"3.1.0" with the appropriate Coil 2.x coordinate) to remain on Kotlin 2.0.21; if
you choose Kotlin upgrade, also scan code for Coil 3.x package/name changes
(e.g., io.coil-kt.coil3 / coil3.*) and adjust imports/usages accordingly.
🧹 Nitpick comments (8)
build-logic/src/main/kotlin/NDGLFeaturePlugin.kt (1)
28-29: feature/home 모듈 외에 Coil을 사용하지 않는 모듈에서 의존성을 제거하세요.현재 모든 feature 모듈(auth, home, travel-helper, travel)이 NDGLFeaturePlugin을 통해 Coil 의존성을 주입받고 있지만, 실제로 Coil(AsyncImage)을 사용하는 모듈은 feature/home뿐입니다. feature/auth, feature/travel-helper, feature/travel에서 불필요한 Coil 의존성을 제거하면 앱 크기와 빌드 시간을 개선할 수 있습니다.
data/travel/src/main/java/com/yapp/ndgl/data/travel/model/InProgressTravel.kt (1)
5-5: FIXME는 이슈로 분리해 주세요.릴리즈 전 추적 가능하도록 티켓화하는 것을 권장합니다. 필요하면 이슈 템플릿 초안도 도와드릴게요.
data/travel/src/main/java/com/yapp/ndgl/data/travel/repository/HomeRepository.kt (1)
132-134: 랜덤 URL은 프리뷰/스냅샷 테스트를 불안정하게 만들 수 있습니다.샘플 데이터라면 시드 고정 또는 고정 이미지 ID를 쓰는 쪽이 재현성에 유리합니다.
♻️ 수정 제안
class HomeRepository `@Inject` constructor() { + private val random = Random(0) ... private fun randomImageUrl(): String { - val randomId = Random.nextInt(0, 500) + val randomId = random.nextInt(0, 500) return "https://picsum.photos/id/$randomId/200/300" } }feature/home/src/main/java/com/yapp/ndgl/feature/home/main/HomeContract.kt (1)
45-56: Compose 안정성 어노테이션 추가를 권장합니다.
TravelPlace,PopularTravelTab도 불변 데이터이므로@Immutable적용이 재구성 최적화에 도움이 됩니다.♻️ 수정 제안
- data class TravelPlace( + `@Immutable` + data class TravelPlace( val category: String, val estimatedTime: String, val name: String, val thumbnailUrl: String, ) - data class PopularTravelTab( + `@Immutable` + data class PopularTravelTab( val tag: String, val name: String, `@DrawableRes` val icon: Int? = null, )feature/home/src/main/java/com/yapp/ndgl/feature/home/main/MyTravelCardSection.kt (1)
145-147:DateTimeFormatter를 composable 외부에서 생성하여 불필요한 재생성을 방지하세요.
DateTimeFormatter.ofPattern()이 composable 내부에서 호출되어 매 recomposition마다 새로운 인스턴스가 생성됩니다. 성능 최적화를 위해remember를 사용하거나 companion object/top-level에서 formatter를 정의하는 것이 좋습니다.♻️ remember를 사용한 개선 제안
+ val dateFormat = stringResource(R.string.home_my_travel_card_date_format) + val dateFormatter = remember(dateFormat) { + DateTimeFormatter.ofPattern(dateFormat) + } - val dateFormatter = DateTimeFormatter.ofPattern( - stringResource(R.string.home_my_travel_card_date_format), - )또는 top-level에서 정의:
private val DATE_FORMATTER = DateTimeFormatter.ofPattern("M월 d일")Lines 215-217의
InProgressTravelCard에서도 동일한 패턴이 사용되고 있으니 함께 수정해 주세요.feature/home/src/main/java/com/yapp/ndgl/feature/home/main/RecommendedContentSection.kt (1)
37-41:modifier파라미터 추가를 권장합니다.다른 섹션 컴포넌트(
MyTravelCardSection,PopularTravelSection)와 달리modifier파라미터가 없어 호출하는 쪽에서 padding, size 등을 커스터마이징하기 어렵습니다. Compose 컨벤션에 맞게modifier파라미터를 추가하는 것이 좋습니다.♻️ modifier 파라미터 추가 제안
`@Composable` internal fun RecommendedContentSection( userName: String, contents: List<TravelSummary>, + modifier: Modifier = Modifier, ) { Column( - modifier = Modifier.fillMaxWidth(), + modifier = modifier.fillMaxWidth(), verticalArrangement = Arrangement.spacedBy(24.dp), ) {feature/home/src/main/java/com/yapp/ndgl/feature/home/main/PopularTravelSection.kt (1)
107-117: 빈 탭 또는 빈 여행 목록에 대한 UI 처리를 고려해 주세요.
tabs.getOrNull(page)?.tag가 null을 반환하거나travelsByTab[tabTag]가 빈 리스트인 경우, 사용자에게 빈 화면이 표시됩니다. 데이터가 없는 경우의 placeholder UI나 empty state 처리가 필요할 수 있습니다.feature/home/src/main/java/com/yapp/ndgl/feature/home/main/HomeScreen.kt (1)
123-131: 주석 처리된 코드를 정리해 주세요.Preview에 주석 처리된 샘플 데이터가 남아있습니다. PR objectives에 TODO로 언급된 사항들이 완료되면 이 코드들도 함께 정리하거나, 필요한 경우 실제 샘플 데이터로 교체해 주세요.
mj010504
left a comment
mj010504
left a comment
There was a problem hiding this comment.
HomeScreen에서 사용되는 Component들에도 preview를 만드는 것이 좋지 않을까 생각이 듭니다.
| onTabSelected = onTabSelected, | ||
| modifier = Modifier.padding(start = 24.dp), | ||
| ) | ||
|
|
There was a problem hiding this comment.
pageSize 고정값보단 contentPadding을 사용하는 것은 어떨까요?? 좀 더 반응형 디자인에 적합한 것 같습니다.
There was a problem hiding this comment.
https://developer.android.com/develop/ui/compose/components/carousel?hl=en
carousel용 라이브러리가 따로 존재하기는 하네요!
There was a problem hiding this comment.
Carousel은 단일 이미지/카드를 나열하는 갤러리형 UI에 최적화되어 있는 것으로 이해하고 있어서
기획 명세상 각 아이템이 단일 카드가 아닌 세로 3행 컬럼(3개의 TravelItem을 묶은 단위)이라, Carousel의 아이템 크기 자동 조절 이점이 크지 않을 것 같습니다..
그리고 CarouselState는 아이템 인덱스 기반이라, 컬럼 단위 스냅 제어가 자연스럽지 않아 보이는데,
지금 구현된 내용이 스펙이랑 맞지 않은 부분이 있어서, LazyRow + snapFlingBehavior로 수정 예정입니다!
jihee-dev
force-pushed
the
feature/NDGL-61
branch
from
2a40676 to
2134a69
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@feature/home/src/main/java/com/yapp/ndgl/feature/home/main/HomeViewModel.kt`:
- Around line 25-34: The failure log in initSession uses string interpolation
which loses the throwable's stack trace; update the onFailure handler to pass
the exception as the throwable to Timber (use Timber.e(exception, "fail to init
session") or the equivalent) so Crashlytics/log trees receive the actual
Throwable coming from authRepository.initSession() inside the
viewModelScope.launch block.
In
`@feature/home/src/main/java/com/yapp/ndgl/feature/home/main/PopularTravelSection.kt`:
- Around line 82-85: Update the guard that renders PopularTravelSection so it
checks both tab list and travel map presence: change the condition in HomeScreen
where you currently do if (state.popularTravelsByTab.isNotEmpty()) to if
(state.popularTravelTabs.isNotEmpty() && state.popularTravelsByTab.isNotEmpty())
to avoid calling rememberPagerState(pageCount = 0) in PopularTravelSection; this
ensures popularTravelTabs, selectedTabIndex and rememberPagerState(pageCount = {
tabs.size }) are only used when tabs exist.
🧹 Nitpick comments (9)
build-logic/src/main/kotlin/NDGLFeaturePlugin.kt (1)
28-29: 모든 feature 모듈에 Coil 의존성이 추가됩니다.
NDGLFeaturePlugin은 모든 feature 모듈에 적용되므로, 이미지 로딩이 필요 없는 모듈에도 Coil이 포함됩니다. 이미지 로딩이 필요한 모듈에서만 직접 의존성을 추가하거나, 별도의 convention plugin을 만드는 것을 고려해 보세요.feature/home/src/main/java/com/yapp/ndgl/feature/home/main/RecommendedContentSection.kt (1)
70-134:RecommendedContentCard의 하단 패널에 수평 패딩 누락 확인 필요.
CountryChip과 텍스트들이 들어 있는 하단Column(Line 88-132)에 수평 패딩이 없어서 카드 좌우 끝까지 콘텐츠가 붙을 수 있습니다. 디자인 의도에 맞는지 확인해 주세요.🔧 수정 제안 (필요 시)
Column( modifier = Modifier .fillMaxWidth() .background(NDGLTheme.colors.white) - .padding(vertical = 16.dp), + .padding(horizontal = 16.dp, vertical = 16.dp), verticalArrangement = Arrangement.spacedBy(10.dp), ) {feature/home/src/main/java/com/yapp/ndgl/feature/home/main/MyTravelCardSection.kt (2)
145-147:DateTimeFormatter가 리컴포지션마다 재생성됩니다.
DateTimeFormatter.ofPattern()이 composable 내부에서 매번 호출됩니다.remember로 캐싱하면 불필요한 객체 생성을 줄일 수 있습니다. (Line 215-217 동일)♻️ 수정 제안
- val dateFormatter = DateTimeFormatter.ofPattern( - stringResource(R.string.home_my_travel_card_date_format), - ) + val pattern = stringResource(R.string.home_my_travel_card_date_format) + val dateFormatter = remember(pattern) { + DateTimeFormatter.ofPattern(pattern) + }
162-186:dDay == 0일 때 표기를 확인해 주세요.
dDay <= 0이면d_day_minus포맷(D%d)을 사용하므로 D-day 당일에는D0으로 표시됩니다. 의도된 동작인지, 혹은D-Day등 별도 표기가 필요한지 확인이 필요합니다.feature/home/src/main/res/values/strings.xml (1)
7-7:DateTimeFormatter와stringResource패턴 호환성 확인.
"M월 d일"패턴은DateTimeFormatter.ofPattern()에서 사용됩니다. 한국어 로케일이 아닌 기기에서는 의도와 다르게 동작할 수 있으므로Locale.KOREA를 명시적으로 지정하는 것을 권장합니다. (MyTravelCardSection.kt의DateTimeFormatter.ofPattern()호출부에서 로케일 지정)feature/home/src/main/java/com/yapp/ndgl/feature/home/main/HomeContract.kt (1)
9-9:TravelSummary데이터 레이어 모델이 UI 상태에 직접 노출되고 있습니다.PR TODO에 이미 "모델을 API 인터페이스에 맞춰 분리"가 언급되어 있으므로 인지하고 계실 것으로 보이지만,
HomeState가data.travel.model.TravelSummary에 직접 의존하면 API 스펙 변경 시 UI 레이어까지 영향이 전파됩니다. 프레젠테이션용 모델로 매핑하는 작업을 우선적으로 진행하시길 권장드립니다.Also applies to: 18-19
feature/home/src/main/java/com/yapp/ndgl/feature/home/main/HomeViewModel.kt (1)
66-101:loadPopularTravel()의 탭 빌딩 로직에서 하드코딩된 한국어 문자열 사용.Line 86의
"전체"문자열이 코드에 직접 하드코딩되어 있습니다. UI 문자열은strings.xml리소스를 통해 관리하는 것이 i18n 및 유지보수 측면에서 바람직합니다. ViewModel에서는StringResID를 저장하고 Composable에서stringResource()로 resolve하는 패턴을 권장합니다.마찬가지로 Line 85의
"all"태그도 상수로 추출하면PopularTravelSection.kt이나 다른 곳에서 참조할 때 오타 위험을 줄일 수 있습니다.feature/home/src/main/java/com/yapp/ndgl/feature/home/main/PopularTravelSection.kt (2)
101-118: Pager의 페이지와 탭이 1:1 매핑되어PageSize.Fixed(310.dp)에서 인접 탭 콘텐츠가 노출됩니다.
HorizontalPager의 각 page가 하나의 탭 콘텐츠(여행 아이템 3개)에 대응됩니다.PageSize.Fixed(310.dp)+contentPadding(horizontal = 24.dp)조합으로 인해, 화면 너비가 넓은 기기에서는 다음 탭의 콘텐츠가 부분적으로 보일 수 있습니다. 이것이 디자인 의도(peek 효과)인지 확인해 주세요. 만약 탭별 콘텐츠가 전체 너비를 차지해야 한다면PageSize.Fill을 사용하는 것이 적합합니다.
132-188:PopularTravelItem에 접근성 콘텐츠 설명이 부족합니다.
AsyncImage의contentDescription으로travel.youtube.title을 사용하는 것은 적절합니다. 다만, 전체Row에semantics블록을 추가하여 TalkBack 사용자가 아이템 전체를 하나의 단위로 인식할 수 있도록 개선하면 좋겠습니다. 또한 국기 이모지(toFlagEmoji())는 스크린 리더가 의미를 전달하기 어려울 수 있으므로 국가명을contentDescription으로 제공하는 것을 고려해 주세요.
| val pagerState = rememberPagerState( | ||
| initialPage = selectedTabIndex, | ||
| pageCount = { tabs.size }, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "*.kt" -type f | head -20Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 1165
🏁 Script executed:
# Search for PopularTravelSection calls
rg -n "PopularTravelSection" --type=kt -B5 -A10Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 105
🏁 Script executed:
# Find HomeScreen.kt specifically
fd "HomeScreen" --type f -e ktRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 151
🏁 Script executed:
cat -n feature/home/src/main/java/com/yapp/ndgl/feature/home/main/HomeScreen.ktRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 7403
popularTravelTabs가 비어있을 수 있는 경우를 방어해야 합니다.
HomeScreen.kt 라인 79에서 if (state.popularTravelsByTab.isNotEmpty()) 조건으로 PopularTravelSection 렌더링을 보호하고 있습니다. 하지만 이 가드는 여행 데이터 맵만 확인하고, popularTravelTabs 리스트의 상태는 확인하지 않습니다.
상태 관리에 데이터 불일치가 발생하면 (예: 여행 데이터는 있지만 탭은 없음) PopularTravelSection이 빈 탭 리스트로 렌더링되어 rememberPagerState(pageCount = 0)이 호출될 수 있습니다.
조건을 if (state.popularTravelTabs.isNotEmpty() && state.popularTravelsByTab.isNotEmpty())으로 변경하여 두 데이터 소스가 모두 있을 때만 섹션을 표시하도록 수정하세요.
🤖 Prompt for AI Agents
In
`@feature/home/src/main/java/com/yapp/ndgl/feature/home/main/PopularTravelSection.kt`
around lines 82 - 85, Update the guard that renders PopularTravelSection so it
checks both tab list and travel map presence: change the condition in HomeScreen
where you currently do if (state.popularTravelsByTab.isNotEmpty()) to if
(state.popularTravelTabs.isNotEmpty() && state.popularTravelsByTab.isNotEmpty())
to avoid calling rememberPagerState(pageCount = 0) in PopularTravelSection; this
ensures popularTravelTabs, selectedTabIndex and rememberPagerState(pageCount = {
tabs.size }) are only used when tabs exist.
2134a69 to
a381382
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@feature/home/src/main/java/com/yapp/ndgl/feature/home/main/HomeScreen.kt`:
- Around line 24-36: HomeRoute currently receives innerPadding but doesn't
forward it to HomeScreen, causing bottom padding to be hardcoded; update the
HomeRoute call to pass innerPadding to HomeScreen (forward the innerPadding
parameter from HomeRoute into the HomeScreen(state = state, innerPadding =
innerPadding, ...)), and then in HomeScreen adjust the LazyColumn contentPadding
bottom to use the passed innerPadding.calculateBottomPadding() (or equivalent)
instead of the hardcoded bottom = 80.dp so content respects system/parent
paddings; reference HomeRoute, HomeScreen, innerPadding, and LazyColumn
contentPadding when making the change.
🧹 Nitpick comments (3)
feature/home/src/main/java/com/yapp/ndgl/feature/home/main/HomeScreen.kt (1)
78-96: 빈 리스트 조건 처리 패턴이 일관되지 않습니다.
PopularTravelSection은item {}내부에서if체크를 하고(Line 78-87),RecommendedContentSection은if체크 후item {}을 생성합니다(Line 89-96). 전자는 빈 상태에서도 빈item이LazyColumn에 추가되어 불필요한 40.dp 간격이 발생합니다.RecommendedContentSection패턴으로 통일하는 것이 좋습니다.♻️ 수정 제안
- item { - if (state.popularTravelsByTab.isNotEmpty()) { - PopularTravelSection( - tabs = state.popularTravelTabs, - selectedTabIndex = state.popularTravelSelectedTabIndex, - travelsByTab = state.popularTravelsByTab, - onTabSelected = onTabSelected, - ) - } - } + if (state.popularTravelsByTab.isNotEmpty()) { + item { + PopularTravelSection( + tabs = state.popularTravelTabs, + selectedTabIndex = state.popularTravelSelectedTabIndex, + travelsByTab = state.popularTravelsByTab, + onTabSelected = onTabSelected, + ) + } + }feature/home/src/main/java/com/yapp/ndgl/feature/home/main/MyTravelCardSection.kt (1)
145-147:DateTimeFormatter를 매 리컴포지션마다 새로 생성하고 있습니다.Line 145-147과 Line 215-217 모두 동일한 패턴으로, 리컴포지션마다
DateTimeFormatter를 새로 생성합니다.remember로 캐싱하면 불필요한 객체 생성을 방지할 수 있습니다.♻️ 수정 제안
- val dateFormatter = DateTimeFormatter.ofPattern( - stringResource(R.string.home_my_travel_card_date_format), - ) + val dateFormatPattern = stringResource(R.string.home_my_travel_card_date_format) + val dateFormatter = remember(dateFormatPattern) { + DateTimeFormatter.ofPattern(dateFormatPattern) + }feature/home/src/main/java/com/yapp/ndgl/feature/home/main/PopularTravelSection.kt (1)
121-129:LaunchedEffect가Column스코프 밖에 위치해 있습니다.Line 87의
Column블록이 Line 119에서 끝나고,LaunchedEffect들은 Line 121-129에 배치되어 있습니다. 기능적으로는 동작하지만, 관련 UI 컴포넌트와 함께Column내부에 배치하는 것이 더 읽기 쉽습니다.
| @Composable | ||
| internal fun HomeRoute( | ||
| viewModel: HomeViewModel = hiltViewModel(), | ||
| innerPadding: PaddingValues, | ||
| ) { | ||
| val state by viewModel.collectAsState() | ||
| HomeScreen( | ||
| state = state, | ||
| onTabSelected = { index -> | ||
| viewModel.onIntent(HomeIntent.SelectPopularTravelTab(index)) | ||
| }, | ||
| ) | ||
| } |
There was a problem hiding this comment.
innerPadding 파라미터가 HomeScreen에 전달되지 않습니다.
HomeRoute가 받은 innerPadding(보통 하단 네비게이션 바 등의 패딩)이 HomeScreen에 전달되지 않아, 컨텐츠가 하단 네비게이션 바 뒤에 가려질 수 있습니다. 현재 HomeScreen 내부에서 bottom = 80.dp로 하드코딩된 값이 이를 우회하고 있지만, 기기별/상황별로 정확하지 않을 수 있습니다.
🔧 수정 제안
`@Composable`
private fun HomeScreen(
state: HomeState = HomeState(),
onTabSelected: (Int) -> Unit = {},
+ innerPadding: PaddingValues = PaddingValues(),
) {그리고 LazyColumn의 contentPadding에서 bottom 패딩을 innerPadding으로부터 계산하도록 변경:
contentPadding = PaddingValues(
top = innerPadding.calculateTopPadding() + 20.dp,
- bottom = 80.dp,
+ bottom = innerPadding.calculateBottomPadding() + 20.dp,
),🤖 Prompt for AI Agents
In `@feature/home/src/main/java/com/yapp/ndgl/feature/home/main/HomeScreen.kt`
around lines 24 - 36, HomeRoute currently receives innerPadding but doesn't
forward it to HomeScreen, causing bottom padding to be hardcoded; update the
HomeRoute call to pass innerPadding to HomeScreen (forward the innerPadding
parameter from HomeRoute into the HomeScreen(state = state, innerPadding =
innerPadding, ...)), and then in HomeScreen adjust the LazyColumn contentPadding
bottom to use the passed innerPadding.calculateBottomPadding() (or equivalent)
instead of the hardcoded bottom = 80.dp so content respects system/parent
paddings; reference HomeRoute, HomeScreen, innerPadding, and LazyColumn
contentPadding when making the change.
a381382 to
0cca857
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@feature/home/src/main/java/com/yapp/ndgl/feature/home/main/HomeScreen.kt`:
- Around line 78-96: The PopularTravelSection is wrapped inside item { ... if
(...) { PopularTravelSection(...) } } which creates an empty item when
state.popularTravelsByTab is empty; change the pattern to match
RecommendedContentSection by moving the conditional outwards so you only add the
LazyList item when data exists (i.e., if
(state.popularTravelsByTab.isNotEmpty()) { item { PopularTravelSection(tabs =
state.popularTravelTabs, selectedTabIndex = state.popularTravelSelectedTabIndex,
travelsByTab = state.popularTravelsByTab, onTabSelected = onTabSelected) } } ),
ensuring no empty item/spacing is produced.
In
`@feature/home/src/main/java/com/yapp/ndgl/feature/home/main/MyTravelCardSection.kt`:
- Around line 56-60: MyTravelCardSection is not passing the incoming modifier to
InProgressTravelCard (while EmptyTravelCard and UpcomingTravelCard do), so
external modifiers like fillMaxWidth won't apply; update the call in
MyTravelCardSection to pass modifier = modifier into InProgressTravelCard and,
if InProgressTravelCard's declaration lacks a Modifier parameter, add a
parameter (e.g., modifier: Modifier = Modifier) to the InProgressTravelCard
composable signature and thread it to the root container inside that composable.
🧹 Nitpick comments (3)
feature/home/src/main/java/com/yapp/ndgl/feature/home/main/HomeViewModel.kt (1)
85-86: 하드코딩된 한국어 문자열"전체"를 string resource로 분리하세요.UI에 표시되는 탭 이름이 ViewModel에 하드코딩되어 있습니다.
strings.xml의 리소스를 사용하면 일관성이 높아지지만, ViewModel에서는 Android 리소스에 직접 접근하기 어려우므로, 도메인 모델이나 상수로 분리하는 것을 고려해 주세요.feature/home/src/main/java/com/yapp/ndgl/feature/home/main/RecommendedContentSection.kt (1)
79-86:AsyncImage에placeholder와error이미지가 없습니다.썸네일 로딩 중이거나 실패 시 빈 영역이 표시됩니다. PR TODO에 스켈레톤 UI가 언급되어 있으므로, 최소한
placeholder와error파라미터를 추가하는 것을 권장합니다. 이 파일뿐 아니라MyTravelCardSection.kt의AsyncImage사용처(Line 122, 290)에도 동일하게 적용해 주세요.feature/home/src/main/java/com/yapp/ndgl/feature/home/main/MyTravelCardSection.kt (1)
145-147:DateTimeFormatter를remember로 감싸 리컴포지션마다 재생성을 방지하세요.
DateTimeFormatter.ofPattern()이 컴포지션마다 새로 생성됩니다.UpcomingTravelCard(Line 145)와InProgressTravelCard(Line 215) 모두 해당됩니다.🔧 수정 제안 (UpcomingTravelCard 예시, InProgressTravelCard에도 동일 적용)
+ val pattern = stringResource(R.string.home_my_travel_card_date_format) + val dateFormatter = remember(pattern) { + DateTimeFormatter.ofPattern(pattern) + } - val dateFormatter = DateTimeFormatter.ofPattern( - stringResource(R.string.home_my_travel_card_date_format), - )Also applies to: 215-217
| item { | ||
| if (state.popularTravelsByTab.isNotEmpty()) { | ||
| PopularTravelSection( | ||
| tabs = state.popularTravelTabs, | ||
| selectedTabIndex = state.popularTravelSelectedTabIndex, | ||
| travelsByTab = state.popularTravelsByTab, | ||
| onTabSelected = onTabSelected, | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| if (state.recommendedContents.isNotEmpty()) { | ||
| item { | ||
| RecommendedContentSection( | ||
| userName = state.userName, | ||
| contents = state.recommendedContents, | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
PopularTravelSection의 조건부 렌더링 패턴이 비어 있는 item을 생성합니다.
Line 89의 RecommendedContentSection은 if가 item {} 밖에 있어 데이터가 없을 때 아이템 자체가 추가되지 않지만, Line 78의 PopularTravelSection은 item {} 안에 if가 있어 데이터가 없어도 빈 아이템이 추가되어 불필요한 40dp 간격이 발생합니다. 패턴을 통일하세요.
🔧 수정 제안
- item {
- if (state.popularTravelsByTab.isNotEmpty()) {
- PopularTravelSection(
- tabs = state.popularTravelTabs,
- selectedTabIndex = state.popularTravelSelectedTabIndex,
- travelsByTab = state.popularTravelsByTab,
- onTabSelected = onTabSelected,
- )
- }
- }
+ if (state.popularTravelsByTab.isNotEmpty()) {
+ item {
+ PopularTravelSection(
+ tabs = state.popularTravelTabs,
+ selectedTabIndex = state.popularTravelSelectedTabIndex,
+ travelsByTab = state.popularTravelsByTab,
+ onTabSelected = onTabSelected,
+ )
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| item { | |
| if (state.popularTravelsByTab.isNotEmpty()) { | |
| PopularTravelSection( | |
| tabs = state.popularTravelTabs, | |
| selectedTabIndex = state.popularTravelSelectedTabIndex, | |
| travelsByTab = state.popularTravelsByTab, | |
| onTabSelected = onTabSelected, | |
| ) | |
| } | |
| } | |
| if (state.recommendedContents.isNotEmpty()) { | |
| item { | |
| RecommendedContentSection( | |
| userName = state.userName, | |
| contents = state.recommendedContents, | |
| ) | |
| } | |
| } | |
| if (state.popularTravelsByTab.isNotEmpty()) { | |
| item { | |
| PopularTravelSection( | |
| tabs = state.popularTravelTabs, | |
| selectedTabIndex = state.popularTravelSelectedTabIndex, | |
| travelsByTab = state.popularTravelsByTab, | |
| onTabSelected = onTabSelected, | |
| ) | |
| } | |
| } | |
| if (state.recommendedContents.isNotEmpty()) { | |
| item { | |
| RecommendedContentSection( | |
| userName = state.userName, | |
| contents = state.recommendedContents, | |
| ) | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@feature/home/src/main/java/com/yapp/ndgl/feature/home/main/HomeScreen.kt`
around lines 78 - 96, The PopularTravelSection is wrapped inside item { ... if
(...) { PopularTravelSection(...) } } which creates an empty item when
state.popularTravelsByTab is empty; change the pattern to match
RecommendedContentSection by moving the conditional outwards so you only add the
LazyList item when data exists (i.e., if
(state.popularTravelsByTab.isNotEmpty()) { item { PopularTravelSection(tabs =
state.popularTravelTabs, selectedTabIndex = state.popularTravelSelectedTabIndex,
travelsByTab = state.popularTravelsByTab, onTabSelected = onTabSelected) } } ),
ensuring no empty item/spacing is produced.
| is MyTravel.InProgress -> InProgressTravelCard( | ||
| travel = myTravel, | ||
| onCardClick = { /* FIXME: 내 여행 페이지 이동 */ }, | ||
| onPlaceClick = { /* FIXME: 장소 상세 보기 페이지 이동 */ }, | ||
| ) |
There was a problem hiding this comment.
InProgressTravelCard에 modifier가 전달되지 않습니다.
EmptyTravelCard과 UpcomingTravelCard는 MyTravelCardSection의 modifier를 전달받지만, InProgressTravelCard는 전달하지 않아 외부에서 지정한 modifier(예: fillMaxWidth)가 적용되지 않습니다.
🔧 수정 제안
is MyTravel.InProgress -> InProgressTravelCard(
+ modifier = modifier,
travel = myTravel,
onCardClick = { /* FIXME: 내 여행 페이지 이동 */ },
onPlaceClick = { /* FIXME: 장소 상세 보기 페이지 이동 */ },
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| is MyTravel.InProgress -> InProgressTravelCard( | |
| travel = myTravel, | |
| onCardClick = { /* FIXME: 내 여행 페이지 이동 */ }, | |
| onPlaceClick = { /* FIXME: 장소 상세 보기 페이지 이동 */ }, | |
| ) | |
| is MyTravel.InProgress -> InProgressTravelCard( | |
| modifier = modifier, | |
| travel = myTravel, | |
| onCardClick = { /* FIXME: 내 여행 페이지 이동 */ }, | |
| onPlaceClick = { /* FIXME: 장소 상세 보기 페이지 이동 */ }, | |
| ) |
🤖 Prompt for AI Agents
In
`@feature/home/src/main/java/com/yapp/ndgl/feature/home/main/MyTravelCardSection.kt`
around lines 56 - 60, MyTravelCardSection is not passing the incoming modifier
to InProgressTravelCard (while EmptyTravelCard and UpcomingTravelCard do), so
external modifiers like fillMaxWidth won't apply; update the call in
MyTravelCardSection to pass modifier = modifier into InProgressTravelCard and,
if InProgressTravelCard's declaration lacks a Modifier parameter, add a
parameter (e.g., modifier: Modifier = Modifier) to the InProgressTravelCard
composable signature and thread it to the root container inside that composable.
6b14774 to
e36fe81
Compare
jihee-dev
force-pushed
the
feature/NDGL-61
branch
2 times, most recently
from
72e87ed to
eab3931
Compare
jihee-dev
force-pushed
the
feature/NDGL-61
branch
from
eab3931 to
c8c179a
Compare
2 participants
NDGL-61 홈 화면 UI 구현
연관 문서
디자인
스크린샷 (Optional)
변경사항
테스트 체크 리스트
Summary by CodeRabbit