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. Walkthrough여행 세부 기능에 장소 상세 화면을 추가합니다. 새로운 PlaceDetail 컴포넌트, ViewModel, 계약 정의를 포함하며, 기존 TravelDetail 기능을 모달, 바텀 시트, 입력 컴포넌트로 확장합니다. UI 유틸리티와 리소스도 추가됩니다. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ 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-63
branch
from
62d50cc to
eebe4f5
Compare
mj010504
force-pushed
the
design/NDGL-53
branch
2 times, most recently
from
cadef03 to
4665ab9
Compare
mj010504
force-pushed
the
design/NDGL-63
branch
from
eebe4f5 to
a2e3ffe
Compare
mj010504
force-pushed
the
design/NDGL-53
branch
2 times, most recently
from
e591261 to
e6dc184
Compare
mj010504
force-pushed
the
design/NDGL-63
branch
2 times, most recently
from
ccd8c28 to
5ebcb9a
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: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/component/TimelineContent.kt (2)
74-91:⚠️ Potential issue | 🟠 Major닫기 버튼에 접근성 라벨이 필요합니다.
클릭 가능한 아이콘인데
contentDescription이 null이라 스크린리더에서 의미가 사라집니다.🔧 제안 수정
Icon( imageVector = ImageVector.vectorResource(R.drawable.ic_24_close), - contentDescription = null, + contentDescription = stringResource(R.string.accessibility_close), tint = NDGLTheme.colors.black900, )
153-224:⚠️ Potential issue | 🟠 Major00:00~00:59 시작 시간이 선택 불가해질 수 있습니다.
isTimeSet이inWholeHours > 0조건이라 0시대는 버튼이 비활성화됩니다. 0시가 유효 값이면 별도 플래그나 분 단위 체크로 분리하는 게 안전합니다.🔧 한 가지 개선 방향 (예시)
- val isTimeSet = selectedStartTime.inWholeHours > 0 && previewEndTime.inWholeHours > 0 + // 0시도 유효 값이라면, "사용자 설정 여부"를 별도 플래그로 관리 권장 + val isTimeSet = previewEndTime.inWholeMinutes > 0
🤖 Fix all issues with AI agents
In `@core/ui/src/main/java/com/yapp/ndgl/core/ui/designsystem/NDGLInputModal.kt`:
- Around line 40-105: NDGLInputModal's internal state (textFieldValue) can drift
from the external value when the parent rejects changes; add a validation
callback parameter (e.g., validateInput: (String) -> Boolean = { true }) to
NDGLInputModal and use it inside the BasicTextField onValueChange handler: call
validateInput(newValue.text) before updating textFieldValue and before invoking
onValueChange, only committing the internal state when validation passes;
alternatively make onValueChange return a Boolean and update textFieldValue only
when the callback approves—update references to textFieldValue, onValueChange,
value, and the LaunchedEffect(value) synchronization accordingly so UI and
external state remain consistent.
In `@core/ui/src/main/java/com/yapp/ndgl/core/ui/util/WebUtil.kt`:
- Around line 12-14: In WebUtil.kt update the catch block that currently reads
catch (e: Exception) { Timber.e("Failed to launch browser: $url") } to log the
actual exception and provide user feedback: include the exception object in the
Timber call (e.g., Timber.e(e, "...")) so stack traces are recorded, and notify
the user of failure (e.g., via Toast or Snackbar from the calling Context) with
a friendly message; locate the try-catch around the browser launch logic (the
method that opens/launches the browser) and replace the single-line log with
these two actions.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/placedetail/component/PlaceInfoTab.kt`:
- Around line 94-99: 현재 PlaceInfoTab.kt의 해당 Row 사용자가 메뉴/웹사이트 행을 클릭했을 때 잘못
clickAddress()를 호출하고 있습니다; 이 Row의 Modifier.noRippleClickable 콜백을
clickAddress()에서 clickMenu()로 변경해 메뉴 클릭 시 clickMenu()가 호출되도록 수정하세요 (참조:
Modifier.noRippleClickable 콜백 위치, clickAddress(), clickMenu(), Row 구성 요소).
- Around line 169-185: PlaceTipsPager should early-return when tips is empty to
avoid creating a Pager/rememberPagerState for size 0; add a guard at the start
of the PlaceTipsPager composable that checks if (tips.isEmpty()) and returns
immediately before creating pagerState or rendering HorizontalPager, so you
don't call rememberPagerState(pageCount = { tips.size }) or render
HorizontalPager when there are no tips.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/placedetail/PlaceDetailContract.kt`:
- Around line 55-56: aspectRatio getter (in PlaceDetailContract.kt) divides
width by height without guarding against height == 0; change the aspectRatio
property to check height for zero (or <= 0) and return a safe default (e.g., 0f
or 1f) when height is zero to avoid Infinity/NaN, otherwise compute
width.toFloat() / height.toFloat(); update any callers of aspectRatio if they
expect non-zero behavior.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/placedetail/PlaceDetailScreen.kt`:
- Around line 172-182: The displayed text in the Text composable uses a
hardcoded "식당 • $20~40 • 리뷰" string; update the buildAnnotatedString call inside
PlaceDetailScreen (the Text / buildAnnotatedString block referencing
placeInfo.rating and placeInfo.formattedRatingCount) to use placeInfo.category
and the price range field (e.g., placeInfo.priceRange or similar) instead of the
hardcoded values, concatenating them with separators and preserving the existing
styles for rating and formattedRatingCount; include sensible null/empty
fallbacks (e.g., "-" or "가격정보 없음") when category or priceRange are missing.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/placedetail/PlaceDetailViewModel.kt`:
- Around line 101-107: confirmChangePlace currently only closes the modal
(reduce { copy(showChangeModal = false) }) but doesn't apply the selected
alternative; update confirmChangePlace to use selectedAlternativePlace to
perform the actual change (e.g., call the ViewModel's place-update function or
trigger navigation) and then close the modal, or if intentional postpone, add a
clear TODO/TODO KTX comment inside confirmChangePlace referencing
selectedAlternativePlace and the intended handler (e.g., updatePlace(...) or
navigateToPlace(...)) so the missing behavior is explicit; look for
confirmChangePlace, selectedAlternativePlace, and reduce/copy(showChangeModal)
in the file to place the change.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/component/DurationPickerContent.kt`:
- Around line 61-63: Normalize currentDuration when computing selectedHour and
selectedMinute: clamp the hour into 0-23 and snap the minute to a 5-minute
bucket so the WheelPicker initial positions match its ranges. In the
DurationPickerContent initialization (where selectedHour and selectedMinute are
set), compute hour = currentDuration.inWholeHours.toInt().coerceIn(0, 23) (or
use modulo 24 for wrap behavior) and minute = ((currentDuration.inWholeMinutes %
60).toInt() / 5) * 5 to force 0,5,10,...,55, then assign selectedHour and
selectedMinute from these normalized values.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/component/PlaceItem.kt`:
- Around line 121-150: The PlaceNumber composable is formatting startTime with
raw hours/minutes which yields values like "50:30" when duration exceeds 24h;
change PlaceNumber to use the shared time formatter (formatString()) or convert
duration into day + hh:mm (e.g., "1일 + 02:30") instead of raw hours, so long
durations render correctly and consistently with
TravelDetailToolBar.toAmPmTimeString(); update the logic that computes
totalMinutes/hours/minutes to call formatString(startTime) or compute days =
hours/24, remHours = hours%24 and build the display string, and replace the
current manual String.format usage in PlaceNumber accordingly.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/component/TimelineContent.kt`:
- Around line 61-64: previewEndTime is only remembered with selectedStartTime as
a key, so it won't update when totalDuration changes; update the remember call
for previewEndTime to include totalDuration as a key (e.g.,
remember(selectedStartTime, totalDuration) { derivedStateOf { selectedStartTime
+ totalDuration } }) so previewEndTime recomputes when either selectedStartTime
or totalDuration changes.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailScreen.kt`:
- Around line 473-501: The cost modal currently keeps prior input when reopened
and silently ignores invalid submits; change the cost state to reset when the
modal opens (e.g., derive/reset mutable state from state.selectedPlace or use
remember with a key like state.showCostModal or state.selectedPlace) so cost is
initialized to state.selectedPlace.userData?.cost?.toString() whenever the modal
appears, and add explicit validation feedback in the NDGLInputModal positive
handler (instead of silently doing nothing on toIntOrNull == null) by either
disabling/gray-out the confirm button when the input is invalid or invoking a
local error state and showing an error message, then call confirmCost(costValue)
on valid parse; update references: state.showCostModal, state.selectedPlace,
cost, NDGLInputModal, confirmCost, dismissCostModal.
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailViewModel.kt`:
- Line 326: totalDuration is computed using the old itineraries list which can
be stale after start-time or duration updates; replace the use of itineraries
with updatedItineraries when computing totalDuration (i.e., use
updatedItineraries.getOrNull(dayIndex)?.totalDuration ?: 0.hours) so endTime
calculations that follow use the updated data (references: totalDuration,
updatedItineraries, itineraries, dayIndex, endTime).
🧹 Nitpick comments (14)
feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/component/PlaceItem.kt (1)
216-288: 프리뷰에 startTime 샘플을 추가하면 UI 확인이 쉬워집니다.
새로 추가된 시간 라인이 실제로 어떻게 보이는지 미리 확인할 수 있어요.core/util/src/main/java/com/yapp/ndgl/core/util/IntUtil.kt (1)
6-6: 로케일 사용에 대한 확인이 필요합니다.
Locale.US를 사용하면 천 단위 구분자로 쉼표(,)를 일관되게 사용합니다. 한국어 앱에서 이 형식이 의도된 것인지 확인해 주세요. 기기 로케일을 따르려면Locale.getDefault()를 사용할 수 있습니다.feature/travel/src/main/java/com/yapp/ndgl/feature/travel/placedetail/component/PlacePhotoTab.kt (1)
35-44: 이미지 로딩 상태 처리 추가를 권장합니다.
AsyncImage에placeholder와error파라미터가 없어 이미지 로딩 실패 시 사용자에게 명확한 피드백이 제공되지 않습니다. 또한Color.LightGray대신 테마 색상 사용을 고려해 주세요.♻️ 제안: 로딩/에러 상태 처리 추가
AsyncImage( model = photo.url, contentDescription = null, + placeholder = painterResource(R.drawable.placeholder_image), + error = painterResource(R.drawable.placeholder_image), modifier = Modifier .fillMaxWidth() .aspectRatio(photo.aspectRatio) .clip(RoundedCornerShape(8.dp)) - .background(Color.LightGray), + .background(NDGLTheme.colors.black100), contentScale = ContentScale.Crop, )feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/component/TravelDetailToolBar.kt (2)
66-66: 아이콘 tint 색상 설정을 확인해 주세요.
Icon에tint파라미터가 지정되지 않아 Material3 기본 색상이 적용됩니다. 다른 UI 요소와의 일관성을 위해 텍스트 색상(NDGLTheme.colors.black400)과 동일하게 설정하는 것이 좋을 수 있습니다.♻️ 제안: tint 색상 추가
- Icon(imageVector = ImageVector.vectorResource(R.drawable.ic_20_clock), contentDescription = null) + Icon( + imageVector = ImageVector.vectorResource(R.drawable.ic_20_clock), + contentDescription = null, + tint = NDGLTheme.colors.black400, + )
88-88: EditTravelButton의 아이콘에도 동일하게 tint 설정을 고려해 주세요.StartTimeSettingButton과 마찬가지로 일관된 스타일링을 위해
ic_20_edit아이콘에도 tint 색상을 명시적으로 지정하는 것이 좋습니다.feature/travel/src/main/java/com/yapp/ndgl/feature/travel/placedetail/component/PlaceDetailTabRow.kt (2)
47-54: 선택된 탭의 시각적 구분을 강화하는 것을 고려해 주세요.현재 선택된 탭과 선택되지 않은 탭의 텍스트 색상이 동일합니다(
black600). 접근성과 사용성 향상을 위해 선택된 탭의 텍스트를 더 진하게 표시하거나 다른 색상을 사용하는 것을 고려해 보세요.♻️ 제안: 선택 상태에 따른 텍스트 스타일 차별화
text = { Text( stringResource(tab.titleRes), - color = NDGLTheme.colors.black600, - style = NDGLTheme.typography.bodyMdSemiBold, + color = if (isSelected) NDGLTheme.colors.black900 else NDGLTheme.colors.black600, + style = if (isSelected) NDGLTheme.typography.bodyMdBold else NDGLTheme.typography.bodyMdSemiBold, textAlign = TextAlign.Center, ) },
17-17: 불필요한 import alias가 있습니다.
SecondaryTabRow를 동일한 이름으로 alias하고 있어 불필요합니다.♻️ 제안: import 간소화
-import androidx.compose.material3.SecondaryTabRow as SecondaryTabRow +import androidx.compose.material3.SecondaryTabRowfeature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/component/DurationPickerContent.kt (1)
148-150: startIndex 계산에서 하드코딩 값을 제거하세요.
- 2는 visibleItemCount 변경 시 중앙 정렬이 깨질 수 있습니다.visibleItemCount / 2로 계산하면 더 안전합니다.♻️ 수정 제안
- val startIndex = (totalItemsCount / 2) - (totalItemsCount / 2 % items.size) + initialIndex - 2 + val startIndex = + (totalItemsCount / 2) - (totalItemsCount / 2 % items.size) + initialIndex - (visibleItemCount / 2)feature/travel/src/main/java/com/yapp/ndgl/feature/travel/placedetail/PlaceDetailContract.kt (1)
21-22: TODO가 남아 있습니다.
추후 변경 예정 항목은 이슈/작업 항목으로 추적해 주세요. 원하시면 이슈 등록 도와드릴게요.feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/component/PlaceBottomSheet.kt (2)
40-41: 불필요한 명시적 import 제거
kotlin.Int와kotlin.String은 Kotlin의 기본 타입으로 명시적 import가 필요하지 않습니다. IDE 자동 완성이나 복사-붙여넣기 과정에서 추가된 것으로 보입니다.♻️ 불필요한 import 제거
-import kotlin.Int -import kotlin.String import kotlin.time.Duration.Companion.hours
188-194: 하드코딩된 통화 단위 문자열
"원"통화 단위가 코드에 직접 하드코딩되어 있습니다. 다른 문자열들은stringResource를 사용하고 있으므로, 일관성과 향후 다국어 지원을 위해 문자열 리소스로 분리하는 것이 좋습니다.♻️ 문자열 리소스 사용 제안
Text( - text = "${cost.formatDecimal()}원", + text = stringResource(R.string.cost_format, cost.formatDecimal()), color = NDGLTheme.colors.black500, style = NDGLTheme.typography.bodyMdMedium, )feature/travel/src/main/java/com/yapp/ndgl/feature/travel/placedetail/PlaceDetailScreen.kt (1)
232-241: 사진 분할 로직 개선 가능
foldIndexed로 mutable list를 생성하는 방식은 동작하지만,partition또는filterIndexed를 사용하면 더 간결하고 immutable하게 처리할 수 있습니다.♻️ partition 활용 예시
PlaceDetailTab.PHOTO -> { - val (leftPhotos, rightPhotos) = state.photos.foldIndexed( - initial = mutableListOf<PlacePhoto>() to mutableListOf<PlacePhoto>(), - ) { index, lists, photo -> - if (index % 2 == 0) { - lists.first.add(photo) - } else { - lists.second.add(photo) - } - lists - } + val (leftPhotos, rightPhotos) = state.photos + .withIndex() + .partition { it.index % 2 == 0 } + .let { (left, right) -> left.map { it.value } to right.map { it.value } } item {feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailViewModel.kt (1)
472-494: confirmCost와 confirmMemo의 중복 패턴두 함수가 거의 동일한 구조를 가지고 있습니다. 현재 상태로도 동작에 문제는 없으나, 향후 유지보수를 위해 공통 로직을 추출하는 것을 고려해볼 수 있습니다.
Also applies to: 506-528
feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailScreen.kt (1)
503-525: 메모 모달도 동일한 상태 초기화 패턴 적용 필요비용 모달과 마찬가지로, 메모 모달도
remember에 key를 추가하여 모달이 다시 열릴 때 최신 값으로 초기화되도록 하는 것이 좋습니다.♻️ key 추가
if (state.showMemoModal && state.selectedPlace != null) { - var memo by remember { mutableStateOf(state.selectedPlace.userData?.memo ?: "") } + var memo by remember(state.showMemoModal) { + mutableStateOf(state.selectedPlace.userData?.memo ?: "") + }
Comment on lines
+169
to
+185
| @Composable | ||
| private fun PlaceTipsPager( | ||
| tips: List<String>, | ||
| creatorName: String, | ||
| ) { | ||
| val pagerState = rememberPagerState(pageCount = { tips.size }) | ||
|
|
||
| Box( | ||
| modifier = Modifier.wrapContentSize(), | ||
| ) { | ||
| HorizontalPager( | ||
| state = pagerState, | ||
| modifier = Modifier.fillMaxWidth(), | ||
| contentPadding = PaddingValues(horizontal = 35.dp), | ||
| pageSpacing = 10.dp, | ||
| ) { page -> | ||
| val tip = tips[page] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the PlaceInfoTab.kt file
find . -name "PlaceInfoTab.kt" -type fRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 176
🏁 Script executed:
# Once we find the file, let's read the full context around the function
# and see how PlaceTipsPager is being used in the composition
fd "PlaceInfoTab.kt"Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 174
🏁 Script executed:
# Search for HorizontalPager usage patterns in the codebase to understand
# how empty states are typically handled
rg "HorizontalPager" --type kt -A 3 -B 3Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 105
🏁 Script executed:
# Read the PlaceInfoTab.kt file
wc -l ./feature/travel/src/main/java/com/yapp/ndgl/feature/travel/placedetail/component/PlaceInfoTab.ktRepository: YAPP-Github/27th-App-Team-1-Android
Length of output: 180
🏁 Script executed:
# Read the file with specific line range around lines 169-185
cat -n ./feature/travel/src/main/java/com/yapp/ndgl/feature/travel/placedetail/component/PlaceInfoTab.kt | sed -n '150,200p'Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 2306
🏁 Script executed:
# Search for HorizontalPager usage without file type flag
rg "HorizontalPager" -A 5 -B 3Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 2571
🏁 Script executed:
# Read more of the PlaceTipsPager function to see the complete implementation
cat -n ./feature/travel/src/main/java/com/yapp/ndgl/feature/travel/placedetail/component/PlaceInfoTab.kt | sed -n '169,250p'Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 4059
🏁 Script executed:
# Check for build.gradle or build files to see Compose version
find . -name "build.gradle*" -o -name "build.gradle.kts" | head -5Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 207
🏁 Script executed:
# Search for any similar patterns of empty list handling in other composables
rg "isEmpty\(\)" -A 2 -B 2 --max-count=10Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 2623
🏁 Script executed:
# Check the Compose version in the build files
cat ./app/build.gradle.kts | grep -i "compose\|androidx.compose" -A 2 | head -20Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 61
🏁 Script executed:
# Also check for any version catalog
cat ./build.gradle.kts | grep -i "compose" -A 2 | head -20Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 243
🏁 Script executed:
# Search for any existing empty state handling with composables returning early
rg "if.*isEmpty.*return" -A 1 -B 1Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 473
🏁 Script executed:
# Check if there's a version catalog file for Compose dependencies
find . -name "libs.versions.toml" -o -name "versions.gradle.kts" | xargs cat 2>/dev/null | grep -i "compose" -A 1 | head -20Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 612
🏁 Script executed:
# Search for any test or error cases related to empty pagers
rg "pageCount.*0|HorizontalPager" -A 3 -B 1Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 1506
🏁 Script executed:
# Let's verify the complete end of the PlaceTipsPager function
cat -n ./feature/travel/src/main/java/com/yapp/ndgl/feature/travel/placedetail/component/PlaceInfoTab.kt | sed -n '240,260p'Repository: YAPP-Github/27th-App-Team-1-Android
Length of output: 868
빈 tips 리스트에 대한 early return 처리 필요
tips가 비어 있을 때 불필요한 HorizontalPager를 렌더링하게 됩니다. 다음과 같이 빈 상태를 먼저 처리하여 코드를 정리해 주세요.
🛠 수정 제안
private fun PlaceTipsPager(
tips: List<String>,
creatorName: String,
) {
+ if (tips.isEmpty()) return
val pagerState = rememberPagerState(pageCount = { tips.size })🤖 Prompt for AI Agents
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/placedetail/component/PlaceInfoTab.kt`
around lines 169 - 185, PlaceTipsPager should early-return when tips is empty to
avoid creating a Pager/rememberPagerState for size 0; add a guard at the start
of the PlaceTipsPager composable that checks if (tips.isEmpty()) and returns
immediately before creating pagerState or rendering HorizontalPager, so you
don't call rememberPagerState(pageCount = { tips.size }) or render
HorizontalPager when there are no tips.
Comment on lines
+473
to
+501
| if (state.showCostModal && state.selectedPlace != null) { | ||
| var cost by remember { mutableStateOf(state.selectedPlace.userData?.cost?.toString() ?: "") } | ||
|
|
||
| NDGLInputModal( | ||
| onDismissRequest = dismissCostModal, | ||
| title = stringResource(R.string.cost_modal_title), | ||
| value = cost, | ||
| onValueChange = { newValue -> | ||
| if (newValue.isEmpty() || newValue.all { it.isDigit() }) { | ||
| cost = newValue | ||
| } | ||
| }, | ||
| placeholder = stringResource(R.string.cost_modal_placeholder), | ||
| keyboardOptions = KeyboardOptions( | ||
| keyboardType = KeyboardType.Number, | ||
| imeAction = ImeAction.Done, | ||
| ), | ||
| positiveButtonText = stringResource(R.string.cost_modal_confirm), | ||
| onPositiveButtonClick = { | ||
| cost.toIntOrNull()?.let { costValue -> | ||
| confirmCost(costValue) | ||
| } | ||
| }, | ||
| negativeButtonText = stringResource(R.string.cost_modal_cancel), | ||
| textAlign = TextAlign.Center, | ||
| placeholderStyle = NDGLTheme.typography.subtitleLgSemiBold, | ||
| textStyle = NDGLTheme.typography.subtitleLgSemiBold, | ||
| ) | ||
| } |
There was a problem hiding this comment.
비용 입력 모달의 상태 관리 및 유효성 검사
두 가지 잠재적 이슈가 있습니다:
remember { mutableStateOf(...) }를 사용하여 모달이 닫혔다 다시 열릴 때 이전 입력값이 유지될 수 있습니다.- Line 491-495에서 빈 문자열이나 유효하지 않은 값 입력 시
toIntOrNull()이 null을 반환하면 아무 동작도 하지 않아 사용자에게 피드백이 없습니다.
💡 개선 제안
if (state.showCostModal && state.selectedPlace != null) {
- var cost by remember { mutableStateOf(state.selectedPlace.userData?.cost?.toString() ?: "") }
+ var cost by remember(state.showCostModal) {
+ mutableStateOf(state.selectedPlace.userData?.cost?.toString() ?: "")
+ }
NDGLInputModal(
...
onPositiveButtonClick = {
- cost.toIntOrNull()?.let { costValue ->
- confirmCost(costValue)
- }
+ val costValue = cost.toIntOrNull()
+ if (costValue != null) {
+ confirmCost(costValue)
+ } else if (cost.isNotEmpty()) {
+ // TODO: Show error feedback for invalid input
+ }
},🤖 Prompt for AI Agents
In
`@feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailScreen.kt`
around lines 473 - 501, The cost modal currently keeps prior input when reopened
and silently ignores invalid submits; change the cost state to reset when the
modal opens (e.g., derive/reset mutable state from state.selectedPlace or use
remember with a key like state.showCostModal or state.selectedPlace) so cost is
initialized to state.selectedPlace.userData?.cost?.toString() whenever the modal
appears, and add explicit validation feedback in the NDGLInputModal positive
handler (instead of silently doing nothing on toIntOrNull == null) by either
disabling/gray-out the confirm button when the input is invalid or invoking a
local error state and showing an error message, then call confirmCost(costValue)
on valid parse; update references: state.showCostModal, state.selectedPlace,
cost, NDGLInputModal, confirmCost, dismissCostModal.
Contributor
Author
There was a problem hiding this comment.
keyboardType이 Number인데도 해당 로직이 필요할까요?
There was a problem hiding this comment.
@mj010504 좋은 지적입니다! 맞습니다.
말씀하신 대로 keyboardType = KeyboardType.Number가 설정되어 있고, 추가로 onValueChange에서 이미 newValue.all { it.isDigit() } 검증을 하고 있어서 숫자가 아닌 입력은 필터링되고 있습니다.
또한 NDGLInputModal이 입력값이 비어있을 때 자동으로 확인 버튼을 비활성화하므로, 빈 문자열 케이스도 이미 UI 레벨에서 처리되고 있습니다.
따라서 toIntOrNull()?.let의 null 체크는 실질적으로:
- 정수 오버플로우 (매우 큰 숫자 입력 시) 같은 극히 드문 엣지 케이스만 방어하는 것입니다
현재 코드로도 충분히 안전하며, 제가 제안한 추가 검증 로직은 선택사항입니다. 기존 구현이 이미 주요 케이스들을 잘 처리하고 있다고 판단됩니다. 👍
🐰
🧠 Learnings used
Learnt from: mj010504
Repo: YAPP-Github/27th-App-Team-1-Android PR: 17
File: feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailContract.kt:52-59
Timestamp: 2026-02-11T12:09:30.033Z
Learning: In the Android project for YAPP 27th App Team 1, the `Budget.formatString()` method in `TravelDetailContract.kt` intentionally returns "만원" for any amount less than 10,000 won as per product requirements. This is not a bug but the intended behavior.
There was a problem hiding this comment.
@mj010504 감사합니다! 현재 구현도 충분히 안전하니 편하실 때 개선하시면 됩니다. 😊
🧠 Learnings used
Learnt from: mj010504
Repo: YAPP-Github/27th-App-Team-1-Android PR: 17
File: feature/travel/src/main/java/com/yapp/ndgl/feature/travel/traveldetail/TravelDetailContract.kt:52-59
Timestamp: 2026-02-11T12:09:30.033Z
Learning: In the Android project for YAPP 27th App Team 1, the `Budget.formatString()` method in `TravelDetailContract.kt` intentionally returns "만원" for any amount less than 10,000 won as per product requirements. This is not a bug but the intended behavior.
bb3e164 to
07576dd
Compare
mj010504
force-pushed
the
design/NDGL-53
branch
4 times, most recently
from
822f9b3 to
1640f1c
Compare
네이밍: 타임라인 자동생성 -> 여행 시작 시간 설정 WheelPicker: initialIndex 조정
-thumbnail 자연스럽게 내리기 -HorizontalPager indicator 조정
- TODO 주석 달기 - 방어적으로 코드 짜기 - TimelineContent - 로직 잘못 설정한 부분 수정 - PlaceInfoTab
mj010504
force-pushed
the
design/NDGL-63
branch
from
07576dd to
3720309
Compare
This was referenced Feb 17, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
개요
디자인
https://www.figma.com/design/qHn9o58ENLeHjiBWNuZFJx/Design_-YAPP-1%ED%8C%80-?node-id=3566-17857&t=ZStfQoOkSrncjY0d-0
영상
NDGL-63.mp4
변경사항
추후 작업사항
테스트 체크 리스트
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항