Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughGoogle Maps 키 및 관련 라이브러리 추가, 내부 패딩 전파, 여행 팔로우·여행 상세 기능(뷰모델·스크린·컴포넌트) 대규모 추가 및 디자인시스템·유틸·리소스 보강; 기존 TravelDetail 구현은 새 traveldetail 패키지로 이동/교체되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자
participant TravelScreen as TravelScreen
participant TravelVM as TravelViewModel
participant Nav as TravelEntry
participant FollowRoute as Route.FollowTravel
participant FollowVM as FollowTravelViewModel
participant FollowScreen as FollowTravelScreen
User->>TravelScreen: 여행 항목 클릭
TravelScreen->>TravelVM: ClickTravelDetail(travelId) 인텐트
TravelVM->>TravelVM: postSideEffect(NavigateToFollowTravel(travelId))
TravelScreen->>Nav: navigateToFollowTravel(travelId)
Nav->>FollowRoute: Route.FollowTravel(travelId) 생성/네비게이션
FollowRoute->>FollowVM: FollowTravelViewModel.Factory.create(travelId)
FollowVM->>FollowScreen: 상태(FollowTravelState) 발행
FollowScreen->>User: FollowTravel 화면 렌더링
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 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 |
mj010504
force-pushed
the
design/NDGL-53
branch
5 times, most recently
from
cadef03 to
4665ab9
Compare
mj010504
force-pushed
the
design/NDGL-49
branch
from
c9c0269 to
e23c4ae
Compare
mj010504
force-pushed
the
design/NDGL-53
branch
2 times, most recently
from
e591261 to
e6dc184
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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/travel/src/main/java/com/yapp/ndgl/feature/travel/travel/TravelScreen.kt (1)
59-74:⚠️ Potential issue | 🟠 Major하드코딩된 travelId로 네비게이션 고정됨
clickTravel(123)/clickTravelDetail(456)는 실제 선택된 여행과 무관하게 항상 고정 ID로 이동합니다. 실제 데이터와 연결된 ID를 전달하거나, 데모/리뷰용 버튼이라면 debug 전용으로 분리하는 편이 안전합니다.
🤖 Fix all issues with AI agents
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/component/TimelineContent.kt`:
- Around line 243-251: startIndex calculation currently centers into a huge list
but doesn't account for the visibleItemCount offset, causing the WheelPicker to
select initialIndex+2; update the startIndex expression (that uses
totalItemsCount, items.size and initialIndex) to subtract visibleItemCount / 2
so that the initial visible center equals initialIndex (adjust the computed
startIndex used to create listState via rememberLazyListState); ensure
derivedStateOf/currentIndex logic using visibleItemCount remains consistent with
this change.
- Around line 154-156: isTimeSet currently requires
selectedStartTime.inWholeHours > 0 and selectedEndTime.inWholeHours > 0 which
blocks 00:00; relax the check so midnight is allowed by changing the condition
on selectedStartTime and selectedEndTime to use >= 0 (or otherwise check they
are non-negative/initialized) in the isTimeSet computation (refer to isTimeSet,
selectedStartTime, selectedEndTime, and keep the existing
isEndTimeExceeds24Hours logic intact).
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/component/TravelMap.kt`:
- Around line 58-77: The code currently calls places.first() when building
initialPosition and builds LatLngBounds from places in the LaunchedEffect, which
will throw for an empty list; update the TravelMap composable to guard against
empty places by checking places.isEmpty() early and either return a placeholder
UI or use a default CameraPosition (replace use of initialPosition /
places.first()), and modify the LaunchedEffect to skip bounds creation (or use a
default CameraUpdate) when places.isEmpty(); update references to
initialPosition, cameraPositionState, and the LaunchedEffect block so they no
longer assume places has at least one element.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailContract.kt`:
- Around line 52-59: The formatString() in TravelDetailContract incorrectly
returns "만원" for any amount < 10000; update the first branch to produce the
correct small-amount formatting by checking thousands and remainder: when amount
< 10000, if amount % 1000 == 0 return "${amount / 1000}천원", else return
"${amount}원" (or use the existing comma-format convention used elsewhere);
adjust the amount < 10000 branch in formatString() accordingly so small amounts
are rendered as "X천원" or "X원" instead of "만원".
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailViewModel.kt`:
- Around line 315-338: reorderPlaces is applying the move to every itinerary
because it iterates tempItineraries unconditionally; restrict the change to only
the currently selected itinerary (e.g., compare itinerary.id or itinerary.date
to the viewmodel's selected identifier like selectedDate or selectedItineraryId)
so other itineraries remain untouched. In the reorderPlaces function, when
mapping tempItineraries, only perform the removeAt/add and sequence
recalculation for the itinerary that matches the selected identifier; for others
return itinerary unchanged. Also ensure the index bounds check
(fromIndex/toIndex in mutablePlaces.indices) uses the selected itinerary's
places so indices are valid for that specific day's list.
🧹 Nitpick comments (7)
core/ui/src/main/java/com/yapp/ndgl/core/ui/designsystem/NDGLCheckBox.kt (2)
22-50: 접근성 및 유연성 개선 필요현재 구현에서 몇 가지 개선이 필요합니다:
- 접근성(Accessibility):
contentDescription이 없어 스크린 리더 사용자가 체크박스 상태를 인식할 수 없습니다.- Modifier 파라미터 누락: 외부에서 크기, 패딩 등을 조정할 수 없습니다.
- 클릭 피드백:
clickable에 indication이 없어 시각적 피드백이 부족할 수 있습니다.♻️ 접근성 및 유연성 개선 제안
`@Composable` fun NDGLCheckbox( checked: Boolean, onClick: () -> Unit, + modifier: Modifier = Modifier, + contentDescription: String? = null, ) { Box( - modifier = Modifier + modifier = modifier .size(24.dp) .clip(RoundedCornerShape(4.dp)) .background( if (checked) NDGLTheme.colors.green500 else NDGLTheme.colors.white, ) .border( width = 1.25.dp, color = if (checked) Color.Unspecified else NDGLTheme.colors.black400, shape = RoundedCornerShape(4.dp), ) - .clickable { onClick() }, + .clickable( + onClick = onClick, + role = Role.Checkbox, + ) + .semantics { + this.stateDescription = if (checked) "선택됨" else "선택 안 됨" + contentDescription?.let { this.contentDescription = it } + }, contentAlignment = Alignment.Center, )추가 import 필요:
import androidx.compose.ui.semantics.Role import androidx.compose.ui.semantics.contentDescription import androidx.compose.ui.semantics.semantics import androidx.compose.ui.semantics.stateDescription
34-38:Color.Unspecified사용 시 주의
checked상태일 때Color.Unspecified를 border color로 사용하고 있습니다. 이 경우 border가 그려지지 않는 것을 의도한 것으로 보이지만,Color.Transparent가 의도를 더 명확하게 표현합니다.♻️ 명시적인 Color.Transparent 사용 제안
.border( width = 1.25.dp, - color = if (checked) Color.Unspecified else NDGLTheme.colors.black400, + color = if (checked) Color.Transparent else NDGLTheme.colors.black400, shape = RoundedCornerShape(4.dp), )core/ui/src/main/java/com/yapp/ndgl/core/ui/designsystem/NDGLBottomSheet.kt (1)
19-42: 외부에서 sheet 상태 제어 불가현재
sheetState가 함수 내부에서 생성되어 외부에서 프로그래매틱하게 sheet를 닫거나 상태를 제어할 수 없습니다. 예를 들어, 버튼 클릭 시sheetState.hide()를 호출하여 sheet를 닫는 것이 불가능합니다.사용 시나리오에 따라 외부에서
sheetState를 전달받을 수 있도록 파라미터화하는 것을 고려해 주세요.♻️ sheetState 파라미터 추가 제안
`@OptIn`(ExperimentalMaterial3Api::class) `@Composable` fun NDGLBottomSheet( modifier: Modifier = Modifier, onDismissRequest: () -> Unit, showDragHandle: Boolean = true, + sheetState: SheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true), content: `@Composable` ColumnScope.() -> Unit, ) { - val sheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true) - ModalBottomSheet(추가 import 필요:
import androidx.compose.material3.SheetStatefeature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/component/TimelineContent.kt (1)
125-135: 시간 휠의 초기 위치를 현재 값으로 맞추면 UX 개선
Line 125-134에서 초기 인덱스를 상수로 고정해 기존 선택값이 반영되지 않습니다.selectedStartTime기반으로 초기 인덱스를 계산하면 재진입 시 값이 유지됩니다.♻️ 제안 수정
val hourItems = remember { (0..23).toList() } val minuteItems = remember { (0..55 step 5).toList() } var selectedHour by remember { mutableIntStateOf(hourItems[0]) } var selectedMinute by remember { mutableIntStateOf(minuteItems[0]) } + val initialHourIndex = (selectedStartTime.inWholeHours % 24).toInt() + val initialMinuteIndex = ((selectedStartTime.inWholeMinutes % 60) / 5).toInt() ... WheelPicker( items = hourItems, onItemSelected = { selectedHour = it }, - initialIndex = 12, + initialIndex = initialHourIndex, ) WheelPicker( items = minuteItems, onItemSelected = { selectedMinute = it }, - initialIndex = 0, + initialIndex = initialMinuteIndex, padEnabled = true, )feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/component/ContentCard.kt (2)
80-88: 접근성: contentDescription에 string resource 사용 권장
contentDescription이 하드코딩된 영어 문자열로 되어 있습니다. 다국어 지원 및 일관성을 위해strings.xml의 리소스를 사용하는 것이 좋습니다.♻️ 제안 수정
AsyncImage( model = contentInfo.videoInfo.profileImage, - contentDescription = "Profile Image", + contentDescription = stringResource(R.string.content_description_profile_image), modifier = Modifier .size(48.dp) .clip(CircleShape) .background(Color.LightGray), contentScale = ContentScale.Crop, )
strings.xml에 추가:<string name="content_description_profile_image">프로필 이미지</string> <string name="content_description_video_thumbnail">비디오 썸네일</string> <string name="content_description_expand">펼치기</string>
74-77: 중복 클릭 핸들러 검토Row(Line 74-77)와 Icon(Line 127-133) 모두
clickable로isExpanded를 토글합니다. Icon 영역을 클릭하면 이벤트가 두 번 처리될 수 있습니다. Row에만 클릭 핸들러를 두거나, Icon에서 클릭 핸들러를 제거하는 것을 권장합니다.♻️ 제안 수정
Icon( imageVector = ImageVector.vectorResource(R.drawable.ic_28_chevron_down), contentDescription = "Expand", modifier = Modifier - .rotate(rotationState) - .clickable { isExpanded = !isExpanded }, + .rotate(rotationState), )Also applies to: 127-133
feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailViewModel.kt (1)
306-313: FIXME 주석: endTime 계산 로직 후속 작업 필요
endTime = duration + 15.hours하드코딩에 FIXME가 있습니다. 실제 일정의 총 소요 시간을 기반으로 계산하도록 후속 구현이 필요합니다.이 작업을 추적하기 위한 이슈를 생성해 드릴까요?
mj010504
force-pushed
the
design/NDGL-49
branch
2 times, most recently
from
e6e34e3 to
1b0fc4a
Compare
mj010504
force-pushed
the
design/NDGL-53
branch
from
e6dc184 to
5667492
Compare
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 Fix all issues with AI agents
In `@app/build.gradle.kts`:
- Around line 12-18: The current defaultConfig block reads local.properties with
rootProject.file("local.properties").bufferedReader(), which throws if the file
is missing; update the logic around localProperties creation in defaultConfig so
it first checks for the file's existence (or uses safe file access/try-catch)
and only loads when present, otherwise fall back to an empty Properties
instance; then set manifestPlaceholders["MAPS_API_KEY"] using
localProperties.getProperty("MAPS_API_KEY", "") so builds in CI or fresh clones
won’t fail when local.properties is absent.
In `@core/ui/src/main/res/drawable/ic_20_check.xml`:
- Around line 1-5: The drawable filename ic_20_check does not match the vector
size (android:width/android:height set to 24dp); either rename the asset to
ic_24_check to match the 24dp dimensions or change the vector attributes
android:width and android:height to 20dp to match the filename; update any
references to the drawable if you rename the file so references to ic_20_check
are switched to ic_24_check, or if you change dimensions ensure the file name
remains ic_20_check and the viewport/paths still render correctly at 20dp.
In `@core/ui/src/main/res/values/strings.xml`:
- Line 33: The two string resources timeline_auto_setting and
timeline_auto_setting_title both contain the identical value "타임라인 자동생성";
confirm whether they were intentionally distinct and if not, consolidate them by
keeping one resource (pick a canonical name, e.g., timeline_auto_setting_title
or timeline_auto_setting), update all usages to reference that single resource,
and remove the duplicate entry to avoid redundant resources and maintain a
single source of truth.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/followtravel/component/ContentCard.kt`:
- Around line 80-88: The contentDescription values in ContentCard's AsyncImage
usages (e.g., the AsyncImage showing profileImage and other AsyncImage calls
around lines with "Expand" and "Video Thumbnail") are hardcoded English strings;
replace them with stringResource(...) references for localization or set
contentDescription = null for purely decorative images. Locate the AsyncImage
invocations in the ContentCard composable and update each contentDescription to
use the appropriate string resource key (add entries to strings.xml) or null
when decorative, ensuring accessibility and i18n support.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/followtravel/FollowTravelContract.kt`:
- Around line 19-70: Several core data models (ContentInfo, VideoInfo, Budget,
TravelPlace, PlaceType, TransportSegment, TransportType) and the
PlaceType.getColor() extension are duplicated between followtravel and
traveldetail; extract these into a single shared package under feature/travel
(e.g., common or model/contract) and replace the duplicated definitions by
importing the shared types. Specifically, create shared definitions for
ContentInfo, VideoInfo, Budget (with formatString), Itinerary, TravelPlace,
TransportSegment, TransportType and the PlaceType enum plus its getColor()
extension, then update all usages and imports in followtravel and traveldetail
to reference the new shared package and remove the old duplicated files. Ensure
public API and data shapes remain unchanged so callers (e.g., functions
referencing Budget.formatString or TravelPlace properties) compile without
further changes.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/followtravel/FollowTravelScreen.kt`:
- Around line 222-231: The Preview function FollowTravelScreenPreview is calling
FollowTravelScreen directly without providing the NDGLTheme CompositionLocal,
causing Preview rendering errors; wrap the FollowTravelScreen call with
NDGLTheme in FollowTravelScreenPreview so NDGLTheme.colors and
NDGLTheme.typography are available (mirror how TravelDetailScreen's preview is
wrapped), i.e., change FollowTravelScreenPreview to create an NDGLTheme {
FollowTravelScreen(...) } around the existing parameters.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/followtravel/FollowTravelViewModel.kt`:
- Around line 113-149: The mock Itinerary for day 2 has two TransportSegment
entries although there are only two TravelPlace items (so transport segments
should be N-1 = 1); update the Itinerary instance in FollowTravelViewModel (the
Itinerary with places containing TravelPlace id 8 and 9) to remove the extra
TransportSegment so transportSegments contains a single TransportSegment (e.g.,
the WALK or TRAIN segment you want to keep), ensuring the TransportSegment list
length matches places.size - 1.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/component/ContentCard.kt`:
- Around line 52-207: The ContentCard composable and its data models
(ContentInfo, VideoInfo, Budget, PlaceType, TravelPlace) are duplicated across
followtravel and traveldetail; consolidate by moving the ContentCard composable
and the shared models into a single shared module (e.g., core/ui or
feature/travel root) and update both packages to import the single source; also
replace hardcoded contentDescription literals in ContentCard ("Profile Image",
"Expand", "Video Thumbnail") with stringResource lookups (use string keys you
add to resources) so the AsyncImage and Icon contentDescription parameters use
stringResource(R.string.xxx) instead of raw strings.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/component/TimelineContent.kt`:
- Around line 80-93: The close icon Box (Box with Modifier.size(32.dp) ...
.padding(8.dp)) currently both hardcodes some contentDescriptions elsewhere and
causes the 24dp icon to be clipped because 32dp - 8dp*2 = 16dp inner area;
change any hardcoded contentDescription (e.g., "Profile Image") to use
stringResource(R.string.profile_image) and prevent clipping by increasing the
Box size to at least 40.dp and/or reducing padding (e.g., padding(4.dp)), and
explicitly set the Icon modifier to Modifier.size(24.dp) while keeping the
existing ImageVector.vectorResource(R.drawable.ic_24_close) and
stringResource(R.string.close) for the close button; update the Box/Icon
modifiers in TimelineContent.kt (the Box containing onDismissRequest and the
Icon call) accordingly.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailScreen.kt`:
- Around line 371-381: The NDGLModal in TravelDetailScreen is wired with
reversed actions: swap the callbacks so that positiveButtonText's handler calls
confirmCancelEditMode and negativeButtonText's handler calls
dismissCancelEditModal; locate the NDGLModal invocation guarded by
state.showCancelEditModal and update the onNegativeButtonClick and
onPositiveButtonClick assignments accordingly (keep titles/texts unchanged).
- Line 40: The import for hiltViewModel is using the deprecated path
androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel; update the import to
androidx.hilt.navigation.compose.hiltViewModel in this file
(TravelDetailScreen.kt) and the other affected screens (HomeScreen.kt,
TravelScreen.kt, FollowTravelScreen.kt, TravelEntry.kt, TravelHelperScreen.kt,
AuthScreen.kt) so usages of hiltViewModel (e.g., calls like val vm:
TravelDetailViewModel = hiltViewModel()) resolve to the supported package from
hilt-navigation-compose.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailViewModel.kt`:
- Around line 182-184: selectDay currently only updates selectedDay, so when
switching days in edit mode previously selectedPlaceIds from another day persist
and break checkSelectAll and confirmDeleteSelectedPlaces; modify selectDay to,
when isEditMode is true, also reset selectedPlaceIds (e.g., set to emptySet())
inside the same reduce call (update selectedDay and selectedPlaceIds together)
so switching days clears prior selections and prevents cross-day
deletion/selection bugs; reference selectDay, isEditMode, selectedPlaceIds,
reduce, copy, checkSelectAll, and confirmDeleteSelectedPlaces.
- Around line 215-229: The checkSelectAll function compares only sizes which
misdetects full-selection when selectedPlaceIds contains IDs from other days;
update checkSelectAll (in TravelDetailViewModel) to compare sets instead of
sizes by checking selectedPlaceIds.containsAll(currentDayPlaceIds) (or
selectedPlaceIds == currentDayPlaceIds) so the selection is true only when all
currentDayPlaceIds are included; keep current logic for choosing
currentItineraries (tempItineraries vs itineraries) and selectedDay as-is.
In `@gradle/libs.versions.toml`:
- Around line 45-51: Update the dependency versions in libs.versions.toml: set
mapsCompose = "8.1.0" and playServicesMaps = "20.0.0", and upgrade coil to
"3.3.0"; additionally, when moving Coil to 3.x update its Gradle coordinates to
the new group ID (io.coil-kt.coil3) wherever the Coil dependency is declared
(search for the symbol coil and replace the old group/artifact with the new
group for Coil 3.x).
🧹 Nitpick comments (33)
feature/travel-helper/src/main/java/com/yapp/ndgl/feature/travelhelper/TravelHelperScreen.kt (1)
15-21:viewModel이 주입만 되고 사용되지 않고 있습니다.현재
TravelHelperRoute에서viewModel을hiltViewModel()로 주입받고 있지만,TravelHelperScreen에 전달하거나 사용하는 곳이 없습니다. 향후 사용 예정이라면 괜찮지만, 현재 상태에서는 불필요한 의존성입니다.innerPadding전파 자체는 문제 없습니다.detekt-config.yml (1)
3-3:FIXME임시 변경이 영구적으로 남을 위험이 있습니다.
LongMethod임계값을 100에서 150으로 올린 것은#FIXME("임시 테스트용")이라고 명시되어 있지만, 되돌리기 위한 이슈 트래킹이 없으면 그대로 남을 가능성이 높습니다.Composable함수는 이미ignoreAnnotated로 제외되므로, 이 변경이 필요한 구체적인 원인을 확인해 주세요.
- 원인이 Composable이 아닌 함수라면 해당 함수를 리팩터링하는 것이 바람직합니다.
- 원인이 Composable 함수라면
ignoreAnnotated에 이미 포함되어 있으므로 임계값 변경이 불필요합니다.되돌리기를 보장하기 위해 관련 이슈를 생성하거나, 이 PR에서 원래 값(100)으로 복원하는 것을 권장합니다.
♻️ 원래 값 복원 제안
- threshold: 150 `#FIXME`("임시 테스트용") + threshold: 100core/ui/src/main/java/com/yapp/ndgl/core/ui/designsystem/NDGLBottomSheet.kt (1)
21-27:sheetState를 외부에서 주입할 수 있도록 파라미터로 노출하는 것을 권장합니다.현재
sheetState가 내부에서만 생성되어, 호출부에서sheetState.hide()등으로 프로그래밍 방식의 제어가 불가능합니다. 디자인 시스템 컴포넌트로서 재사용성을 높이려면 외부 주입을 허용하되, 기본값을 제공하는 패턴이 적합합니다.♻️ sheetState 파라미터 노출 제안
`@OptIn`(ExperimentalMaterial3Api::class) `@Composable` fun NDGLBottomSheet( modifier: Modifier = Modifier, onDismissRequest: () -> Unit, showDragHandle: Boolean = true, + sheetState: SheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true), content: `@Composable` ColumnScope.() -> Unit, ) { - val sheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true) - ModalBottomSheet(core/ui/src/main/res/drawable/ic_140_no_schedule_calendar.xml (1)
1-193: 복잡한 벡터 드로어블의 렌더링 성능 고려이 파일은 약 60개 이상의
<path>요소와 다수의<clip-path>를 포함한 매우 복잡한 벡터 드로어블입니다. 이런 고밀도 벡터는 저사양 기기에서 인플레이션 및 렌더링 시 성능 저하(jank)를 유발할 수 있습니다.일러스트레이션 스타일의 에셋은 WebP 또는 PNG (density별 제공) 포맷이 렌더링 성능 면에서 더 유리할 수 있습니다. 다만 현재 단일 해상도만 필요하고 empty state 등 빈번히 렌더되지 않는 곳에서만 사용된다면 현재 상태로도 괜찮습니다.
feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/component/TransportSegment.kt (2)
26-58: 컴포저블과 데이터 클래스의 이름 충돌
TransportSegment컴포저블 함수명이 파라미터 타입인TransportSegment데이터 클래스와 동일합니다. Kotlin 컴파일러가 구분할 수 있어 동작에는 문제 없지만, 코드 가독성 측면에서 혼동을 줄 수 있습니다. 예를 들어TransportSegmentRow등으로 구분하는 것을 고려해볼 수 있습니다.
31-36: clickable에 접근성 라벨 누락
clickablemodifier에onClickLabel이 없어 TalkBack 등 접근성 도구에서 이 요소의 동작을 설명하지 못합니다. 교통수단 세그먼트의 클릭 목적을 설명하는 라벨 추가를 권장합니다.core/ui/src/main/java/com/yapp/ndgl/core/ui/designsystem/NDGLCheckBox.kt (1)
27-39: 접근성(Accessibility):clickable대신toggleable을 사용하세요.현재
.clickable를 사용하고 있어 TalkBack 등 스크린 리더에서 이 컴포넌트가 체크박스임을 인식하지 못합니다.Modifier.toggleable에role = Role.Checkbox를 지정하면 접근성 서비스에 체크박스 상태가 올바르게 전달됩니다.♻️ 접근성 개선 제안
+import androidx.compose.ui.semantics.Role +import androidx.compose.foundation.selection.toggleableBox( modifier = Modifier .size(24.dp) .clip(RoundedCornerShape(4.dp)) .background( if (checked) NDGLTheme.colors.green500 else NDGLTheme.colors.white, ) .border( width = 1.25.dp, color = if (checked) Color.Unspecified else NDGLTheme.colors.black400, shape = RoundedCornerShape(4.dp), ) - .clickable { onClick() }, + .toggleable( + value = checked, + role = Role.Checkbox, + onValueChange = { onClick() }, + ), contentAlignment = Alignment.Center, ) {feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/component/TravelDetailToolBar.kt (1)
42-55: 최소 터치 영역 확보를 위한 개선 제안
clickable가Row에 직접 적용되어 있어, 콘텐츠 크기에 따라 터치 영역이 Material 권장 최소 크기(48dp)보다 작을 수 있습니다. 동일한 패턴이EditTravelButton에도 적용됩니다.Modifier.minimumInteractiveComponentSize()를 추가하면 접근성이 향상됩니다.feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailViewModel.kt (2)
306-313:endTime하드코딩 FIXME 확인
duration + 15.hours가 하드코딩되어 있습니다. FIXME 주석이 있으므로 인지하고 계신 것으로 보이며, 실제 일정 기반 계산으로 대체가 필요합니다.이 작업에 대한 GitHub 이슈를 생성해드릴까요?
243-267: 삭제 후 교통수단 처리가 불완전Line 253-254에서
transportSegments를 단순히take(remainingPlaces.size - 1)로 잘라내고 있습니다. 장소가 중간에서 삭제되면, 남은 transport segment의 순서가 실제 장소 간 경로와 불일치할 수 있습니다. TODO 주석으로 인지하고 계신 부분이지만, 추후 반드시 올바른 매핑이 필요합니다.이 작업에 대한 GitHub 이슈를 생성해드릴까요?
feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/component/TravelMap.kt (1)
89-99:clip(RoundedCornerShape)GoogleMap에 시각적으로 적용되지 않을 수 있음GoogleMap은 내부적으로
SurfaceView또는TextureView를 사용하는데,SurfaceView는 Compose의clipmodifier를 지원하지 않습니다. 현재 Maps SDK가 사용하는 렌더링 방식(TextureView 또는 SurfaceView)에 따라 둥근 모서리가 실제 디바이스에서 표시되지 않을 수 있습니다. 시각적으로 확인한 후 필요하면 오버레이를 이용한 마스킹 방식을 고려하세요.feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/component/TimelineContent.kt (2)
97-151: 시간 설정 모드에서 매직 넘버15.hours가 사용됩니다.Line 149의
selectedStartTime + 15.hours에서 15시간이 하드코딩되어 있습니다. 이 값의 의미(예: 기본 일정 길이)를 상수로 추출하면 가독성과 유지보수성이 향상됩니다.♻️ 제안
+private val DEFAULT_ITINERARY_DURATION = 15.hours + // Line 149 내부: - selectedEndTime = selectedStartTime + 15.hours + selectedEndTime = selectedStartTime + DEFAULT_ITINERARY_DURATION
270-313: WheelPicker의 접근성(Accessibility) 지원이 부족합니다.현재 WheelPicker는
contentDescription이나semantics정보가 없어 TalkBack 사용자가 시간 값을 인식하기 어렵습니다. 접근성 개선을 위해 선택된 항목에semantics블록을 추가하는 것을 고려해 주세요.feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/component/ContentCard.kt (2)
80-88:contentDescription에 하드코딩된 영어 문자열 대신 문자열 리소스를 사용해 주세요.Line 82
"Profile Image", Line 129"Expand", Line 146"Video Thumbnail"등이 하드코딩되어 있습니다. 접근성 및 다국어 지원을 위해stringResource()를 사용하거나, 장식용 이미지라면null로 설정하는 것이 좋습니다.♻️ 제안
AsyncImage( model = contentInfo.videoInfo.profileImage, - contentDescription = "Profile Image", + contentDescription = null, // 장식용 이미지 ... Icon( imageVector = ImageVector.vectorResource(R.drawable.ic_28_chevron_down), - contentDescription = "Expand", + contentDescription = null, // 접근성: Row의 clickable에서 처리 ... AsyncImage( model = contentInfo.videoInfo.thumbnail, - contentDescription = "Video Thumbnail", + contentDescription = null, // 장식용 이미지Also applies to: 144-153, 127-133
209-232: Preview 함수의 가시성을private로 변경하세요.
PreviewContentCard()가public으로 선언되어 있지만, Preview 함수는 외부에서 사용할 필요가 없으므로private로 제한하는 것이 일반적입니다.traveldetail패키지의PlaceItem.kt에서는 이미private가시성을 사용하고 있어 일관성이 떨어집니다.♻️ 제안
`@Preview` `@Composable` -fun PreviewContentCard() { +private fun PreviewContentCard() {feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/component/PlaceItem.kt (3)
74-116:EditablePlaceItem의modifier파라미터가 루트 컴포저블이 아닌 내부Icon에 적용됩니다.Compose 컨벤션에 따르면
modifier파라미터는 최상위 컴포저블에 적용되어야 합니다. 현재 Line 82에서 Box는 새로운Modifier를 사용하고, 전달받은modifier는 Line 109의 드래그 핸들Icon에만 적용됩니다. 드래그 핸들용 modifier가 필요하다면 별도 파라미터(예:dragModifier)로 분리하세요.♻️ 제안
`@Composable` internal fun EditablePlaceItem( - modifier: Modifier = Modifier, + dragHandleModifier: Modifier = Modifier, place: TravelPlace, checked: Boolean, onCheck: () -> Unit, ) { Box( - modifier = Modifier + modifier = Modifier // 또는 외부 modifier 파라미터를 추가하여 적용 .fillMaxWidth() .padding(horizontal = 24.dp), ) { // ... Icon( - modifier = modifier, + modifier = dragHandleModifier, imageVector = ImageVector.vectorResource(R.drawable.ic_24_menu),
86-96: 불필요한.padding(horizontal = 0.dp)제거Line 93의
.padding(horizontal = 0.dp)는 아무 효과가 없으므로 제거해 주세요.♻️ 제안
.background( color = NDGLTheme.colors.white, shape = RoundedCornerShape(16.dp), ) - .padding(horizontal = 0.dp), + ,
188-196:contentDescription이 하드코딩된 영어 문자열입니다.Line 190의
"Place Thumbnail"은 리소스 문자열을 사용하거나 장식용 이미지이면null로 설정하세요.feature/travel/src/main/java/com/yapp/ndgl/feature/travel/followtravel/FollowTravelContract.kt (2)
91-109: FIXME 주석 — API 미제작 상태로 표시되어 있습니다.
TransportSegment에 FIXME 주석이 있습니다. 이 클래스의 API가 확정되면 업데이트가 필요합니다. 추적을 위해 이슈를 생성해 드릴까요?
97-109:formatDistance()의 부동소수점 비교가 잠재적으로 불안정합니다.Line 101의
km % 1 == 0.0은 부동소수점 정밀도 문제가 발생할 수 있습니다.distance가Int이므로 정수 나눗셈으로 처리하면 더 안전합니다.♻️ 제안
fun formatDistance(): String { return when { distance >= 1000 -> { - val km = distance / 1000.0 - if (km % 1 == 0.0) { - "${km.toInt()}km" + if (distance % 1000 == 0) { + "${distance / 1000}km" } else { + val km = distance / 1000.0 String.format(java.util.Locale.getDefault(), "%.1fkm", km) } } else -> "${distance}m" } }feature/travel/src/main/java/com/yapp/ndgl/feature/travel/followtravel/component/TransportSegment.kt (1)
25-47: Composable 함수명이 데이터 클래스명과 동일하여 혼동의 소지가 있습니다.
TransportSegmentComposable 함수(Line 26)와TransportSegment데이터 클래스(import, Line 21)가 같은 이름을 사용합니다. 현재는 패키지로 구분되지만, import 충돌이나 가독성 저하가 우려됩니다. Composable 함수명을TransportSegmentItem등으로 변경하는 것을 고려해 주세요.feature/travel/src/main/java/com/yapp/ndgl/feature/travel/followtravel/component/PlaceItem.kt (1)
89-150:PlaceCard가public으로 노출되어 있습니다.
followtravel패키지의PlaceCard는public가시성인 반면,traveldetail패키지의 동일 컴포넌트는private입니다. 외부 모듈에서의 의도치 않은 사용을 방지하려면internal로 제한하세요.또한 이 컴포넌트는
traveldetail/component/PlaceCard와 거의 동일합니다. 앞서FollowTravelContract.kt에서 언급한 대로 공통 모듈로 통합하면 중복을 해소할 수 있습니다.♻️ 제안
`@Composable` -fun PlaceCard( +internal fun PlaceCard( place: TravelPlace, interactionSource: MutableInteractionSource = remember { MutableInteractionSource() }, ) {core/ui/src/main/res/values/strings.xml (1)
3-5: 포맷 문자열에 위치 인자(positional arguments)를 사용해 주세요.Line 3
%d/%s, Line 4%s/%d등이 위치 인자 없이 사용됩니다. 다국어 지원 시 번역가가 인자 순서를 변경해야 할 수 있으므로, Line 11의%1$s/%2$s처럼 위치 인자를 일관되게 사용하는 것이 좋습니다.♻️ 제안
- <string name="budget_bar_format">%d일차 여행 예산: %s</string> - <string name="content_card_info_format">%s • %s • %d박%d일</string> - <string name="content_card_budget_format">1인 기준 여행 예산 : %s</string> + <string name="budget_bar_format">%1$d일차 여행 예산: %2$s</string> + <string name="content_card_info_format">%1$s • %2$s • %3$d박%4$d일</string> + <string name="content_card_budget_format">1인 기준 여행 예산 : %1$s</string>feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/component/EditControlBar.kt (1)
33-56: 클릭 가능한 텍스트 요소에 접근성 및 터치 영역 개선을 고려해 주세요.
Text에.clickable만 적용하면 기본 ripple가 텍스트 영역에만 나타나고, 터치 영역이 작아 사용성이 떨어질 수 있습니다. "전체 선택" 영역은 체크박스와 텍스트를 하나의 클릭 영역으로 묶고, "선택 삭제"에는minimumInteractiveComponentSize를 적용하면 더 나은 UX를 제공할 수 있습니다.♻️ 제안: 체크박스와 텍스트를 하나의 클릭 영역으로 통합
- Row(modifier = Modifier.fillMaxWidth(), verticalAlignment = Alignment.CenterVertically) { - NDGLCheckbox( - checked = isAllSelected, - onClick = onSelectAllClick, - ) - Spacer(Modifier.width(8.dp)) - Text( - text = stringResource(R.string.select_all), - color = NDGLTheme.colors.black700, - style = NDGLTheme.typography.bodyMdMedium, - modifier = Modifier.clickable { - onSelectAllClick() - }, - ) + Row( + modifier = Modifier + .fillMaxWidth() + .clickable { onSelectAllClick() }, + verticalAlignment = Alignment.CenterVertically, + ) { + NDGLCheckbox( + checked = isAllSelected, + onClick = onSelectAllClick, + ) + Spacer(Modifier.width(8.dp)) + Text( + text = stringResource(R.string.select_all), + color = NDGLTheme.colors.black700, + style = NDGLTheme.typography.bodyMdMedium, + )feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailScreen.kt (2)
262-299: 모든 장소가 단일 LazyColumn item 내에서 렌더링됩니다.
currentPlaces.forEachIndexed가item(key = "places_...")블록 안에 있어 모든 장소가 한 번에 compose됩니다. 장소 수가 많아지면 성능에 영향을 줄 수 있습니다. 각 장소를 개별item으로 분리하면 LazyColumn의 lazy 렌더링 이점을 활용할 수 있습니다. (FollowTravelScreen에서는items/item을 개별적으로 사용하고 있습니다.)
127-132:persistentListOf생성 방식이 FollowTravelScreen과 다릅니다.여기서는
.let { persistentListOf(*it.toTypedArray()) }를 사용하고,FollowTravelScreen에서는.toPersistentList()를 사용하고 있습니다. 일관성을 위해 동일한 방식으로 통일하는 것을 권장합니다.♻️ 제안
val tabs = (1..state.contentInfo.days).map { day -> NDGLChipTabAttr.Tab( tag = "d$day", name = stringResource(R.string.day_format, day), ) - }.let { persistentListOf(*it.toTypedArray()) } + }.toPersistentList()feature/travel/src/main/java/com/yapp/ndgl/feature/travel/followtravel/FollowTravelScreen.kt (1)
46-61: 뒤로가기 버튼이 ViewModel Intent를 거치지 않고 직접navigateBack을 호출합니다.
TravelDetailRoute에서는clickBack이viewModel.onIntent(TravelDetailIntent.ClickBack)을 거쳐 ViewModel에서 사이드이펙트로 네비게이션을 처리합니다. 반면FollowTravelRoute에서는clickBackButton = navigateBack으로 직접 연결되어 있습니다. 현재는 편집 모드가 없으므로 문제없지만, 향후 FollowTravel에서도 뒤로가기 전 확인이 필요해질 경우 ViewModel을 거치는 패턴이 더 확장성이 좋습니다. 또한 Line 53의collectSideEffectTODO가 이 부분과 관련됩니다.feature/travel/src/main/java/com/yapp/ndgl/feature/travel/followtravel/component/ContentCard.kt (2)
52-53:ContentCard의 가시성을internal로 제한하는 것을 고려해 주세요.같은 패키지의 다른 컴포넌트(
TransportSegment,PlaceItem등)는internal로 선언되어 있습니다.ContentCard도 feature 모듈 외부에서 사용되지 않는다면internal로 통일하는 것이 좋습니다.#!/bin/bash # ContentCard의 모듈 외부 사용 여부 확인 rg -n "ContentCard" --type=kt -g '!**/followtravel/**'
209-211: Preview 함수의 가시성을private으로 변경해 주세요.
PreviewContent()가public으로 선언되어 있어 모듈 외부에서 접근 가능합니다. 다른 파일의 Preview 함수들은 모두private으로 선언되어 있습니다.♻️ 수정 제안
`@Preview` `@Composable` -fun PreviewContent() { +private fun ContentCardPreview() {feature/travel/src/main/java/com/yapp/ndgl/feature/travel/followtravel/FollowTravelViewModel.kt (1)
10-14: 테스트 데이터 제거 TODO가 남아 있습니다.
TEST_PROFILE_IMAGE_URL과TEST_THUMBNAIL_URL상수와loadTravelData()의 모든 mock 데이터가 릴리스 전에 제거되어야 합니다. 이 TODO를 추적하기 위해 이슈를 생성해 드릴까요?feature/travel/src/main/java/com/yapp/ndgl/feature/travel/followtravel/component/TravelMap.kt (1)
58-61:places가 빈 리스트일 경우places.first()에서NoSuchElementException이 발생합니다.현재 호출부(
FollowTravelScreen)에서places.isNotEmpty()체크 후 호출하므로 당장 문제는 없지만, 컴포넌트 자체적으로 방어 코드를 추가하면 향후 다른 곳에서 재사용 시 안전합니다.🛡️ 제안: early return 추가
`@Composable` internal fun TravelMap( places: List<TravelPlace>, onScrollEnabledChange: (Boolean) -> Unit, ) { + if (places.isEmpty()) return + val routePoints = remember(places) {feature/travel/src/main/java/com/yapp/ndgl/feature/travel/travel/TravelScreen.kt (2)
41-46:innerPadding에 기본값 추가를 권장합니다.다른 파라미터(
state,clickTravel,clickTravelDetail)는 모두 기본값이 있지만innerPadding만 기본값이 없어 불일치합니다.PaddingValues()를 기본값으로 지정하면 Preview 호출 시에도 더 간결해지고, 파라미터 간 일관성이 유지됩니다.♻️ 제안된 변경
`@Composable` private fun TravelScreen( state: TravelState = TravelState(), clickTravel: (Int) -> Unit = {}, clickTravelDetail: (Int) -> Unit = {}, - innerPadding: PaddingValues, + innerPadding: PaddingValues = PaddingValues(), ) {
59-74: 하드코딩된 travelId(123, 456)는 임시 값으로 보입니다.PR 설명에서 테스트용 버튼으로 언급되어 있어 현재 단계에서는 문제없지만, 실제 데이터 연동 시 제거/교체가 필요합니다. TODO 주석을 남겨두면 추후 정리 시 식별이 쉬워집니다.
| defaultConfig { | ||
| val localProperties = Properties().apply { | ||
| load(rootProject.file("local.properties").bufferedReader()) | ||
| } | ||
|
|
||
| manifestPlaceholders["MAPS_API_KEY"] = localProperties.getProperty("MAPS_API_KEY", "") | ||
| } |
There was a problem hiding this comment.
local.properties 파일 부재 시 빌드 실패 가능성
rootProject.file("local.properties").bufferedReader()는 파일이 없으면 FileNotFoundException을 발생시킵니다. CI 환경이나 fresh clone에서 빌드가 실패할 수 있습니다.
🛡️ 방어 코드 제안
defaultConfig {
- val localProperties = Properties().apply {
- load(rootProject.file("local.properties").bufferedReader())
- }
+ val localPropertiesFile = rootProject.file("local.properties")
+ val localProperties = Properties().apply {
+ if (localPropertiesFile.exists()) {
+ load(localPropertiesFile.bufferedReader())
+ }
+ }
manifestPlaceholders["MAPS_API_KEY"] = localProperties.getProperty("MAPS_API_KEY", "")
}📝 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.
| defaultConfig { | |
| val localProperties = Properties().apply { | |
| load(rootProject.file("local.properties").bufferedReader()) | |
| } | |
| manifestPlaceholders["MAPS_API_KEY"] = localProperties.getProperty("MAPS_API_KEY", "") | |
| } | |
| defaultConfig { | |
| val localPropertiesFile = rootProject.file("local.properties") | |
| val localProperties = Properties().apply { | |
| if (localPropertiesFile.exists()) { | |
| load(localPropertiesFile.bufferedReader()) | |
| } | |
| } | |
| manifestPlaceholders["MAPS_API_KEY"] = localProperties.getProperty("MAPS_API_KEY", "") | |
| } |
🤖 Prompt for AI Agents
In `@app/build.gradle.kts` around lines 12 - 18, The current defaultConfig block
reads local.properties with
rootProject.file("local.properties").bufferedReader(), which throws if the file
is missing; update the logic around localProperties creation in defaultConfig so
it first checks for the file's existence (or uses safe file access/try-catch)
and only loads when present, otherwise fall back to an empty Properties
instance; then set manifestPlaceholders["MAPS_API_KEY"] using
localProperties.getProperty("MAPS_API_KEY", "") so builds in CI or fresh clones
won’t fail when local.properties is absent.
| <vector xmlns:android="http://schemas.android.com/apk/res/android" | ||
| android:width="24dp" | ||
| android:height="24dp" | ||
| android:viewportWidth="24" | ||
| android:viewportHeight="24"> |
There was a problem hiding this comment.
파일명과 실제 벡터 크기가 불일치합니다.
파일명은 ic_20_check(20dp 암시)이지만, 실제 android:width/android:height는 24dp로 설정되어 있습니다. 다른 아이콘들(ic_24_*)과의 네이밍 컨벤션 일관성을 위해 파일명을 ic_24_check로 변경하거나, 실제로 20dp 크기를 의도했다면 width/height를 20dp로 수정해 주세요.
🤖 Prompt for AI Agents
In `@core/ui/src/main/res/drawable/ic_20_check.xml` around lines 1 - 5, The
drawable filename ic_20_check does not match the vector size
(android:width/android:height set to 24dp); either rename the asset to
ic_24_check to match the 24dp dimensions or change the vector attributes
android:width and android:height to 20dp to match the filename; update any
references to the drawable if you rename the file so references to ic_20_check
are switched to ic_24_check, or if you change dimensions ensure the file name
remains ic_20_check and the viewport/paths still render correctly at 20dp.
|
|
||
|
|
||
| <!-- Travel Detail --> | ||
| <string name="timeline_auto_setting">타임라인 자동생성</string> |
There was a problem hiding this comment.
중복 문자열 리소스: timeline_auto_setting과 timeline_auto_setting_title
Line 33의 timeline_auto_setting과 Line 54의 timeline_auto_setting_title이 동일한 값 "타임라인 자동생성"을 가지고 있습니다. 의도적으로 분리한 것인지 확인해 주세요. 동일 용도라면 하나로 통합하는 것이 좋습니다.
Also applies to: 54-54
🤖 Prompt for AI Agents
In `@core/ui/src/main/res/values/strings.xml` at line 33, The two string resources
timeline_auto_setting and timeline_auto_setting_title both contain the identical
value "타임라인 자동생성"; confirm whether they were intentionally distinct and if not,
consolidate them by keeping one resource (pick a canonical name, e.g.,
timeline_auto_setting_title or timeline_auto_setting), update all usages to
reference that single resource, and remove the duplicate entry to avoid
redundant resources and maintain a single source of truth.
| AsyncImage( | ||
| model = contentInfo.videoInfo.profileImage, | ||
| contentDescription = "Profile Image", | ||
| modifier = Modifier | ||
| .size(48.dp) | ||
| .clip(CircleShape) | ||
| .background(Color.LightGray), | ||
| contentScale = ContentScale.Crop, | ||
| ) |
There was a problem hiding this comment.
contentDescription에 하드코딩된 영문 문자열 대신 stringResource를 사용해 주세요.
"Profile Image", "Expand", "Video Thumbnail" 등이 하드코딩되어 있습니다. 접근성과 다국어 지원을 위해 stringResource를 사용하거나, 장식용 이미지라면 null로 설정하는 것이 좋습니다.
Also applies to: 127-133, 144-153
🤖 Prompt for AI Agents
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/followtravel/component/ContentCard.kt`
around lines 80 - 88, The contentDescription values in ContentCard's AsyncImage
usages (e.g., the AsyncImage showing profileImage and other AsyncImage calls
around lines with "Expand" and "Video Thumbnail") are hardcoded English strings;
replace them with stringResource(...) references for localization or set
contentDescription = null for purely decorative images. Locate the AsyncImage
invocations in the ContentCard composable and update each contentDescription to
use the appropriate string resource key (add entries to strings.xml) or null
when decorative, ensuring accessibility and i18n support.
| data class ContentInfo( | ||
| val travelId: String = "", | ||
| val country: String = "", | ||
| val city: String = "", | ||
| val budgetPerPerson: Budget = Budget(0), | ||
| val nights: Int = 0, | ||
| val days: Int = 0, | ||
| val videoInfo: VideoInfo = VideoInfo(), | ||
| ) | ||
|
|
||
| data class VideoInfo( | ||
| val title: String = "", | ||
| val name: String = "", | ||
| val profileImage: String = "", | ||
| val thumbnail: String = "", | ||
| val link: String = "", | ||
| val summary: String = "", | ||
| ) | ||
|
|
||
| data class Budget( | ||
| val amount: Int, | ||
| ) { | ||
| fun formatString(): String { | ||
| return when { | ||
| amount < 10000 -> "만원" | ||
| amount % 10000 == 0 -> "${amount / 10000}만원" | ||
| amount % 1000 == 0 -> "${amount / 10000}만 ${(amount % 10000) / 1000}천원" | ||
| else -> "${amount}원" | ||
| } | ||
| } | ||
| } | ||
|
|
||
| data class Itinerary( | ||
| val budget: Budget = Budget(0), | ||
| val places: List<TravelPlace> = emptyList(), | ||
| val transportSegments: List<TransportSegment> = emptyList(), | ||
| ) | ||
|
|
||
| data class TravelPlace( | ||
| val id: Int, | ||
| val day: Int, | ||
| val sequence: Int, | ||
| val estimatedDuration: Duration, | ||
| val googlePlaceId: String, | ||
| val thumbnail: String, | ||
| val latitude: Double, | ||
| val longitude: Double, | ||
| val name: String, | ||
| val regularOpeningHours: String, | ||
| val googleMapsUri: String, | ||
| val placeType: PlaceType, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify duplication of data models between followtravel and traveldetail packages
echo "=== Budget class definitions ==="
rg -n "^data class Budget" --type kotlin
echo ""
echo "=== ContentInfo class definitions ==="
rg -n "^data class ContentInfo" --type kotlin
echo ""
echo "=== VideoInfo class definitions ==="
rg -n "^data class VideoInfo" --type kotlin
echo ""
echo "=== TravelPlace class definitions ==="
rg -n "^data class TravelPlace" --type kotlin
echo ""
echo "=== PlaceType enum definitions ==="
rg -n "^enum class PlaceType" --type kotlin
echo ""
echo "=== TransportSegment definitions ==="
rg -n "^data class TransportSegment" --type kotlin
echo ""
echo "=== TransportType definitions ==="
rg -n "^enum class TransportType" --type kotlin
echo ""
echo "=== Checking imports in ContentCard.kt ==="
fd "ContentCard.kt" --type f -x cat -n {}
echo ""
echo "=== Checking imports in PlaceItem.kt ==="
fd "PlaceItem.kt" --type f -x cat -n {}Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 44768
🏁 Script executed:
#!/bin/bash
# Check for differences between duplicated class definitions
echo "=== Comparing Budget classes ==="
diff <(sed -n '38,46p' feature/travel/src/main/java/com/yapp/ndgl/feature/travel/followtravel/FollowTravelContract.kt) \
<(sed -n '49,57p' feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailContract.kt) || echo "Differences found or files not identical"
echo ""
echo "=== Comparing PlaceType enums ==="
diff <(sed -n '72,90p' feature/travel/src/main/java/com/yapp/ndgl/feature/travel/followtravel/FollowTravelContract.kt) \
<(sed -n '83,101p' feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailContract.kt) || echo "Differences found or files not identical"Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 421
🏁 Script executed:
#!/bin/bash
# Read and compare the actual class definitions
echo "=== FollowTravelContract.kt Budget ==="
sed -n '38,46p' feature/travel/src/main/java/com/yapp/ndgl/feature/travel/followtravel/FollowTravelContract.kt
echo ""
echo "=== TravelDetailContract.kt Budget ==="
sed -n '49,57p' feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailContract.kt
echo ""
echo "=== FollowTravelContract.kt PlaceType ==="
sed -n '72,90p' feature/travel/src/main/java/com/yapp/ndgl/feature/travel/followtravel/FollowTravelContract.kt
echo ""
echo "=== TravelDetailContract.kt PlaceType ==="
sed -n '83,101p' feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailContract.kt
echo ""
echo "=== Checking for getColor() function in followtravel ==="
rg -n "fun PlaceType.getColor" feature/travel/src/main/java/com/yapp/ndgl/feature/travel/followtravel/ -A 5
echo ""
echo "=== Checking for getColor() function in traveldetail ==="
rg -n "fun PlaceType.getColor" feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/ -A 5Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 4425
followtravel과 traveldetail 패키지 간 데이터 모델 중복 정의 제거 필요
ContentInfo, VideoInfo, Budget, TravelPlace, PlaceType, TransportSegment, TransportType 등 핵심 데이터 모델이 두 패키지에 각각 정의되어 있습니다. 특히 PlaceType.getColor() 확장 함수도 동일한 로직으로 중복 구현되어 있습니다.
동일한 모델이 분산되어 있으면 향후 비즈니스 로직 변경 시 두 곳을 모두 수정해야 하는 유지보수 비용이 발생합니다. 공통 데이터 모델을 feature/travel 루트의 공유 패키지(예: common, model, contract)로 추출하여 단일 소스로 관리하기를 권장합니다.
🤖 Prompt for AI Agents
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/followtravel/FollowTravelContract.kt`
around lines 19 - 70, Several core data models (ContentInfo, VideoInfo, Budget,
TravelPlace, PlaceType, TransportSegment, TransportType) and the
PlaceType.getColor() extension are duplicated between followtravel and
traveldetail; extract these into a single shared package under feature/travel
(e.g., common or model/contract) and replace the duplicated definitions by
importing the shared types. Specifically, create shared definitions for
ContentInfo, VideoInfo, Budget (with formatString), Itinerary, TravelPlace,
TransportSegment, TransportType and the PlaceType enum plus its getColor()
extension, then update all usages and imports in followtravel and traveldetail
to reference the new shared package and remove the old duplicated files. Ensure
public API and data shapes remain unchanged so callers (e.g., functions
referencing Budget.formatString or TravelPlace properties) compile without
further changes.
| import androidx.compose.ui.res.vectorResource | ||
| import androidx.compose.ui.tooling.preview.Preview | ||
| import androidx.compose.ui.unit.dp | ||
| import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for hiltViewModel imports in Kotlin files
rg -n "import.*hiltViewModel" --type=ktRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 105
🏁 Script executed:
#!/bin/bash
# Check Hilt dependency version in gradle files
find . -name "*.toml" -o -name "build.gradle*" | head -20 | xargs grep -l -i "hilt" 2>/dev/nullRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 125
🏁 Script executed:
#!/bin/bash
# Check the actual Hilt version configuration
rg -A 2 -B 2 "hilt" gradle/libs.versions.toml 2>/dev/null || rg -A 2 -B 2 "hilt" build.gradle.kts 2>/dev/null || rg -A 2 -B 2 "hilt" build.gradle 2>/dev/nullRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 874
🏁 Script executed:
#!/bin/bash
# Search for hiltViewModel imports without file type restriction
rg -n "import.*hiltViewModel"Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 1119
🏁 Script executed:
#!/bin/bash
# Check the specific file mentioned in the review
cat -n "feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailScreen.kt" | head -50Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 2832
hiltViewModel import 경로를 수정해 주세요.
이 파일을 포함하여 프로젝트 전체에서 deprecated된 import 경로 androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel을 사용 중입니다. 프로젝트가 보유한 Hilt 2.57.2 및 hilt-navigation-compose 1.3.0 버전에서는 이 import 경로가 더 이상 제공되지 않으므로, 반드시 androidx.hilt.navigation.compose.hiltViewModel로 변경해야 합니다.
영향을 받는 파일들:
- feature/home/src/main/java/com/yapp/ndgl/feature/home/HomeScreen.kt
- feature/travel/src/main/java/com/yapp/ndgl/feature/travel/travel/TravelScreen.kt
- feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailScreen.kt
- feature/travel/src/main/java/com/yapp/ndgl/feature/travel/followtravel/FollowTravelScreen.kt
- feature/travel/src/main/java/com/yapp/ndgl/feature/travel/navigation/TravelEntry.kt
- feature/travel-helper/src/main/java/com/yapp/ndgl/feature/travelhelper/TravelHelperScreen.kt
- feature/auth/src/main/java/com/yapp/ndgl/feature/auth/AuthScreen.kt
🤖 Prompt for AI Agents
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailScreen.kt`
at line 40, The import for hiltViewModel is using the deprecated path
androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel; update the import to
androidx.hilt.navigation.compose.hiltViewModel in this file
(TravelDetailScreen.kt) and the other affected screens (HomeScreen.kt,
TravelScreen.kt, FollowTravelScreen.kt, TravelEntry.kt, TravelHelperScreen.kt,
AuthScreen.kt) so usages of hiltViewModel (e.g., calls like val vm:
TravelDetailViewModel = hiltViewModel()) resolve to the supported package from
hilt-navigation-compose.
| if (state.showCancelEditModal) { | ||
| NDGLModal( | ||
| onDismissRequest = dismissCancelEditModal, | ||
| title = stringResource(R.string.cancel_edit_dialog_title), | ||
| body = stringResource(R.string.cancel_edit_dialog_message), | ||
| negativeButtonText = stringResource(R.string.cancel_edit_dialog_cancel), | ||
| onNegativeButtonClick = confirmCancelEditMode, | ||
| positiveButtonText = stringResource(R.string.cancel_edit_dialog_confirm), | ||
| onPositiveButtonClick = dismissCancelEditModal, | ||
| ) | ||
| } |
There was a problem hiding this comment.
편집 취소 모달의 버튼 액션이 뒤바뀌어 있습니다.
negativeButtonText(취소 버튼)에 confirmCancelEditMode가, positiveButtonText(확인 버튼)에 dismissCancelEditModal이 연결되어 있습니다. "편집을 취소하시겠습니까?" 다이얼로그에서 "확인"을 누르면 편집 모드에 머무르고, "취소"를 누르면 편집이 취소되는 역전된 동작이 발생합니다.
🐛 수정 제안
if (state.showCancelEditModal) {
NDGLModal(
onDismissRequest = dismissCancelEditModal,
title = stringResource(R.string.cancel_edit_dialog_title),
body = stringResource(R.string.cancel_edit_dialog_message),
negativeButtonText = stringResource(R.string.cancel_edit_dialog_cancel),
- onNegativeButtonClick = confirmCancelEditMode,
+ onNegativeButtonClick = dismissCancelEditModal,
positiveButtonText = stringResource(R.string.cancel_edit_dialog_confirm),
- onPositiveButtonClick = dismissCancelEditModal,
+ onPositiveButtonClick = confirmCancelEditMode,
)
}📝 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.
| if (state.showCancelEditModal) { | |
| NDGLModal( | |
| onDismissRequest = dismissCancelEditModal, | |
| title = stringResource(R.string.cancel_edit_dialog_title), | |
| body = stringResource(R.string.cancel_edit_dialog_message), | |
| negativeButtonText = stringResource(R.string.cancel_edit_dialog_cancel), | |
| onNegativeButtonClick = confirmCancelEditMode, | |
| positiveButtonText = stringResource(R.string.cancel_edit_dialog_confirm), | |
| onPositiveButtonClick = dismissCancelEditModal, | |
| ) | |
| } | |
| if (state.showCancelEditModal) { | |
| NDGLModal( | |
| onDismissRequest = dismissCancelEditModal, | |
| title = stringResource(R.string.cancel_edit_dialog_title), | |
| body = stringResource(R.string.cancel_edit_dialog_message), | |
| negativeButtonText = stringResource(R.string.cancel_edit_dialog_cancel), | |
| onNegativeButtonClick = dismissCancelEditModal, | |
| positiveButtonText = stringResource(R.string.cancel_edit_dialog_confirm), | |
| onPositiveButtonClick = confirmCancelEditMode, | |
| ) | |
| } |
🤖 Prompt for AI Agents
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailScreen.kt`
around lines 371 - 381, The NDGLModal in TravelDetailScreen is wired with
reversed actions: swap the callbacks so that positiveButtonText's handler calls
confirmCancelEditMode and negativeButtonText's handler calls
dismissCancelEditModal; locate the NDGLModal invocation guarded by
state.showCancelEditModal and update the onNegativeButtonClick and
onPositiveButtonClick assignments accordingly (keep titles/texts unchanged).
| private fun selectDay(day: Int) { | ||
| reduce { copy(selectedDay = day) } | ||
| } |
There was a problem hiding this comment.
버그: 편집 모드에서 날짜 전환 시 selectedPlaceIds가 초기화되지 않음
selectDay가 isEditMode 상태에서 selectedPlaceIds를 초기화하지 않아, 다른 날짜의 장소 ID가 남아있게 됩니다. 이로 인해 다음 함수들이 오동작합니다:
checkSelectAll(Line 222):selectedPlaceIds.size에 다른 날짜의 ID가 포함되어 전체 선택/해제 토글 로직이 깨집니다.confirmDeleteSelectedPlaces(Line 245-247): 모든 itinerary에서selectedPlaceIds기반으로 삭제하므로, 이전 날짜에서 선택된 장소까지 삭제될 수 있습니다.
🐛 수정 제안: 날짜 전환 시 선택 초기화
private fun selectDay(day: Int) {
- reduce { copy(selectedDay = day) }
+ reduce {
+ copy(
+ selectedDay = day,
+ selectedPlaceIds = emptySet(),
+ )
+ }
}📝 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.
| private fun selectDay(day: Int) { | |
| reduce { copy(selectedDay = day) } | |
| } | |
| private fun selectDay(day: Int) { | |
| reduce { | |
| copy( | |
| selectedDay = day, | |
| selectedPlaceIds = emptySet(), | |
| ) | |
| } | |
| } |
🤖 Prompt for AI Agents
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailViewModel.kt`
around lines 182 - 184, selectDay currently only updates selectedDay, so when
switching days in edit mode previously selectedPlaceIds from another day persist
and break checkSelectAll and confirmDeleteSelectedPlaces; modify selectDay to,
when isEditMode is true, also reset selectedPlaceIds (e.g., set to emptySet())
inside the same reduce call (update selectedDay and selectedPlaceIds together)
so switching days clears prior selections and prevents cross-day
deletion/selection bugs; reference selectDay, isEditMode, selectedPlaceIds,
reduce, copy, checkSelectAll, and confirmDeleteSelectedPlaces.
| private fun checkSelectAll() { | ||
| reduce { | ||
| val currentItineraries = if (isEditMode) tempItineraries else itineraries | ||
| val currentDayPlaceIds = currentItineraries.getOrNull(selectedDay - 1) | ||
| ?.places?.map { it.id }?.toSet() ?: emptySet() | ||
|
|
||
| copy( | ||
| selectedPlaceIds = if (selectedPlaceIds.size == currentDayPlaceIds.size) { | ||
| emptySet() | ||
| } else { | ||
| currentDayPlaceIds | ||
| }, | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
checkSelectAll 로직이 ID 집합이 아닌 크기만 비교
현재 selectedPlaceIds.size == currentDayPlaceIds.size로 비교하고 있어, 다른 날짜의 ID가 selectedPlaceIds에 남아 있으면 실제로 현재 날짜의 전체가 선택되지 않았는데도 전체 선택으로 판정될 수 있습니다. selectDay에서 선택을 초기화하더라도, 집합 동등성(containsAll)으로 비교하는 것이 더 안전합니다.
🛡️ 수정 제안
copy(
- selectedPlaceIds = if (selectedPlaceIds.size == currentDayPlaceIds.size) {
+ selectedPlaceIds = if (selectedPlaceIds.containsAll(currentDayPlaceIds) && currentDayPlaceIds.isNotEmpty()) {
emptySet()
} else {
currentDayPlaceIds
},
)🤖 Prompt for AI Agents
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailViewModel.kt`
around lines 215 - 229, The checkSelectAll function compares only sizes which
misdetects full-selection when selectedPlaceIds contains IDs from other days;
update checkSelectAll (in TravelDetailViewModel) to compare sets instead of
sizes by checking selectedPlaceIds.containsAll(currentDayPlaceIds) (or
selectedPlaceIds == currentDayPlaceIds) so the selection is true only when all
currentDayPlaceIds are included; keep current logic for choosing
currentItineraries (tempItineraries vs itineraries) and selectedDay as-is.
| # Google Maps | ||
| mapsCompose = "6.4.1" | ||
| playServicesMaps = "19.1.0" | ||
|
|
||
| # Coil | ||
| coil = "2.7.0" | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Google Maps Compose latest version 2026
💡 Result:
As of February 14, 2026, the latest Google Maps Compose (artifact com.google.maps.android:maps-compose) release is 8.1.0 (released 2026-02-06). [1][2]
Gradle (Kotlin DSL):
dependencies {
implementation("com.google.maps.android:maps-compose:8.1.0")
}Sources: GitHub releases show v8.1.0 as “Latest”. [1] Maven Central listings also show 8.1.0 as the newest published version (Feb 2026). [2]
🌐 Web query:
Play Services Maps latest version 2026
💡 Result:
Latest Google Play services Maps artifact (com.google.android.gms:play-services-maps) version in 2026 is 20.0.0, released January 14, 2026. [1][2]
Gradle:
implementation "com.google.android.gms:play-services-maps:20.0.0"🌐 Web query:
Coil image loading library latest version 2026
💡 Result:
As of February 14, 2026, the latest released version of Coil (v3) is 3.3.0 (released July 22, 2025). [1] [2] [3]
Gradle examples (common artifacts):
implementation("io.coil-kt.coil3:coil-compose:3.3.0")
implementation("io.coil-kt.coil3:coil-network-okhttp:3.3.0")[2] [3]
Note: Coil v3 uses the io.coil-kt.coil3 group ID (the old io.coil-kt:coil coordinates are relocated). [3]
Sources: [1] [2] [3]
의존성 버전을 최신 안정 버전으로 업데이트하세요.
현재 지정된 버전들이 상당히 오래되었습니다:
mapsCompose: 6.4.1 → 최신 8.1.0 (2026년 2월 6일 출시)playServicesMaps: 19.1.0 → 최신 20.0.0 (2026년 1월 14일 출시)coil: 2.7.0 → 최신 3.3.0 (2025년 7월 22일 출시)
Google Maps Compose는 특히 2개 주요 버전이 뒤떨어져 있습니다. Coil 3.x로 업그레이드하는 경우 그룹 ID를 io.coil-kt.coil3로 변경해야 합니다.
🤖 Prompt for AI Agents
In `@gradle/libs.versions.toml` around lines 45 - 51, Update the dependency
versions in libs.versions.toml: set mapsCompose = "8.1.0" and playServicesMaps =
"20.0.0", and upgrade coil to "3.3.0"; additionally, when moving Coil to 3.x
update its Gradle coordinates to the new group ID (io.coil-kt.coil3) wherever
the Coil dependency is declared (search for the symbol coil and replace the old
group/artifact with the new group for Coil 3.x).
ae97e34 to
822f9b3
Compare
mj010504
force-pushed
the
design/NDGL-53
branch
from
822f9b3 to
1640f1c
Compare
1 participant
개요
디자인
https://www.figma.com/design/qHn9o58ENLeHjiBWNuZFJx/Design_-YAPP-1%ED%8C%80-?node-id=3565-18311&t=ZStfQoOkSrncjY0d-0
영상
NDGL-53.mp4
변경사항
참고사항
테스트 체크 리스트
Summary by CodeRabbit
릴리스 노트
새로운 기능
UI 개선
의존성 추가
구성