캠퍼스맵 UI 구현#731
Open
ikseong00 wants to merge 21 commits into
Open
Conversation
Contributor
Walkthrough장소 모델과 JSON 데이터가 시설·운영시간을 지원하도록 확장되었습니다. 캠퍼스맵에 검색, 카테고리 필터, 위치 권한, 지도 마커, 검색 결과 및 장소 상세 바텀시트가 추가되었습니다. Changes캠퍼스맵 기능 확장
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/bottomsheet/DragHandle.kt (1)
32-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win하드코딩된 색상을 디자인 시스템 토큰으로 교체하세요.
Color(0xFFD4D4D4)가 하드코딩되어 있습니다. 코딩 가이드라인에 따라KuringTheme.colors.*토큰을 사용해야 합니다.CampusMapBottomSheet.kt에서KuringTheme.colors.gray300을 이미 사용하고 있으므로 유사한 토큰으로 교체를 권장합니다.As per coding guidelines:
feature/**/compose/**/*.kt경로의 파일은 hardcoded color 대신KuRingColors디자인 시스템 토큰을 사용해야 합니다.♻️ 제안하는 수정
Box( modifier = Modifier .size(width = 35.dp, height = 4.dp) .clip(RoundedCornerShape(999.dp)) - .background(Color(0xFFD4D4D4)), + .background(KuringTheme.colors.gray300), )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/bottomsheet/DragHandle.kt` at line 32, Replace the hardcoded Color(0xFFD4D4D4) in DragHandle with the matching KuringTheme.colors gray token, preferably gray300, while preserving the existing background modifier behavior.Source: Coding guidelines
feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/bottomsheet/CampusMapBottomSheet.kt (1)
266-266: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win하드코딩된 색상을 디자인 시스템 토큰으로 교체하세요.
PlaceImage의 플레이스홀더 배경색(Color(0xFFD9D9D9))과FacilityInfo의 시설명 텍스트 색상(Color(0xFF39312C))이 하드코딩되어 있습니다. 코딩 가이드라인에 따라KuringTheme.colors.*토큰을 사용해야 합니다. 파일 내 다른 곳에서KuringTheme.colors.gray100,KuringTheme.colors.textTitle등을 이미 사용하고 있으므로 일관성을 맞춰주세요.As per coding guidelines:
feature/**/compose/**/*.kt경로의 파일은KuRingColors디자인 시스템 토큰을 사용해야 하며,feature/main/src/main/java/**/*.kt경로에서는 디자인 시스템의 공유 리소스를 로컬에 재정의하지 않아야 합니다.♻️ 제안하는 수정
`@Composable` private fun PlaceImage( imageUrl: String?, modifier: Modifier = Modifier, ) { Box( contentAlignment = Alignment.Center, modifier = modifier .clip(RoundedCornerShape(16.dp)) - .background(Color(0xFFD9D9D9)), + .background(KuringTheme.colors.gray200), ) {Text( text = facility.name, style = KuringTheme.typography.caption1_1, - color = Color(0xFF39312C), + color = KuringTheme.colors.textTitle, maxLines = 1, overflow = TextOverflow.Ellipsis, modifier = Modifier.weight(1f), )Also applies to: 382-382
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/bottomsheet/CampusMapBottomSheet.kt` at line 266, Replace the hardcoded colors in PlaceImage and FacilityInfo with the matching KuringTheme.colors tokens, using existing gray100 for the placeholder background and textTitle for the facility name. Keep the surrounding Compose styling unchanged and do not define local color resources.Source: Coding guidelines
data/place/src/main/assets/KonkukPlaces.json (1)
356-388: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
facilities가 있는 항목에서places필드가 중복됩니다.
동물생명과학관(356-360)과상허기념도서관(497-507)은places와facilities를 모두 가지고 있으나, 매퍼는facilities가 존재하면places를 무시합니다 (facilities?.map { it.toDomain() } ?: places.toFacilities()). 두 필드의 내용도 서로 다르므로,places는 잔여 데이터이며 향후 혼란을 방지하기 위해 제거를 권장합니다.Also applies to: 497-571
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@data/place/src/main/assets/KonkukPlaces.json` around lines 356 - 388, Remove the redundant places field from the 동물생명과학관 and 상허기념도서관 entries in KonkukPlaces.json, preserving their facilities data and all other fields unchanged. Ensure each affected entry relies solely on facilities, matching the mapper’s existing precedence behavior.feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/map/CampusPlaceMarker.kt (1)
137-145: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Color.White대신 디자인 시스템 토큰 사용을 권장합니다.비포커스 마커(116번 줄)는
KuringTheme.colors.background를 사용하지만, 포커스 마커(140번 줄)는Color.White를 사용하고 있어 일관성이 깨집니다. 다크 모드에서background가 흰색이 아닐 수 있으므로, 선택된 마커 드로어블의 배경이 항상 흰색이라면KuringTheme.colors.background대신 의도적인 흰색 토큰이 필요할 수 있습니다. 코딩 가이드라인에 따라 하드코딩된 색상 대신 디자인 시스템 토큰을 사용해야 합니다.As per coding guidelines:
feature/**/compose/**/*.kt: Use core:designsystem tokens (KuRingColors, KuRingTypography) instead of hardcoded colors and font sizes.♻️ 제안: 디자인 시스템 토큰 사용
Icon( painter = painterResource(id = iconRes), contentDescription = null, - tint = Color.White, + tint = KuringTheme.colors.background, modifier = Modifier .align(Alignment.TopCenter) .padding(top = 10.dp) .size(20.dp), )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/map/CampusPlaceMarker.kt` around lines 137 - 145, Update the focused marker Icon tint in CampusPlaceMarker to use the appropriate KuringTheme design-system color token instead of hardcoded Color.White; preserve the intended selected-marker appearance, using a dedicated intentional-white token if the drawable requires white in all themes.Source: Coding guidelines
feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/CampusMapUiState.kt (1)
15-63: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
visiblePlaces/searchResults가 매번 재계산됩니다.두 프로퍼티가 커스텀
get()으로 정의되어 있어, 같은CampusMapUiState인스턴스를 여러 곳에서 읽을 때마다(CampusMapScreen.kt에서uiState.visiblePlaces를 직접 참조하고,CampusMapSearchResultSheetEffect가uiState.searchResults를 읽으며 내부적으로visiblePlaces를 또 계산)searchCampusPlaces/filterCampusPlaces+toImmutableList()연산이 동일 recomposition 내에서 중복 수행됩니다.by lazy로 바꾸면 인스턴스 생성 시점 기준 1회만 계산되어 불필요한 필터링/리스트 할당을 줄일 수 있습니다. data class의 constructor 프로퍼티가 아니므로equals/copy()동작에는 영향이 없습니다.♻️ 제안 수정
- val visiblePlaces: ImmutableList<Place> - get() = if (hasSubmittedQuery) { - searchCampusPlaces( - places = campusPlaces, - query = submittedSearchQuery.orEmpty(), - ) - } else { - filterCampusPlaces( - places = campusPlaces, - selectedCategory = selectedCategory, - ) - }.toImmutableList() - - val searchResults: ImmutableList<CampusMapSearchResult> - get() = buildCampusMapSearchResults( - places = visiblePlaces, - selectedCategory = selectedCategory, - ).toImmutableList() + val visiblePlaces: ImmutableList<Place> by lazy { + if (hasSubmittedQuery) { + searchCampusPlaces( + places = campusPlaces, + query = submittedSearchQuery.orEmpty(), + ) + } else { + filterCampusPlaces( + places = campusPlaces, + selectedCategory = selectedCategory, + ) + }.toImmutableList() + } + + val searchResults: ImmutableList<CampusMapSearchResult> by lazy { + buildCampusMapSearchResults( + places = visiblePlaces, + selectedCategory = selectedCategory, + ).toImmutableList() + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/CampusMapUiState.kt` around lines 15 - 63, Update the visiblePlaces and searchResults computed properties in CampusMapUiState to use lazy initialization so each value is calculated at most once per state instance. Preserve their existing filtering, search-result construction, and immutable-list conversion logic, and keep them as non-constructor properties so equals and copy behavior remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@data/place/src/main/assets/KonkukPlaces.json`:
- Line 1031: KonkukPlaces.json의 해당 ATM name 값에 포함된 오타 “건국대민자식댕내”를 올바른 명칭인
“건국대기숙사내”로 수정하고, 나머지 장소 데이터는 변경하지 마세요.
In
`@feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/bottomsheet/CampusMapBottomSheet.kt`:
- Around line 420-425: Update the vacation CampusMapDetailRow in the
CampusMapOperationHours section to use the same labelStyle, labelColor, and
valueStyle as the semester row: caption1_1 for both text styles and textBody for
the label color. Preserve the existing label, value, width, and modifier.
In
`@feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/bottomsheet/CampusMapSearchResultBottomSheet.kt`:
- Around line 320-322: Update the background color in the
CampusMapSearchResultBottomSheet composable to use the appropriate
KuringTheme.colors design-system token instead of the hardcoded
Color(0xFFD9D9D9), preserving the existing rounded clipping behavior and
ensuring the color adapts to theme modes.
- Around line 109-171: CampusMapSearchResultBottomSheet의 AnchoredDraggable API
사용에 ExperimentalFoundationApi 옵트인이 없습니다. 해당 컴포저블 또는 파일 수준에
`@OptIn`(ExperimentalFoundationApi::class)을 추가해 AnchoredDraggableState,
DraggableAnchors, anchoredDraggable, AnchoredDraggableDefaults.flingBehavior 사용이
컴파일되도록 하세요.
In
`@feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/inner_screen/CampusMapSearchScreen.kt`:
- Around line 79-85: Move the live search derivation out of CampusMapSearchRoute
and into CampusMapUiState by adding a derived liveSearchResults property that
applies searchCampusPlaces to campusPlaces and searchInput. Update the Route to
consume uiState.liveSearchResults directly and remove its
remember/searchCampusPlaces calculation, preserving the existing immutable-list
result behavior.
In `@feature/main/src/main/res/values/strings.xml`:
- Line 130: Update the string value for campus_map_recent_search_delete_all to
“전체 삭제” with a space, matching the existing delete_all_keyword_history wording.
---
Nitpick comments:
In `@data/place/src/main/assets/KonkukPlaces.json`:
- Around line 356-388: Remove the redundant places field from the 동물생명과학관 and
상허기념도서관 entries in KonkukPlaces.json, preserving their facilities data and all
other fields unchanged. Ensure each affected entry relies solely on facilities,
matching the mapper’s existing precedence behavior.
In
`@feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/CampusMapUiState.kt`:
- Around line 15-63: Update the visiblePlaces and searchResults computed
properties in CampusMapUiState to use lazy initialization so each value is
calculated at most once per state instance. Preserve their existing filtering,
search-result construction, and immutable-list conversion logic, and keep them
as non-constructor properties so equals and copy behavior remain unchanged.
In
`@feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/bottomsheet/CampusMapBottomSheet.kt`:
- Line 266: Replace the hardcoded colors in PlaceImage and FacilityInfo with the
matching KuringTheme.colors tokens, using existing gray100 for the placeholder
background and textTitle for the facility name. Keep the surrounding Compose
styling unchanged and do not define local color resources.
In
`@feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/bottomsheet/DragHandle.kt`:
- Line 32: Replace the hardcoded Color(0xFFD4D4D4) in DragHandle with the
matching KuringTheme.colors gray token, preferably gray300, while preserving the
existing background modifier behavior.
In
`@feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/map/CampusPlaceMarker.kt`:
- Around line 137-145: Update the focused marker Icon tint in CampusPlaceMarker
to use the appropriate KuringTheme design-system color token instead of
hardcoded Color.White; preserve the intended selected-marker appearance, using a
dedicated intentional-white token if the drawable requires white in all themes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 754954af-2f77-44c3-8455-f6960dde7331
⛔ Files ignored due to path filters (1)
feature/main/src/main/assets/campusmap/empty_search_image.svgis excluded by!**/*.svg
📒 Files selected for processing (36)
app/src/main/AndroidManifest.xmldata/domain/src/main/java/com/ku_stacks/ku_ring/domain/Place.ktdata/place/src/main/assets/KonkukPlaces.jsondata/place/src/main/java/com/ku_stacks/ku_ring/place/mapper/JsonToDomain.ktdata/place/src/main/java/com/ku_stacks/ku_ring/place/model/JsonPlace.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/MainScreen.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/CampusMapUiState.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/CampusMapViewModel.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/CampusMapScreen.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/CampusPlaceMarker.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/bottomsheet/AnchoredSheetNestedScroll.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/bottomsheet/CampusMapBottomSheet.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/bottomsheet/CampusMapDraggableBottomSheet.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/bottomsheet/CampusMapSearchResultBottomSheet.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/bottomsheet/DragHandle.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/map/CampusPlaceMarker.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/map/CurrentLocationFab.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/map/LibrarySeatFab.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/map/NaverMapSection.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/inner_screen/CampusMapSearchScreen.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/preview/CampusMapPreviewParameterProvider.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/type/CampusMapCategory.ktfeature/main/src/main/res/drawable/ic_campus_map_copy.xmlfeature/main/src/main/res/drawable/ic_campus_map_empty_search.xmlfeature/main/src/main/res/drawable/ic_campus_map_icon_building.xmlfeature/main/src/main/res/drawable/ic_campus_map_icon_cafe.xmlfeature/main/src/main/res/drawable/ic_campus_map_icon_kcube.xmlfeature/main/src/main/res/drawable/ic_campus_map_icon_print.xmlfeature/main/src/main/res/drawable/ic_campus_map_icon_rest.xmlfeature/main/src/main/res/drawable/ic_campus_map_icon_restaurant.xmlfeature/main/src/main/res/drawable/ic_campus_map_icon_smoke.xmlfeature/main/src/main/res/drawable/ic_campus_map_icon_store.xmlfeature/main/src/main/res/drawable/ic_campus_map_marker_selected.xmlfeature/main/src/main/res/drawable/ic_campus_map_recent_search.xmlfeature/main/src/main/res/drawable/ic_campus_map_search_clear.xmlfeature/main/src/main/res/values/strings.xml
💤 Files with no reviewable changes (1)
- feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/CampusPlaceMarker.kt
Comment on lines
+320
to
+322
| .clip(RoundedCornerShape(8.dp)) | ||
| .background(Color(0xFFD9D9D9)), | ||
| ) { |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
디자인 시스템 토큰 대신 하드코딩된 색상을 사용했습니다.
Color(0xFFD9D9D9)는 KuringTheme.colors 토큰이 아닌 하드코딩된 값이라 다크 모드에서 톤/대비가 어긋날 수 있습니다.
🎨 제안 수정
- .background(Color(0xFFD9D9D9)),
+ .background(KuringTheme.colors.gray200), // 실제 디자인 시스템 토큰으로 교체As per coding guidelines, feature/**/compose/**/*.kt: "Use core:designsystem tokens (KuRingColors, KuRingTypography) instead of hardcoded colors and font sizes."
📝 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.
Suggested change
| .clip(RoundedCornerShape(8.dp)) | |
| .background(Color(0xFFD9D9D9)), | |
| ) { | |
| .clip(RoundedCornerShape(8.dp)) | |
| .background(KuringTheme.colors.gray200), | |
| ) { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/bottomsheet/CampusMapSearchResultBottomSheet.kt`
around lines 320 - 322, Update the background color in the
CampusMapSearchResultBottomSheet composable to use the appropriate
KuringTheme.colors design-system token instead of the hardcoded
Color(0xFFD9D9D9), preserving the existing rounded clipping behavior and
ensuring the color adapts to theme modes.
Source: Coding guidelines
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -e kt . | xargs rg -l "KuringColors|object.*Colors" --iglob '*designsystem*' 2>/dev/null | head -20
echo "---"
fd -i "colors" --type f
echo "---"
fd -i "kuringcolor" --type fLength of output: 956
🏁 Script executed:
#!/bin/bash
cat -n core/designsystem/src/main/java/com/ku_stacks/ku_ring/designsystem/kuringtheme/KuringColors.kt
echo "===="
cat -n core/designsystem/src/main/java/com/ku_stacks/ku_ring/designsystem/kuringtheme/values/Colors.ktLength of output: 16783
@mwy3055 확인해봤는데, 정확히 D9D9D9와 일치하는 디자인 시스템 토큰은 없습니다. core/designsystem/.../values/Colors.kt에 정의된 그레이 계열은 다음과 같습니다:
gray100=#F2F3F5(light) /#3D3D3E(dark)gray200=#E5E5E5(light) /#4E4E4E(dark)gray300=#999999(light) /#6B6B6B(dark)gray400=#434343(light) /#B0B0B0(dark)gray600=#262626(light) /#DFDFDF(dark)
가장 가까운 값은 gray200(#E5E5E5)이지만 정확히 D9D9D9는 아닙니다. 톤 차이가 크지 않아 KuringTheme.colors.gray200으로 대체해도 시각적으로 큰 차이는 없을 것 같고, 다크 모드에서도 자연스럽게 대응됩니다. 다만 디자인 의도상 정확한 값이 필요하다면 새로운 토큰을 디자인 시스템에 추가하는 것도 고려해볼 수 있습니다.
✏️ Learnings added
Learnt from: mwy3055
Repo: ku-ring/KU-Ring-Android PR: 731
File: feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/bottomsheet/CampusMapSearchResultBottomSheet.kt:320-322
Timestamp: 2026-07-13T13:50:08.033Z
Learning: In `feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/bottomsheet/CampusMapSearchResultBottomSheet.kt`, `core/designsystem` module (`KuringColors`/`values/Colors.kt`) does not define an exact gray token for `0xFFD9D9D9`. The closest existing token is `gray200` (`#E5E5E5` light / `#4E4E4E` dark). When flagging hardcoded gray colors close to this value in campusmap feature Compose files, note that no exact match exists in the design system and suggest `gray200` as the closest alternative, or suggest adding a new token if exact match is required.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
mwy3055
reviewed
Jul 13, 2026
| contentAlignment = Alignment.Center, | ||
| modifier = modifier | ||
| .clip(RoundedCornerShape(16.dp)) | ||
| .background(Color(0xFFD9D9D9)), |
Member
There was a problem hiding this comment.
리소스 값이 없나요? 없다면 디자인팀에 생성 요청해야 합니다. (라이트/다크모드 둘다)
Contributor
Author
There was a problem hiding this comment.
이 부분은, Light Dark 둘 다 D9D9D9값으로 되어있어서 냅두겠습니다!
| Text( | ||
| text = facility.name, | ||
| style = KuringTheme.typography.caption1_1, | ||
| color = Color(0xFF39312C), |
Member
There was a problem hiding this comment.
여기도요 (색상값 하드코딩) 다크모드에서 전혀 안 보이는데 최소한 여기는 리소스값 선언해야 할듯합니다
l2hyunwoo
reviewed
Jul 13, 2026
Comment on lines
+11
to
+17
| val number: Int? = null, | ||
| val iconUrl: String? = null, | ||
| val phone: String? = null, | ||
| val data: String? = null, | ||
| val imageUrl: String? = null, | ||
| val operationHours: PlaceOperationHours? = null, | ||
| val facilities: List<PlaceFacility> = emptyList(), |
Collaborator
There was a problem hiding this comment.
이거 진짜...nullable...이어야하는거...맞죠...?
Contributor
Author
There was a problem hiding this comment.
아직 API 미구현이라 임시 설정으로 해놨습니다!
boiledeggg
reviewed
Jul 13, 2026
| operationHours = operationHours?.toDomain(), | ||
| ) | ||
|
|
||
| private fun com.ku_stacks.ku_ring.place.model.JsonPlaceOperationHours.toDomain() = PlaceOperationHours( |
| tint = KuringTheme.colors.gray600, | ||
| modifier = Modifier | ||
| .size(24.dp) | ||
| .rotate(180f), |
Member
There was a problem hiding this comment.
왼쪽 방향을 향하는 아이콘이 없나요?
방향별로 아이콘을 둬야 코드 볼 때 무슨 아이콘인지 인지하기 쉬울 것 같습니다!
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/inner_screen/CampusMapSearchScreen.kt (1)
334-348: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
when구문을 사용한 명시적 상태 분기를 고려해 보세요.과거 리뷰에서 두
if문의 통합 여부에 대한 논의가 있었습니다. 각 조건마다 표시해야 하는 빈 상태 메시지 리소스가 다르기 때문에 완전히 하나의 단일if조건으로 묶기는 어렵습니다 (의도된 분기).다만 Jetpack Compose에서는 early return(빠른 반환) 패턴보다는
when구문을 활용하여 상호 배타적인 UI 상태(빈 최근 검색, 빈 검색 결과, 일반 목록)를 명시적으로 표현하는 것이 선언형 UI 작성 관례에 더 부합합니다.♻️ UI 분기 로직 리팩터링 제안
- if (trimmedQuery.isEmpty() && recentSearches.isEmpty()) { - CampusMapSearchEmptyState( - message = stringResource(id = R.string.campus_map_empty_recent_search), - modifier = modifier, - ) - return - } - - if (trimmedQuery.isNotEmpty() && searchResults.isEmpty()) { - CampusMapSearchEmptyState( - message = stringResource(id = R.string.campus_map_empty_search_result), - modifier = modifier, - ) - return - } - - LazyColumn( - contentPadding = PaddingValues(top = 4.dp, bottom = 24.dp), - modifier = modifier, - ) { + when { + trimmedQuery.isEmpty() && recentSearches.isEmpty() -> { + CampusMapSearchEmptyState( + message = stringResource(id = R.string.campus_map_empty_recent_search), + modifier = modifier, + ) + } + trimmedQuery.isNotEmpty() && searchResults.isEmpty() -> { + CampusMapSearchEmptyState( + message = stringResource(id = R.string.campus_map_empty_search_result), + modifier = modifier, + ) + } + else -> { + LazyColumn( + contentPadding = PaddingValues(top = 4.dp, bottom = 24.dp), + modifier = modifier, + ) {위와 같이 수정할 경우,
LazyColumn의 스코프 닫는 괄호}아래에else블록을 닫는}를 추가해주어야 합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/inner_screen/CampusMapSearchScreen.kt` around lines 334 - 348, CampusMapSearchScreen의 빈 상태 분기를 early return 기반의 두 if문에서 상호 배타적인 when 표현식으로 리팩터링하세요. trimmedQuery와 recentSearches 조건에는 campus_map_empty_recent_search를, 검색 결과가 없는 조건에는 campus_map_empty_search_result를 표시하고, 두 조건이 모두 아니면 기존 일반 목록 UI를 실행하세요. LazyColumn 스코프 이후 when의 else 블록이 올바르게 닫히도록 구조를 유지하세요.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/inner_screen/CampusMapSearchScreen.kt`:
- Around line 334-348: CampusMapSearchScreen의 빈 상태 분기를 early return 기반의 두 if문에서
상호 배타적인 when 표현식으로 리팩터링하세요. trimmedQuery와 recentSearches 조건에는
campus_map_empty_recent_search를, 검색 결과가 없는 조건에는 campus_map_empty_search_result를
표시하고, 두 조건이 모두 아니면 기존 일반 목록 UI를 실행하세요. LazyColumn 스코프 이후 when의 else 블록이 올바르게
닫히도록 구조를 유지하세요.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 699665d8-48a5-4fc8-88b5-b2094d952d8f
📒 Files selected for processing (7)
core/designsystem/src/main/res/drawable/ic_chevron_left_v2.xmldata/place/src/main/java/com/ku_stacks/ku_ring/place/mapper/JsonToDomain.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/CampusMapUiState.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/CampusMapScreen.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/bottomsheet/CampusMapSearchResultBottomSheet.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/inner_screen/CampusMapSearchScreen.ktfeature/main/src/main/res/values/strings.xml
💤 Files with no reviewable changes (1)
- feature/main/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (4)
- data/place/src/main/java/com/ku_stacks/ku_ring/place/mapper/JsonToDomain.kt
- feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/bottomsheet/CampusMapSearchResultBottomSheet.kt
- feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/CampusMapScreen.kt
- feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/CampusMapUiState.kt
* CampusPlaceMarker의 아이콘 틴트 색상을 KuringTheme.colors.white로 변경 * DragHandle의 배경색을 KuringTheme.colors.gray200으로 변경
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/CampusMapScreen.kt (1)
518-546: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win검색 결과 상태 변경마다 시트 콘텐츠가 null → 재설정되며 깜빡일 수 있음.
DisposableEffect(uiState.showSearchResultSheet, results)는results(List) 인스턴스가 바뀔 때마다(예: 검색어 입력 중 매 emission) 이전 effect의onDispose(content를 null로)가 먼저 실행된 뒤 새 content가 설정됩니다. 검색 중 사용자가 타이핑할 때마다 바텀시트 콘텐츠가 순간적으로 사라졌다가 다시 나타나는 깜빡임이 발생할 수 있습니다.♻️ 제안하는 수정
- DisposableEffect(uiState.showSearchResultSheet, results) { - currentOnContentChange( - if (uiState.showSearchResultSheet) { - CampusMapSearchResultSheetContent( - results = results, - onResultClick = { result -> currentOnResultClick(result.place) }, - onDismiss = { currentOnDismiss() }, - ) - } else { - null - }, - ) - onDispose { - currentOnContentChange(null) - } - } + LaunchedEffect(uiState.showSearchResultSheet, results) { + currentOnContentChange( + if (uiState.showSearchResultSheet) { + CampusMapSearchResultSheetContent( + results = results, + onResultClick = { result -> currentOnResultClick(result.place) }, + onDismiss = { currentOnDismiss() }, + ) + } else { + null + }, + ) + } + DisposableEffect(Unit) { + onDispose { + currentOnContentChange(null) + } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/CampusMapScreen.kt` around lines 518 - 546, Update CampusMapSearchResultSheetEffect so searchResults changes update the existing sheet content without first clearing it through DisposableEffect.onDispose. Keep null content only when the sheet is dismissed, and preserve the current result-click and dismiss callbacks via rememberUpdatedState.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/CampusMapScreen.kt`:
- Around line 518-546: Update CampusMapSearchResultSheetEffect so searchResults
changes update the existing sheet content without first clearing it through
DisposableEffect.onDispose. Keep null content only when the sheet is dismissed,
and preserve the current result-click and dismiss callbacks via
rememberUpdatedState.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 775cd2b7-78ab-41df-9f05-9c14d69f1612
📒 Files selected for processing (11)
core/preferences/src/main/java/com/ku_stacks/ku_ring/preferences/PreferenceUtil.ktcore/util/src/main/java/com/ku_stacks/ku_ring/util/PermissionUtil.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/MainScreen.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/CampusMapUiState.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/CampusMapViewModel.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/CampusMapScreen.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/bottomsheet/CampusMapBottomSheet.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/bottomsheet/DragHandle.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/map/CampusPlaceMarker.ktfeature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/map/NaverMapSection.ktfeature/main/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (5)
- feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/map/CampusPlaceMarker.kt
- feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/bottomsheet/DragHandle.kt
- feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/CampusMapViewModel.kt
- feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/compose/component/map/NaverMapSection.kt
- feature/main/src/main/java/com/ku_stacks/ku_ring/main/campusmap/CampusMapUiState.kt
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
4 participants
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.
요약
설명
영상
Screen_recording_20260712_061148.mp4
Summary by CodeRabbit
새 기능
데이터 개선
Summary by CodeRabbit
새로운 기능
개선 사항
권한
추가 구현
TODO