[Campus][Feature] 분실물 QA 수정 1차

[KYM-P]

PR 개요

이슈 번호: #1214

PR 체크리스트

• Code convention을 잘 지켰나요?
• Lint check를 수행하였나요?
• Assignees를 추가했나요?

작업사항

• 버그 수정
• 신규 기능
• 코드 스타일 수정 (포맷팅 등)
• 리팩토링 (기능 수정 X, API 수정 X)
• 기타

작업사항의 상세한 설명

리스트 정보가 가 2번 fetch 되는 현상을 수정했습니다.

• searchQuery 를 stateFlow 로 구독하면서 방출되는 emit 이 뒤늦게 fetch 되는 현상을 수정했습니다.
  drop 으로 버려도 state 가 바뀌며 계속 호출되고 stateFlow 만 쓰자니 처음 fetch 되는 속도가 너무 느려 내부 bool 값으로 처리했습니다.

bottomSheet 에서 다른 화면으로 넘어갈 때 늦게 닫히는 현상을 수정했습니다.

• 내부에 scope 를 통하여 sheetState.hide() 를 직접 호출해 닫습니다. 여는 작업은 uiState 로 하기에 false 반환은 계속 수행합니다.

필터 선택에서 분실물 종류는 중복 선택이 가능하도록 바꿨습니다.

• 전체 비활성 상태에서 전체를 누르면 모든 선택이 사라지고 전체만 남습니다.
• 전체만 활성 상태에서 다른 선택지를 누르면 전체가 취소되고 다른 선택지가 선택됩니다.
• 같은 항목을 재 클릭 시 클릭 이벤트 자체를 취소합니다.

논의 사항

스크린샷

추가내용

• develop, sprint 브랜치를 향하고 있습니다
• production 브랜치를 향하고 있습니다

[JaeYoung290]

수고 많으셨습니다!
코멘트 한 번만 확인 부탁드려요!

[JaeYoung290]

launch는 비동기 실행이라 hide 애니메이션이 실행되는 동시에 onDismissRequest()가 같이 실행되지 않나요?
혹시 애니메이션이 끊기거나 하는 증상은 없었나요?

[KYM-P]

👍 애니메이션이 끊겨도 상관 없을 것 같습니다. 어차피 기존에는 dismiss 밖에 없었어서

[JaeYoung290]

이 부분에서 아이템 개수를 3개로 고정한 이유가 있으실까요?
없다면 flowrow로 대응하는 건 어떤가요?

[kongwoojin]

굳이 첫번째를 제외하는 이유가 있나요

[KYM-P]

리스트 화면 진입 시 fetch 가 2번 일어나던 원흉입니다.
observe 하면서 첫 emit 에 대해 fetchLostAndFoundItem() 를 실행하기에 이를 막고자 제외했습니다.
drop 으로 해보려 했지만 state 의 변경을 observe 하기에 state 변경마다 호출되어서 제대로 하기 어렵더라구요

[KYM-P]

그렇다고 init 에서 호출하는 fetchLostAndFoundItem() 를 빼자니 stateFlow 의 호출 시간이 너무 느려서 보기에 안좋았어요

[kongwoojin]

처음 값은 쓰레기값이다라는 가정하에 작동하는 코드인데, 좋은 방법은 아닌 것 같네요

[KYM-P]

흠 동의는 합니다 좀 어거지로 첫 emit 을 무시하거든요.
다만 자체 debounce를 주고있어서 해당 stateFlow 만으로는 처음 사용자에게 [빈 리스트 -> 로딩] 의 과정을 보여주게 되어 불편하더라구요.

[kongwoojin]

빈 리스트 -> 로딩 이 부분이 이해가 안되네요
애초에 로딩을 보여주면 되는거 아닌가요?

[KYM-P]

굳이 첫 화면부터 debounce 로 0.3~5초 간의 딜레이를 주는게 별로였습니다.
또 지금 과정은 진입 시 [검색 결과 없음 화면 -> 로딩 -> 리스트] 과정을 보여주어서 개인적으로 좋아 보이지 않습니다.
차라리 [로딩 -> 리스트] 라면 더 좋다고 생각합니다.
검색 debounce 를 0.1초로 하던가 시작부터 isLoading 을 true 로 주는 것이 그 대안입니다.
아니면 그냥 stateFlow 만 쓰고 빈 리스트 화면을 보여주거나요.

[KYM-P]

생각해보니 .distinctUntilChanged() 후에 drop 을 하면 내부 flag 변수 없이 처리가 가능했네요.

[kongwoojin]

왜 PR이 이 브랜치로 머지되는건가요 @JaeYoung290

[JaeYoung290]

qa1 브랜치에서 qa 수정한 다음 base 브랜치로 머지하기로 해서 여기로 머지했습니다!

[KYM-P]

바보같이 중복제거 이후에 drop 을 하면 해결되는데 너무 먼 길을 돌아갔네요.
drop(1) 로 수정했습니다.

[kongwoojin]

동일한 함수를 두번 호출하고, 하나는 drop(1)로 날리는 로직은 매우 안 좋아보입니다.

String 값 변화를 굳이 ViewModel에서 collect 하는 것도 이해가 되지 않고요. (ViewModel의 생명주기가 더 긴데, Activity는 destroy 되고 ViewModel이 살아있다면, 불필요한 API fetch가 일어날 수 있어보이네요)

Composable에서 아래와 같이 처리하고

LaunchedEffect(Unit) {
    snapshotFlow { uiState.searchQuery }.debounce(SEARCH_DEBOUNCE_MS)
        .distinctUntilChanged().collect {
            viewModel.fetchLostAndFoundItem()
        }
}

init에서 호출하던 API fetch 호출을 제거한 다음, isFirstPageLoaded 변수로 막으면 초기 호출 여부를 체크하고 게시글이 존재하지 않습니다를 보여줄 지 분기처리하면 될 것 같습니다.