Feat/#5 mypage screen구현#37
Merged
starshape7 merged 30 commits intodevelopfrom Jan 21, 2026
Hidden character warning
The head ref may contain hidden characters: "feat/#5-mypage-screen\uad6c\ud604"
Merged
Conversation
Walkthrough마이페이지의 참여/모집/대기 스터디 탭용 UI·네비게이션·데이터 계층(스크린, 뷰모델, 상태, 내비게이션, 리포지토리/데이터소스/서비스)과 스터디 아이템에 선택적 슬롯( Changes
Sequence Diagram(s)sequenceDiagram
participant Screen as ParticipatingStudyScreen
participant VM as ParticipatingStudyViewModel
participant Repo as StudyRepository
participant DS as StudyDataSource
participant Service as StudyService
participant API as RemoteAPI
Screen->>VM: loadParticipatingStudy()
VM->>VM: uiState = Loading
VM->>Repo: getParticipatingStudy(cursor=null, size=10)
Repo->>DS: getMyPageStudy(statuses=["OWNER","APPROVED"], cursor, size)
DS->>Service: getMyPageStudy(statuses, cursor, size)
Service->>API: GET /api/studies/me?statuses=...&cursor=...&size=...
API-->>Service: BaseResponse<StudyResponseDto>
Service-->>DS: BaseResponse<StudyResponseDto>
DS-->>Repo: BaseResponse<StudyResponseDto>
Repo->>Repo: map DTO -> StudyResult (includes isOwner)
Repo-->>VM: Result<StudyResultList>
VM->>VM: uiState = Success/Empty
VM-->>Screen: emit uiState
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
토끼의 축시
🚥 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
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}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
data/weather/src/main/java/com/umcspot/spot/weather/mapper/WeatherMapper.kt (1)
9-15:!!연산자 사용으로 인한 런타임 크래시 위험이 있습니다.
WeatherResponse.body가 nullable로 변경되었으나, 여기서!!연산자를 사용하면 body가 null일 때KotlinNullPointerException이 발생합니다. body를 nullable로 만든 목적이 무효화됩니다.null인 경우를 안전하게 처리하거나, 기본값/에러 상태를 반환하도록 수정해야 합니다.
🐛 Null 안전 처리를 위한 수정 제안
fun WeatherResponseDto.toDomain(): WeatherResult { - val items = response.body!!.items.item + val body = response.body + ?: return WeatherResult( + currentTime = LocalTime.MIDNIGHT, + weatherTemp = null, + weatherType = WeatherType.NO_DATA // 또는 적절한 기본값 + ) + val items = body.items.item // 기준 시간 (HHmm) val time = items.firstOrNull()?.baseTime
🤖 Fix all issues with AI agents
In
`@data/study/src/main/java/com/umcspot/spot/study/repositoryimpl/StudyRepositoryImpl.kt`:
- Around line 174-175: getWaitingStudy의 onFailure 블록에서 복사·붙여넣기로 인해 로그 메시지가 잘못
`"getRecruitingStudy failed"`로 기록되어 있으니, StudyRepositoryImpl의 getWaitingStudy
메서드 내부 onFailure 핸들러(Log.e 호출)를 찾아 로그 메시지를 올바르게 `"getWaitingStudy failed"`로 수정해
주세요; 필요하면 메시지에 예외 정보(e)를 그대로 포함시켜 디버깅 정보를 보존하세요.
In
`@data/weather/src/main/java/com/umcspot/spot/weather/dto/response/WeatherResponseDto.kt`:
- Around line 20-21: WeatherMapper currently uses response.body!! (val items =
response.body!!.items.item) which will NPE if body is null; update the mapper to
handle the nullable body field explicitly by replacing the non-null assertion
with a safe-check: detect when response.body is null and either return an empty
items list or a default Weather DTO (or throw a controlled exception type), then
map items.item only when body is non-null; ensure the mapper function name where
this occurs (the WeatherMapper mapping function that computes items) is updated
accordingly and adjust callers (e.g., WeatherRepositoryImpl) to rely on the
mapper's explicit null-handling instead of relying on runCatching to absorb
NPEs.
In
`@feature/mypage/src/main/java/com/umcspot/spot/mypage/participating/ParticipatingStudyViewModel.kt`:
- Around line 70-74: When merging the next page into the existing
participatingStudy state in ParticipatingStudyViewModel, also update the
pagination metadata (e.g., nextCursor and hasNext) from newPage instead of
keeping the previous cursor; modify the merged object created from currentList
and newPage.studyList so it copies or sets nextCursor/hasNext (or whatever
pagination fields exist on currentList/newPage) from newPage, then call
_uiState.update { it.copy(participatingStudy = UiState.Success(merged)) } with
that merged object to prevent duplicate loads/infinite loading.
- Around line 29-51: The onFailure branch of loadParticipatingStudy currently
only logs the error and leaves _uiState in Loading; update the onFailure { e ->
... } handler for studyRepository.getParticipatingStudy to set _uiState.update {
it.copy(participatingStudy = UiState.Failure(e.message ?: "참여 스터디 불러오기 실패")) }
(note: use participatingStudy field, not the commented weatherInfo) so the UI
transitions to Failure with a helpful message.
In
`@feature/mypage/src/main/java/com/umcspot/spot/mypage/recruiting/RecruitingStudyViewModel.kt`:
- Around line 52-77: In loadNextPage() the code incorrectly calls
studyRepository.getParticipatingStudy(...) and fails to propagate pagination
metadata when merging pages; change the repository call to
studyRepository.getRecruitingStudy(cursor = currentList.nextCursor, size = 10)
and when constructing merged (used to update _uiState.recruitingStudy) copy over
newPage.hasNext and newPage.nextCursor (e.g., merged =
currentList.copy(studyList = currentList.studyList + newPage.studyList, hasNext
= newPage.hasNext, nextCursor = newPage.nextCursor)) so subsequent
loadNextPage() calls use the correct cursor and hasNext flag.
In
`@feature/mypage/src/main/java/com/umcspot/spot/mypage/waiting/WaitingStudyViewModel.kt`:
- Around line 52-77: In loadNextPage (WaitingStudyViewModel) the merged page is
built from currentList but does not incorporate newPage.hasNext and
newPage.nextCursor; update the merged assignment (where currentList.copy(...) is
used) to also set hasNext = newPage.hasNext and nextCursor = newPage.nextCursor
so pagination state advances correctly when updating _uiState
(UiState.Success(merged)).
🧹 Nitpick comments (9)
feature/mypage/src/main/java/com/umcspot/spot/mypage/participating/ParticipatingStudyScreen.kt (1)
162-217:indexOf대신itemsIndexed로 마지막 아이템 체크를 최적화하세요. 리스트가 커지면indexOf반복 호출이 O(n^2)로 커질 수 있습니다.♻️ 개선 제안
- items( - items = studyList, - key = { it.id } - ) { item -> + itemsIndexed( + items = studyList, + key = { _, item -> item.id } + ) { index, item -> ... - if (studyList.indexOf(item) != studyList.lastIndex) { + if (index != studyList.lastIndex) { Spacer(Modifier.padding(screenHeightDp(5.dp))) HorizontalDivider( modifier = Modifier.fillMaxWidth(), color = SpotTheme.colors.G300, thickness = 1.dp ) } }data/study/src/main/java/com/umcspot/spot/study/dto/response/StudyResponseDto.kt (1)
45-47: 기본값 추가를 고려해 주세요.
isOwner필드에 기본값이 없어서, 이 필드를 포함하지 않는 기존 API 응답이 있을 경우 역직렬화 실패가 발생할 수 있습니다. 하위 호환성을 위해 기본값 추가를 고려해 주세요.♻️ 제안하는 수정
`@SerialName`("isOwner") - val isOwner: Boolean, + val isOwner: Boolean = false,data/study/src/main/java/com/umcspot/spot/study/repositoryimpl/StudyRepositoryImpl.kt (1)
133-146: 상태 값을 상수 또는 enum으로 추출하는 것을 고려해 주세요.
"OWNER","APPROVED","APPLIED"등의 상태 문자열이 하드코딩되어 있습니다. 유지보수성과 타입 안전성을 위해 상수나 enum으로 추출하면 좋을 것 같습니다.// 예시: 별도 상수 또는 enum 정의 object StudyMemberStatus { const val OWNER = "OWNER" const val APPROVED = "APPROVED" const val APPLIED = "APPLIED" }Also applies to: 148-161, 163-176
feature/mypage/src/main/java/com/umcspot/spot/mypage/recruiting/RecruitingStudyViewModel.kt (1)
26-50: 함수명과 실제 동작이 불일치합니다.
loadParticipatingStudy()라는 함수명이지만 실제로는getRecruitingStudy()를 호출합니다. 함수명을loadRecruitingStudy()로 변경하거나 의미에 맞게 수정해주세요.또한 로그 태그가
"HomeViewModel"로 되어있어 디버깅 시 혼란을 줄 수 있습니다.♻️ 제안하는 수정사항
- fun loadParticipatingStudy() { + fun loadRecruitingStudy() { _uiState.update { it.copy(recruitingStudy = UiState.Loading) } viewModelScope.launch { studyRepository.getRecruitingStudy( cursor = null, size = 10 ) .onSuccess { info -> _uiState.update { it.copy( recruitingStudy = if (info.studyList.isEmpty()) { UiState.Empty } else { UiState.Success(info) } ) } } .onFailure { e -> - Log.e("HomeViewModel", "loadParticipatingStudy error", e) + Log.e("RecruitingStudyViewModel", "loadRecruitingStudy error", e) + _uiState.update { it.copy(recruitingStudy = UiState.Failure(e.message ?: "로딩 실패")) } } } }feature/mypage/src/main/java/com/umcspot/spot/mypage/recruiting/RecruitingStudyScreen.kt (1)
146-187:studyList.indexOf(item)사용으로 O(n²) 성능 문제가 발생합니다.
items블록 내에서studyList.indexOf(item)을 호출하면 각 아이템마다 O(n) 탐색이 발생하여 전체적으로 O(n²) 복잡도가 됩니다.itemsIndexed를 사용하면 인덱스를 직접 제공받아 성능을 개선할 수 있습니다.♻️ itemsIndexed 사용 제안
- items( - items = studyList, - key = { it.id } - ) { item -> + itemsIndexed( + items = studyList, + key = { _, item -> item.id } + ) { index, item -> Spacer(Modifier.padding(screenHeightDp(5.dp))) Box( modifier = Modifier.fillMaxWidth(), contentAlignment = Alignment.TopStart ) { StudyListItem( item = item, modifier = Modifier.fillMaxWidth(), onClick = { onStudyClick(item.id) }, checkAppliedSlot = { TextButton( modifier = Modifier .width(screenWidthDp(60.dp)) .height(screenHeightDp(26.dp)), text = "신청 확인", style = SpotTheme.typography.regular_500, onClick = moveToCheckApplied, state = TextButtonState.B500State, shape = SpotShapes.Hard ) } ) } - if (studyList.indexOf(item) != studyList.lastIndex) { + if (index != studyList.lastIndex) { Spacer(Modifier.padding(screenHeightDp(5.dp))) HorizontalDivider( modifier = Modifier .fillMaxWidth(), color = SpotTheme.colors.G300, thickness = 1.dp ) } }feature/mypage/src/main/java/com/umcspot/spot/mypage/waiting/WaitingStudyScreen.kt (2)
32-38: 사용하지 않는 import가 있습니다.다음 import들이 사용되지 않는 것으로 보입니다:
TextButton(Line 32)TextButtonState(Line 33)EmptyAlert(Line 34) -EmptyAlertWithButton만 사용됨SpotShapes(Line 37)G300(Line 38)
146-174:studyList.indexOf(item)사용으로 O(n²) 성능 문제가 발생합니다.RecruitingStudyScreen과 동일한 이슈입니다.
itemsIndexed를 사용하여 성능을 개선해주세요.♻️ itemsIndexed 사용 제안
- items( - items = studyList, - key = { it.id } - ) { item -> + itemsIndexed( + items = studyList, + key = { _, item -> item.id } + ) { index, item -> Spacer(Modifier.padding(screenHeightDp(5.dp))) Box( modifier = Modifier.fillMaxWidth(), contentAlignment = Alignment.TopStart ) { StudyListItem( item = item, modifier = Modifier.fillMaxWidth(), onClick = { onStudyClick(item.id) } ) } - if (studyList.indexOf(item) != studyList.lastIndex) { + if (index != studyList.lastIndex) { Spacer(Modifier.padding(screenHeightDp(5.dp))) HorizontalDivider( modifier = Modifier .fillMaxWidth(), color = SpotTheme.colors.G300, thickness = 1.dp ) } }feature/main/src/main/java/com/umcspot/spot/main/MainNavHost.kt (1)
130-136:moveToMakeStudy와moveToCheckApplied콜백이 빈 람다로 구현되어 있습니다.PR 설명에 따르면 내부 모달 기능이 아직 미구현 상태인 것으로 보입니다. TODO 주석을 추가하거나 이슈를 생성하여 추적하는 것을 권장합니다.
해당 기능 구현을 위한 이슈를 생성해 드릴까요?
feature/mypage/src/main/java/com/umcspot/spot/mypage/waiting/WaitingStudyViewModel.kt (1)
26-50: 함수명이 실제 동작과 일치하지 않습니다.
loadParticipatingStudy()라는 이름이지만 실제로는getWaitingStudy()를 호출합니다.loadWaitingStudy()로 이름을 변경해주세요.로그 태그도
"HomeViewModel"에서"WaitingStudyViewModel"로 수정이 필요합니다.♻️ 수정 제안
- fun loadParticipatingStudy() { + fun loadWaitingStudy() { _uiState.update { it.copy(waitingStudy = UiState.Loading) } viewModelScope.launch { studyRepository.getWaitingStudy( cursor = null, size = 10 ) .onSuccess { info -> _uiState.update { it.copy( waitingStudy = if (info.studyList.isEmpty()) { UiState.Empty } else { UiState.Success(info) } ) } } .onFailure { e -> - Log.e("HomeViewModel", "loadParticipatingStudy error", e) -// _uiState.update { it.copy(weatherInfo = UiState.Failure(e.message ?: "날씨 불러오기 실패")) } + Log.e("WaitingStudyViewModel", "loadWaitingStudy error", e) + _uiState.update { it.copy(waitingStudy = UiState.Failure(e.message ?: "로딩 실패")) } } } }
Comment on lines
+174
to
+175
| }.onFailure { e -> | ||
| Log.e("StudyRepository", "getRecruitingStudy failed", e) |
There was a problem hiding this comment.
복사-붙여넣기 오류: 잘못된 로그 메시지.
getWaitingStudy 메서드에서 로그 메시지가 "getRecruitingStudy failed"로 잘못 표시되어 있습니다. 디버깅 시 혼란을 줄 수 있습니다.
🐛 제안하는 수정
}.onFailure { e ->
- Log.e("StudyRepository", "getRecruitingStudy failed", e)
+ Log.e("StudyRepository", "getWaitingStudy failed", e)
}📝 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
| }.onFailure { e -> | |
| Log.e("StudyRepository", "getRecruitingStudy failed", e) | |
| }.onFailure { e -> | |
| Log.e("StudyRepository", "getWaitingStudy failed", e) |
🤖 Prompt for AI Agents
In
`@data/study/src/main/java/com/umcspot/spot/study/repositoryimpl/StudyRepositoryImpl.kt`
around lines 174 - 175, getWaitingStudy의 onFailure 블록에서 복사·붙여넣기로 인해 로그 메시지가 잘못
`"getRecruitingStudy failed"`로 기록되어 있으니, StudyRepositoryImpl의 getWaitingStudy
메서드 내부 onFailure 핸들러(Log.e 호출)를 찾아 로그 메시지를 올바르게 `"getWaitingStudy failed"`로 수정해
주세요; 필요하면 메시지에 예외 정보(e)를 그대로 포함시켜 디버깅 정보를 보존하세요.
Comment on lines
20
to
+21
| @SerialName("body") | ||
| val body: WeatherBody | ||
| val body: WeatherBody? = null |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for all usages of response.body to check null handling
rg -n --type=kotlin 'response\.body' -C3Repository: SPOTeam/Android-V2
Length of output: 814
🏁 Script executed:
#!/bin/bash
# Search for all usages of toDomain() to see error handling context
rg -n --type=kotlin 'toDomain\(\)' -C3
# Also search for WeatherResponseDto usages
rg -n --type=kotlin 'WeatherResponseDto' -C2
# Check if body is accessed in other ways
rg -n --type=kotlin '\.body' data/weather -C2Repository: SPOTeam/Android-V2
Length of output: 24024
🏁 Script executed:
#!/bin/bash
# Check the full context around the toDomain() call in WeatherRepositoryImpl
sed -n '60,85p' data/weather/src/main/java/com/umcspot/spot/weather/repositoryimpl/WeatherRepositoryImpl.ktRepository: SPOTeam/Android-V2
Length of output: 854
response.body!! 사용으로 null 처리 오류 발생 위험
body 필드를 nullable로 변경한 것은 API 응답에서 body가 누락될 수 있는 상황을 처리하기 위한 것으로 보입니다. 그러나 WeatherMapper.kt 라인 10에서 response.body!!.items.item으로 non-null assertion을 사용하고 있어, body가 실제로 null인 경우 NullPointerException이 발생합니다.
현재는 WeatherRepositoryImpl.kt에서 runCatching { }으로 감싸져 있어 예외가 처리되지만, 이는 임시 방편입니다. 매퍼 함수 내에서 null 케이스를 명시적으로 처리하도록 수정하세요.
WeatherMapper.kt 라인 10
val items = response.body!!.items.item🤖 Prompt for AI Agents
In
`@data/weather/src/main/java/com/umcspot/spot/weather/dto/response/WeatherResponseDto.kt`
around lines 20 - 21, WeatherMapper currently uses response.body!! (val items =
response.body!!.items.item) which will NPE if body is null; update the mapper to
handle the nullable body field explicitly by replacing the non-null assertion
with a safe-check: detect when response.body is null and either return an empty
items list or a default Weather DTO (or throw a controlled exception type), then
map items.item only when body is non-null; ensure the mapper function name where
this occurs (the WeatherMapper mapping function that computes items) is updated
accordingly and adjust callers (e.g., WeatherRepositoryImpl) to rely on the
mapper's explicit null-handling instead of relying on runCatching to absorb
NPEs.
Comment on lines
29
to
51
| fun loadParticipatingStudy() { | ||
| _uiState.update { it.copy(participatingStudy = UiState.Loading) } | ||
|
|
||
| viewModelScope.launch { | ||
| studyRepository.getParticipatingStudy( | ||
| cursor = null, | ||
| size = 10 | ||
| ) | ||
| .onSuccess { info -> | ||
| _uiState.update { | ||
| it.copy( | ||
| participatingStudy = if (info.studyList.isEmpty()) { | ||
| UiState.Empty | ||
| } else { | ||
| UiState.Success(info) | ||
| } | ||
| ) | ||
| } | ||
| } | ||
| .onFailure { e -> | ||
| Log.e("HomeViewModel", "loadParticipatingStudy error", e) | ||
| // _uiState.update { it.copy(weatherInfo = UiState.Failure(e.message ?: "날씨 불러오기 실패")) } | ||
| } |
There was a problem hiding this comment.
실패 시 UiState를 Failure로 전환하세요. 지금은 로그만 찍어 화면이 Loading에 머물 수 있습니다.
✅ 수정 제안
.onFailure { e ->
Log.e("HomeViewModel", "loadParticipatingStudy error", e)
-// _uiState.update { it.copy(weatherInfo = UiState.Failure(e.message ?: "날씨 불러오기 실패")) }
+ _uiState.update {
+ it.copy(participatingStudy = UiState.Failure(e.message ?: "참여 스터디 불러오기 실패"))
+ }
}🤖 Prompt for AI Agents
In
`@feature/mypage/src/main/java/com/umcspot/spot/mypage/participating/ParticipatingStudyViewModel.kt`
around lines 29 - 51, The onFailure branch of loadParticipatingStudy currently
only logs the error and leaves _uiState in Loading; update the onFailure { e ->
... } handler for studyRepository.getParticipatingStudy to set _uiState.update {
it.copy(participatingStudy = UiState.Failure(e.message ?: "참여 스터디 불러오기 실패")) }
(note: use participatingStudy field, not the commented weatherInfo) so the UI
transitions to Failure with a helpful message.
...re/mypage/src/main/java/com/umcspot/spot/mypage/participating/ParticipatingStudyViewModel.kt
Show resolved
Hide resolved
feature/mypage/src/main/java/com/umcspot/spot/mypage/recruiting/RecruitingStudyViewModel.kt
Show resolved
Hide resolved
feature/mypage/src/main/java/com/umcspot/spot/mypage/waiting/WaitingStudyViewModel.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In
`@feature/mypage/src/main/java/com/umcspot/spot/mypage/participating/ParticipatingStudyViewModel.kt`:
- Line 49: Update the Log.e tag string in the ParticipatingStudyViewModel to
correctly identify the class: replace the current Log.e("HomeViewModel",
"loadParticipatingStudy error", e) call inside the loadParticipatingStudy method
of ParticipatingStudyViewModel with Log.e("ParticipatingStudyViewModel",
"loadParticipatingStudy error", e) so logs correctly indicate the source class.
- Line 78: Replace the incorrect log tag "HomeViewModel" with
"ParticipatingStudyViewModel" in the error logging call so the log entry is
associated with the correct class; locate the Log.e call (currently
Log.e("HomeViewModel", "loadNextpageError", e)) inside the
ParticipatingStudyViewModel (e.g., in the loadNextPage / error handling block)
and update the first argument to "ParticipatingStudyViewModel" to match the
class and other logs.
In
`@feature/mypage/src/main/java/com/umcspot/spot/mypage/recruiting/RecruitingStudyViewModel.kt`:
- Around line 26-50: Rename the function loadParticipatingStudy to
loadRecruitingStudy (function symbol: loadParticipatingStudy) and update all
call sites accordingly; inside the function replace the incorrect log tag
"HomeViewModel" with the proper class/tag (e.g., "RecruitingStudyViewModel");
implement failure handling by replacing the commented-out line with
_uiState.update { it.copy(recruitingStudy = UiState.Failure(e.message ?: "모집 스터디
불러오기 실패")) } in the onFailure block (referencing _uiState.update and UiState),
and remove the leftover commented code.
In
`@feature/mypage/src/main/java/com/umcspot/spot/mypage/waiting/WaitingStudyViewModel.kt`:
- Around line 45-48: Update the onFailure handler in WaitingStudyViewModel:
replace the incorrect Log.e tag "HomeViewModel" with the correct class/tag
(e.g., "WaitingStudyViewModel") and restore/update the UI state by calling
_uiState.update { it.copy(weatherInfo = UiState.Failure(e.message ?: "날씨 불러오기
실패")) } (or the appropriate UiState field used in this ViewModel) so failures
both log the error and surface UiState.Failure to the UI.
- Line 26: Rename the misnamed function loadParticipatingStudy to
loadWaitingStudy inside WaitingStudyViewModel and update any internal call that
invokes getWaitingStudy so the renamed method still calls getWaitingStudy; also
update all callers/references to use loadWaitingStudy to keep the API consistent
(ensure method signature and visibility remain unchanged and run project-wide
refactor/rename to catch usages).
♻️ Duplicate comments (3)
feature/mypage/src/main/java/com/umcspot/spot/mypage/participating/ParticipatingStudyViewModel.kt (1)
48-51: 이전 리뷰에서 지적된 실패 처리 미구현 이슈가 아직 해결되지 않았습니다.현재
onFailure블록에서 로그만 출력하고UiState.Failure로 전환하지 않아 UI가Loading상태에 머물 수 있습니다.feature/mypage/src/main/java/com/umcspot/spot/mypage/waiting/WaitingStudyViewModel.kt (1)
74-76: 페이지네이션 에러 처리 개선이 필요합니다.로그 태그가
"HomeViewModel"로 잘못되어 있으며, 페이지네이션 실패 시 사용자에게 알림이 없습니다. 토스트 메시지나 스낵바 등으로 사용자에게 피드백을 제공하는 것을 고려해보세요.🐛 제안된 수정
}.onFailure { e -> - Log.e("HomeViewModel", "loadNextpageError", e) + Log.e("WaitingStudyViewModel", "loadNextPageError", e) + // TODO: 사용자에게 페이지네이션 실패 알림 추가 고려 }feature/mypage/src/main/java/com/umcspot/spot/mypage/recruiting/RecruitingStudyViewModel.kt (1)
74-76: 로그 태그가 여전히 잘못되어 있습니다이전 리뷰에서 제안된 수정 사항 중 로그 태그 변경이 적용되지 않았습니다. 디버깅 편의를 위해 올바른 클래스명으로 수정해 주세요.
🐛 제안된 수정
}.onFailure { e -> - Log.e("HomeViewModel", "loadNextpageError", e) + Log.e("RecruitingStudyViewModel", "loadNextPageError", e) }
🧹 Nitpick comments (3)
feature/mypage/src/main/java/com/umcspot/spot/mypage/waiting/WaitingStudyViewModel.kt (1)
20-24:uiState에도asStateFlow()사용을 고려해보세요.
isLoadingMore는asStateFlow()를 사용하고 있지만,uiState는 직접 할당하고 있습니다. 일관성을 위해 동일한 패턴 적용을 권장합니다.♻️ 제안된 수정
private val _uiState = MutableStateFlow(WaitingStudyState()) -val uiState: StateFlow<WaitingStudyState> = _uiState +val uiState: StateFlow<WaitingStudyState> = _uiState.asStateFlow()feature/mypage/src/main/java/com/umcspot/spot/mypage/recruiting/RecruitingStudyViewModel.kt (2)
20-24: StateFlow 노출 방식 불일치
uiState는 타입 선언만으로 노출하고,isLoadingMore는.asStateFlow()를 사용합니다. 일관성을 위해 둘 다.asStateFlow()를 사용하는 것이 좋습니다.♻️ 제안된 수정
private val _uiState = MutableStateFlow(RecruitingStudyState()) - val uiState: StateFlow<RecruitingStudyState> = _uiState + val uiState: StateFlow<RecruitingStudyState> = _uiState.asStateFlow()
45-48: 실패 시 UI 상태 업데이트 누락두 함수 모두
onFailure에서 로그만 기록하고 UI 상태를UiState.Failure로 업데이트하지 않습니다. 사용자에게 오류 상황을 알리지 못해 UX가 저하될 수 있습니다. 최소한 초기 로딩 실패 시에는 에러 상태를 표시하는 것이 좋습니다.Also applies to: 74-76
...re/mypage/src/main/java/com/umcspot/spot/mypage/participating/ParticipatingStudyViewModel.kt
Outdated
Show resolved
Hide resolved
...re/mypage/src/main/java/com/umcspot/spot/mypage/participating/ParticipatingStudyViewModel.kt
Outdated
Show resolved
Hide resolved
Comment on lines
26
to
50
There was a problem hiding this comment.
함수명과 실제 동작 불일치 및 로그 태그 오류
- 함수명이
loadParticipatingStudy이지만 실제로는getRecruitingStudy를 호출합니다.loadRecruitingStudy로 이름을 변경해야 합니다. - 로그 태그가
"HomeViewModel"로 되어 있어 디버깅 시 혼란을 줄 수 있습니다. - Line 47의 주석 처리된 코드는 제거하거나 실패 상태 처리를 구현해야 합니다.
🐛 제안된 수정
- fun loadParticipatingStudy() {
+ fun loadRecruitingStudy() {
_uiState.update { it.copy(recruitingStudy = UiState.Loading) }
viewModelScope.launch {
studyRepository.getRecruitingStudy(
cursor = null,
size = 10
)
.onSuccess { info ->
_uiState.update {
it.copy(
recruitingStudy = if (info.studyList.isEmpty()) {
UiState.Empty
} else {
UiState.Success(info)
}
)
}
}
.onFailure { e ->
- Log.e("HomeViewModel", "loadParticipatingStudy error", e)
-// _uiState.update { it.copy(weatherInfo = UiState.Failure(e.message ?: "날씨 불러오기 실패")) }
+ Log.e("RecruitingStudyViewModel", "loadRecruitingStudy error", e)
+ _uiState.update { it.copy(recruitingStudy = UiState.Failure(e.message ?: "스터디 불러오기 실패")) }
}
}
}📝 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
| fun loadParticipatingStudy() { | |
| _uiState.update { it.copy(recruitingStudy = UiState.Loading) } | |
| viewModelScope.launch { | |
| studyRepository.getRecruitingStudy( | |
| cursor = null, | |
| size = 10 | |
| ) | |
| .onSuccess { info -> | |
| _uiState.update { | |
| it.copy( | |
| recruitingStudy = if (info.studyList.isEmpty()) { | |
| UiState.Empty | |
| } else { | |
| UiState.Success(info) | |
| } | |
| ) | |
| } | |
| } | |
| .onFailure { e -> | |
| Log.e("HomeViewModel", "loadParticipatingStudy error", e) | |
| // _uiState.update { it.copy(weatherInfo = UiState.Failure(e.message ?: "날씨 불러오기 실패")) } | |
| } | |
| } | |
| } | |
| fun loadRecruitingStudy() { | |
| _uiState.update { it.copy(recruitingStudy = UiState.Loading) } | |
| viewModelScope.launch { | |
| studyRepository.getRecruitingStudy( | |
| cursor = null, | |
| size = 10 | |
| ) | |
| .onSuccess { info -> | |
| _uiState.update { | |
| it.copy( | |
| recruitingStudy = if (info.studyList.isEmpty()) { | |
| UiState.Empty | |
| } else { | |
| UiState.Success(info) | |
| } | |
| ) | |
| } | |
| } | |
| .onFailure { e -> | |
| Log.e("RecruitingStudyViewModel", "loadRecruitingStudy error", e) | |
| _uiState.update { it.copy(recruitingStudy = UiState.Failure(e.message ?: "스터디 불러오기 실패")) } | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
`@feature/mypage/src/main/java/com/umcspot/spot/mypage/recruiting/RecruitingStudyViewModel.kt`
around lines 26 - 50, Rename the function loadParticipatingStudy to
loadRecruitingStudy (function symbol: loadParticipatingStudy) and update all
call sites accordingly; inside the function replace the incorrect log tag
"HomeViewModel" with the proper class/tag (e.g., "RecruitingStudyViewModel");
implement failure handling by replacing the commented-out line with
_uiState.update { it.copy(recruitingStudy = UiState.Failure(e.message ?: "모집 스터디
불러오기 실패")) } in the onFailure block (referencing _uiState.update and UiState),
and remove the leftover commented code.
There was a problem hiding this comment.
함수명과 실제 동작이 일치하지 않습니다.
loadParticipatingStudy라는 함수명이지만 실제로는 getWaitingStudy를 호출합니다. 함수명을 loadWaitingStudy로 변경하여 명확성을 높이세요.
🐛 제안된 수정
-fun loadParticipatingStudy() {
+fun loadWaitingStudy() {📝 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
| fun loadParticipatingStudy() { | |
| fun loadWaitingStudy() { |
🤖 Prompt for AI Agents
In
`@feature/mypage/src/main/java/com/umcspot/spot/mypage/waiting/WaitingStudyViewModel.kt`
at line 26, Rename the misnamed function loadParticipatingStudy to
loadWaitingStudy inside WaitingStudyViewModel and update any internal call that
invokes getWaitingStudy so the renamed method still calls getWaitingStudy; also
update all callers/references to use loadWaitingStudy to keep the API consistent
(ensure method signature and visibility remain unchanged and run project-wide
refactor/rename to catch usages).
Comment on lines
45
to
48
| .onFailure { e -> | ||
| Log.e("HomeViewModel", "loadParticipatingStudy error", e) | ||
| // _uiState.update { it.copy(weatherInfo = UiState.Failure(e.message ?: "날씨 불러오기 실패")) } | ||
| } |
There was a problem hiding this comment.
에러 처리 시 UI 상태가 업데이트되지 않습니다.
실패 시 로그만 남기고 UI 상태를 UiState.Failure로 업데이트하지 않아 사용자에게 에러 피드백이 전달되지 않습니다. 또한 로그 태그가 "HomeViewModel"로 잘못되어 있습니다.
🐛 제안된 수정
.onFailure { e ->
- Log.e("HomeViewModel", "loadParticipatingStudy error", e)
-// _uiState.update { it.copy(weatherInfo = UiState.Failure(e.message ?: "날씨 불러오기 실패")) }
+ Log.e("WaitingStudyViewModel", "loadWaitingStudy error", e)
+ _uiState.update { it.copy(waitingStudy = UiState.Failure(e.message ?: "대기 스터디 불러오기 실패")) }
}🤖 Prompt for AI Agents
In
`@feature/mypage/src/main/java/com/umcspot/spot/mypage/waiting/WaitingStudyViewModel.kt`
around lines 45 - 48, Update the onFailure handler in WaitingStudyViewModel:
replace the incorrect Log.e tag "HomeViewModel" with the correct class/tag
(e.g., "WaitingStudyViewModel") and restore/update the UI state by calling
_uiState.update { it.copy(weatherInfo = UiState.Failure(e.message ?: "날씨 불러오기
실패")) } (or the appropriate UiState field used in this ViewModel) so failures
both log the error and surface UiState.Failure to the UI.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@feature/mypage/src/main/java/com/umcspot/spot/mypage/recruiting/RecruitingStudyScreen.kt`:
- Line 152: The Spacer calls in RecruitingStudyScreen.kt use
Modifier.padding(...) which doesn't create vertical space; replace those
Spacer(Modifier.padding(screenHeightDp(5.dp))) usages (both occurrences) with
Spacer(Modifier.height(screenHeightDp(5.dp))) so the Spacer provides the
intended vertical spacing in the composable layout.
♻️ Duplicate comments (3)
feature/mypage/src/main/java/com/umcspot/spot/mypage/waiting/WaitingStudyViewModel.kt (1)
45-48: 에러 발생 시 UI 상태가UiState.Failure로 업데이트되지 않습니다.실패 시 로그만 남기고 UI 상태를 업데이트하지 않아 사용자에게 에러 피드백이 전달되지 않습니다. 주석 처리된 코드도 제거하거나 올바르게 구현해야 합니다.
🐛 제안된 수정
.onFailure { e -> Log.e("WaitingStudyViewModel", "loadWaiting error", e) -// _uiState.update { it.copy(weatherInfo = UiState.Failure(e.message ?: "날씨 불러오기 실패")) } + _uiState.update { it.copy(waitingStudy = UiState.Failure(e.message ?: "대기 스터디 불러오기 실패")) } }feature/mypage/src/main/java/com/umcspot/spot/mypage/participating/ParticipatingStudyViewModel.kt (1)
48-51: 실패 시 UiState를 Failure로 전환해야 합니다.현재
onFailure블록에서 로그만 출력하고 있어, API 호출 실패 시 UI가Loading상태에 무한히 머물게 됩니다. 주석 처리된weatherInfo코드는 다른 ViewModel에서 복사한 흔적으로 보입니다.✅ 수정 제안
.onFailure { e -> Log.e("ParticipatingStudyViewModel", "loadParticipatingStudy error", e) -// _uiState.update { it.copy(weatherInfo = UiState.Failure(e.message ?: "날씨 불러오기 실패")) } + _uiState.update { + it.copy(participatingStudy = UiState.Failure(e.message ?: "참여 스터디 불러오기 실패")) + } }feature/mypage/src/main/java/com/umcspot/spot/mypage/recruiting/RecruitingStudyViewModel.kt (1)
45-48: 실패 상태가 UI에 전파되지 않음현재
onFailure블록에서 에러를 로깅만 하고 UI 상태를 업데이트하지 않아 사용자가 로딩 실패 시 아무런 피드백을 받지 못합니다. 또한 주석 처리된 코드에 "weatherInfo" 참조가 남아있어 제거해야 합니다.🐛 수정 제안
.onFailure { e -> Log.e("RecruitingStudyViewModel", "loadRecruitingStudy error", e) -// _uiState.update { it.copy(weatherInfo = UiState.Failure(e.message ?: "날씨 불러오기 실패")) } + _uiState.update { + it.copy(recruitingStudy = UiState.Failure(e.message ?: "스터디 불러오기 실패")) + } }
🧹 Nitpick comments (8)
feature/mypage/src/main/java/com/umcspot/spot/mypage/waiting/WaitingStudyViewModel.kt (2)
20-24:uiState에도asStateFlow()사용을 권장합니다.
isLoadingMore에는asStateFlow()를 사용하고uiState에는 직접 할당하는 방식이 일관성이 없습니다. 둘 다 동일한 패턴을 사용하면 코드 일관성이 향상됩니다.♻️ 제안된 수정
private val _uiState = MutableStateFlow(WaitingStudyState()) -val uiState: StateFlow<WaitingStudyState> = _uiState +val uiState: StateFlow<WaitingStudyState> = _uiState.asStateFlow()
30-33: 페이지 크기를 상수로 추출하는 것을 권장합니다.
size = 10이 두 곳에서 하드코딩되어 있습니다. 상수로 추출하면 유지보수성이 향상됩니다.♻️ 제안된 수정
`@HiltViewModel` class WaitingStudyViewModel `@Inject` constructor( private val studyRepository: StudyRepository ) : ViewModel() { companion object { private const val PAGE_SIZE = 10 } // ... 이후 size = PAGE_SIZE 로 사용 }Also applies to: 63-66
feature/mypage/src/main/java/com/umcspot/spot/mypage/participating/ParticipatingStudyViewModel.kt (2)
3-5: 사용되지 않는 import를 제거하세요.
Application,PackageManager,Build는 이 ViewModel에서 사용되지 않습니다. 다른 ViewModel에서 복사한 것으로 보입니다.♻️ 수정 제안
package com.umcspot.spot.mypage.participating -import android.app.Application -import android.content.pm.PackageManager -import android.os.Build import android.util.Log import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope
77-79: 페이지네이션 실패 시 사용자 피드백을 고려하세요.현재
loadNextPage실패 시 로그만 출력됩니다. 사용자가 다음 페이지 로드 실패를 인지하고 재시도할 수 있도록 토스트 메시지나 재시도 UI를 제공하는 것이 좋습니다.feature/mypage/src/main/java/com/umcspot/spot/mypage/recruiting/RecruitingStudyViewModel.kt (1)
20-24:uiState에.asStateFlow()누락Line 24의
isLoadingMore는.asStateFlow()를 사용하지만, Line 21의uiState는 직접 노출하고 있습니다. 일관성과 외부에서MutableStateFlow로 캐스팅되는 것을 방지하기 위해 동일한 패턴을 사용하는 것이 좋습니다.♻️ 수정 제안
private val _uiState = MutableStateFlow(RecruitingStudyState()) - val uiState: StateFlow<RecruitingStudyState> = _uiState + val uiState: StateFlow<RecruitingStudyState> = _uiState.asStateFlow()feature/mypage/src/main/java/com/umcspot/spot/mypage/recruiting/RecruitingStudyScreen.kt (3)
52-52: 파라미터 이름이 Kotlin 명명 규칙과 맞지 않음
viewmodel파라미터가 소문자 'm'을 사용하고 있습니다. Kotlin의 camelCase 명명 규칙에 따라viewModel로 변경하는 것이 좋습니다.♻️ 수정 제안
- viewmodel : RecruitingStudyViewModel = hiltViewModel() + viewModel: RecruitingStudyViewModel = hiltViewModel()함수 내부의
viewmodel사용도 함께viewModel로 변경해야 합니다.
104-119: Empty와 Failure 상태가 동일하게 처리됨
UiState.Empty와UiState.Failure가 동일한 UI를 표시하여 사용자가 "데이터 없음"과 "로딩 실패"를 구분할 수 없습니다. 의도된 디자인일 수 있지만, 에러 발생 시 다른 메시지나 재시도 버튼을 제공하는 것이 더 나은 UX가 될 수 있습니다.
177-186:indexOf()사용으로 인한 O(n²) 복잡도
studyList.indexOf(item)은 각 아이템마다 O(n) 검색을 수행하여 전체 리스트 렌더링이 O(n²)가 됩니다.itemsIndexed를 사용하면 인덱스를 직접 받아 성능을 개선할 수 있습니다.♻️ 수정 제안
- items( - items = studyList, - key = { it.id } - ) { item -> + itemsIndexed( + items = studyList, + key = { _, item -> item.id } + ) { index, item -> Spacer(Modifier.height(screenHeightDp(5.dp))) Box( modifier = Modifier.fillMaxWidth(), contentAlignment = Alignment.TopStart ) { StudyListItem( item = item, modifier = Modifier.fillMaxWidth(), onClick = { onStudyClick(item.id) }, checkAppliedSlot = { TextButton( modifier = Modifier .width(screenWidthDp(60.dp)) .height(screenHeightDp(26.dp)), text = "신청 확인", style = SpotTheme.typography.regular_500, onClick = moveToCheckApplied, state = TextButtonState.B500State, shape = SpotShapes.Hard ) } ) } - if (studyList.indexOf(item) != studyList.lastIndex) { + if (index != studyList.lastIndex) { Spacer(Modifier.height(screenHeightDp(5.dp))) HorizontalDivider( modifier = Modifier .fillMaxWidth(), color = SpotTheme.colors.G300, thickness = 1.dp ) } }
feature/mypage/src/main/java/com/umcspot/spot/mypage/recruiting/RecruitingStudyScreen.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@feature/mypage/src/main/java/com/umcspot/spot/mypage/participating/ParticipatingStudyScreen.kt`:
- Around line 187-191: The meetball Icon used in ParticipatingStudyScreen is
interactive but has contentDescription=null; update this Icon to supply a
meaningful, localized content description (use stringResource with a new or
existing string like "meetball menu" / R.string.meetball_menu) so screen readers
convey its purpose; ensure the description is non-null for the clickable element
and update any related string resources accordingly.
In
`@feature/mypage/src/main/java/com/umcspot/spot/mypage/waiting/WaitingStudyScreen.kt`:
- Around line 146-173: The divider condition currently uses
studyList.indexOf(item) which is O(n) per item and breaks if duplicate items
exist; replace the items(...) call with itemsIndexed(...) so you receive the
index directly (use the index parameter to compare to lastIndex), update the
lambda to accept (index, item) and use index != studyList.lastIndex for the
divider check, and add the import androidx.compose.foundation.lazy.itemsIndexed
at the top of WaitingStudyScreen.kt.
- Around line 94-102: The loading branch currently passes a pre-scaled value to
SpotSpinner causing double scaling; in the UiState.Loading block, stop wrapping
the size with screenWidthDp and pass the raw Dp (e.g., 30.dp) to SpotSpinner so
SpotSpinner's internal screenWidthDp(size) handles scaling; locate the
SpotSpinner call in WaitingStudyScreen (UiState.Loading) and replace
screenWidthDp(30.dp) with the unscaled size constant, or alternatively adjust
SpotSpinner to accept a pre-scaled value and document that change.
🧹 Nitpick comments (3)
feature/mypage/src/main/java/com/umcspot/spot/mypage/participating/ParticipatingStudyScreen.kt (3)
112-127:Failure상태와Empty상태를 동일하게 처리하면 사용자 경험이 저하될 수 있습니다.현재 두 상태 모두 동일한 "참여 중인 스터디가 아직 없어요!" 메시지를 표시합니다. 네트워크 오류나 서버 에러(
Failure) 발생 시 사용자가 문제를 인지하기 어렵고, 재시도 옵션도 제공되지 않습니다.♻️ Failure 상태 분리 처리 제안
- is UiState.Empty, is UiState.Failure -> { + is UiState.Empty -> { Box( modifier = Modifier .fillMaxSize() .background(SpotTheme.colors.white), contentAlignment = Alignment.Center ) { EmptyAlertWithButton( alertTitle = "참여 중인 스터디가 아직 없어요!", alertDes = "스팟에서 내 목표를 이뤄봐요", buttonText = "스터디 둘러보기", painter = painterResource(R.drawable.study_default), onClick = { moveToRecruitingStudy() }, ) } } + is UiState.Failure -> { + Box( + modifier = Modifier + .fillMaxSize() + .background(SpotTheme.colors.white), + contentAlignment = Alignment.Center + ) { + EmptyAlertWithButton( + alertTitle = "데이터를 불러오지 못했어요", + alertDes = "네트워크 상태를 확인해주세요", + buttonText = "다시 시도", + painter = painterResource(R.drawable.study_default), + onClick = { viewmodel.loadParticipatingStudy() }, + ) + } + }
138-140: 미구현 콜백 함수들이 빈 람다로 설정되어 있습니다.PR 설명에 언급된 대로
onEditClick,onReportClick,onLeaveClick이 아직 구현되지 않았습니다. 사용자가 메뉴를 클릭해도 아무 동작이 없어 혼란을 줄 수 있습니다.추후 모달 구현 시 이 콜백들을 연결해야 합니다. 이 작업을 추적하기 위해 이슈를 생성할까요?
207-216:indexOf()호출로 인한 O(n²) 성능 문제
studyList.indexOf(item)은 각 아이템마다 O(n) 탐색을 수행하여 전체 O(n²) 복잡도가 됩니다. 리스트가 커지면 스크롤 성능에 영향을 줄 수 있습니다.♻️ itemsIndexed 사용으로 개선
- items( - items = studyList, - key = { it.id } - ) { item -> + itemsIndexed( + items = studyList, + key = { _, item -> item.id } + ) { index, item -> Spacer(Modifier.height(screenHeightDp(5.dp))) Box( // ... existing code ... ) - if (studyList.indexOf(item) != studyList.lastIndex) { + if (index != studyList.lastIndex) { Spacer(Modifier.height(screenHeightDp(5.dp))) HorizontalDivider( // ... existing code ... ) } }
Comment on lines
+187
to
+191
| Icon( | ||
| painter = painterResource(R.drawable.meetball), | ||
| contentDescription = null, | ||
| modifier = Modifier.size(screenWidthDp(14.dp)) | ||
| ) |
There was a problem hiding this comment.
접근성: 인터랙티브 아이콘에 contentDescription이 누락되었습니다.
미트볼 메뉴 아이콘은 클릭 가능한 요소이지만 contentDescription이 null로 설정되어 있어 스크린 리더 사용자가 이 버튼의 용도를 알 수 없습니다.
🔧 접근성 개선 제안
Icon(
painter = painterResource(R.drawable.meetball),
- contentDescription = null,
+ contentDescription = "더보기 메뉴",
modifier = Modifier.size(screenWidthDp(14.dp))
)📝 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
| Icon( | |
| painter = painterResource(R.drawable.meetball), | |
| contentDescription = null, | |
| modifier = Modifier.size(screenWidthDp(14.dp)) | |
| ) | |
| Icon( | |
| painter = painterResource(R.drawable.meetball), | |
| contentDescription = "더보기 메뉴", | |
| modifier = Modifier.size(screenWidthDp(14.dp)) | |
| ) |
🤖 Prompt for AI Agents
In
`@feature/mypage/src/main/java/com/umcspot/spot/mypage/participating/ParticipatingStudyScreen.kt`
around lines 187 - 191, The meetball Icon used in ParticipatingStudyScreen is
interactive but has contentDescription=null; update this Icon to supply a
meaningful, localized content description (use stringResource with a new or
existing string like "meetball menu" / R.string.meetball_menu) so screen readers
convey its purpose; ensure the description is non-null for the clickable element
and update any related string resources accordingly.
Comment on lines
+94
to
+102
| is UiState.Loading -> { | ||
| Box( | ||
| modifier = Modifier | ||
| .fillMaxSize() | ||
| .background(SpotTheme.colors.white), | ||
| contentAlignment = Alignment.Center | ||
| ) { | ||
| SpotSpinner(size = screenWidthDp(30.dp)) | ||
| } |
There was a problem hiding this comment.
스피너 크기 이중 스케일링 수정 필요
SpotSpinner 내부에서 이미 screenWidthDp(size)를 적용합니다. 여기서 screenWidthDp(30.dp)를 넘기면 크기가 두 번 스케일돼 과대 표시될 수 있어요.
🐛 제안 수정
- SpotSpinner(size = screenWidthDp(30.dp))
+ SpotSpinner(size = 30.dp)📝 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
| is UiState.Loading -> { | |
| Box( | |
| modifier = Modifier | |
| .fillMaxSize() | |
| .background(SpotTheme.colors.white), | |
| contentAlignment = Alignment.Center | |
| ) { | |
| SpotSpinner(size = screenWidthDp(30.dp)) | |
| } | |
| is UiState.Loading -> { | |
| Box( | |
| modifier = Modifier | |
| .fillMaxSize() | |
| .background(SpotTheme.colors.white), | |
| contentAlignment = Alignment.Center | |
| ) { | |
| SpotSpinner(size = 30.dp) | |
| } |
🤖 Prompt for AI Agents
In
`@feature/mypage/src/main/java/com/umcspot/spot/mypage/waiting/WaitingStudyScreen.kt`
around lines 94 - 102, The loading branch currently passes a pre-scaled value to
SpotSpinner causing double scaling; in the UiState.Loading block, stop wrapping
the size with screenWidthDp and pass the raw Dp (e.g., 30.dp) to SpotSpinner so
SpotSpinner's internal screenWidthDp(size) handles scaling; locate the
SpotSpinner call in WaitingStudyScreen (UiState.Loading) and replace
screenWidthDp(30.dp) with the unscaled size constant, or alternatively adjust
SpotSpinner to accept a pre-scaled value and document that change.
Comment on lines
+146
to
+173
| items( | ||
| items = studyList, | ||
| key = { it.id } | ||
| ) { item -> | ||
| Spacer(Modifier.height(screenHeightDp(5.dp))) | ||
|
|
||
| Box( | ||
| modifier = Modifier.fillMaxWidth(), | ||
| contentAlignment = Alignment.TopStart | ||
| ) { | ||
| StudyListItem( | ||
| item = item, | ||
| modifier = Modifier.fillMaxWidth(), | ||
| onClick = { onStudyClick(item.id) } | ||
| ) | ||
| } | ||
|
|
||
| if (studyList.indexOf(item) != studyList.lastIndex) { | ||
| Spacer(Modifier.height(screenHeightDp(5.dp))) | ||
|
|
||
| HorizontalDivider( | ||
| modifier = Modifier | ||
| .fillMaxWidth(), | ||
| color = SpotTheme.colors.G300, | ||
| thickness = 1.dp | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
divider 조건에서 indexOf 사용으로 성능/정확성 저하
studyList.indexOf(item)는 매 항목마다 O(n)이고, 동일한 값이 중복될 경우 마지막 항목에서도 divider가 그려질 수 있습니다. itemsIndexed로 인덱스를 받아 비교하세요.
🐛 제안 수정
- items(
- items = studyList,
- key = { it.id }
- ) { item ->
+ itemsIndexed(
+ items = studyList,
+ key = { _, item -> item.id }
+ ) { index, item ->
Spacer(Modifier.height(screenHeightDp(5.dp)))
Box(
modifier = Modifier.fillMaxWidth(),
contentAlignment = Alignment.TopStart
) {
StudyListItem(
item = item,
modifier = Modifier.fillMaxWidth(),
onClick = { onStudyClick(item.id) }
)
}
- if (studyList.indexOf(item) != studyList.lastIndex) {
+ if (index != studyList.lastIndex) {
Spacer(Modifier.height(screenHeightDp(5.dp)))
HorizontalDivider(
modifier = Modifier
.fillMaxWidth(),
color = SpotTheme.colors.G300,
thickness = 1.dp
)
}
}추가로 import androidx.compose.foundation.lazy.itemsIndexed가 필요합니다.
📝 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
| items( | |
| items = studyList, | |
| key = { it.id } | |
| ) { item -> | |
| Spacer(Modifier.height(screenHeightDp(5.dp))) | |
| Box( | |
| modifier = Modifier.fillMaxWidth(), | |
| contentAlignment = Alignment.TopStart | |
| ) { | |
| StudyListItem( | |
| item = item, | |
| modifier = Modifier.fillMaxWidth(), | |
| onClick = { onStudyClick(item.id) } | |
| ) | |
| } | |
| if (studyList.indexOf(item) != studyList.lastIndex) { | |
| Spacer(Modifier.height(screenHeightDp(5.dp))) | |
| HorizontalDivider( | |
| modifier = Modifier | |
| .fillMaxWidth(), | |
| color = SpotTheme.colors.G300, | |
| thickness = 1.dp | |
| ) | |
| } | |
| } | |
| itemsIndexed( | |
| items = studyList, | |
| key = { _, item -> item.id } | |
| ) { index, item -> | |
| Spacer(Modifier.height(screenHeightDp(5.dp))) | |
| Box( | |
| modifier = Modifier.fillMaxWidth(), | |
| contentAlignment = Alignment.TopStart | |
| ) { | |
| StudyListItem( | |
| item = item, | |
| modifier = Modifier.fillMaxWidth(), | |
| onClick = { onStudyClick(item.id) } | |
| ) | |
| } | |
| if (index != studyList.lastIndex) { | |
| Spacer(Modifier.height(screenHeightDp(5.dp))) | |
| HorizontalDivider( | |
| modifier = Modifier | |
| .fillMaxWidth(), | |
| color = SpotTheme.colors.G300, | |
| thickness = 1.dp | |
| ) | |
| } | |
| } |
🤖 Prompt for AI Agents
In
`@feature/mypage/src/main/java/com/umcspot/spot/mypage/waiting/WaitingStudyScreen.kt`
around lines 146 - 173, The divider condition currently uses
studyList.indexOf(item) which is O(n) per item and breaks if duplicate items
exist; replace the items(...) call with itemsIndexed(...) so you receive the
index directly (use the index parameter to compare to lastIndex), update the
lambda to accept (index, item) and use index != studyList.lastIndex for the
divider check, and add the import androidx.compose.foundation.lazy.itemsIndexed
at the top of WaitingStudyScreen.kt.
This was referenced Jan 22, 2026
Merged
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.
Related issue 🛠
Work Description 📝
Screenshot 📸
Uncompleted Tasks 😅
PR Point 📌
트러블 슈팅 💥
Summary by CodeRabbit
새로운 기능
개선
✏️ Tip: You can customize this high-level summary in your review settings.