Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1.0.3] - 페이지네이션 버그 수정 및 개선 #123

Merged
merged 13 commits into from
Sep 17, 2024

Conversation

ShapeKim98
Copy link
Contributor

#️⃣연관된 이슈

#120

📝작업 내용

이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)

  • 빠르게 스크롤 시 페이지네이션이 멈추는 현상 수정
  • 조회했던 페이징 번호까지 다시 재조회하는 로직 추가

--------------- 추가 사항 -----------------

  • 미분류에서 링크 삭제가 안되는 문제 수정
  • 목록이 보여지는 뷰에서의 하단 패딩 수정
  • SearchFeatureDateFormat 캐싱 적용
  • ContentDetail에서 링크 삭제시 부모뷰에 반영이 안되는 문제 수정

스크린샷 (선택)

ScreenRecording_09-16-2024.21-13-38_1.mov

💬리뷰 요구사항(선택)

리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요

  1. 페이지네이션 멈춤 현상은 비동기 오류가 아닌 PokitLoading뷰의 생명주기와 연관이 있었습니다.
    페이지 증가 조회를 하면서 액션에 대한 애니메이션을 넣었었는데, 원래라면 LazyVStack으로 인해 PokitLoading뷰가 소멸 생성을 반복 했어야 했지만 애니메이션 때문에 생명주기가 늦춰지면서,
    PokitLoading뷰가 소멸이 안돼서 onAppear가 한번만 발생하게 문제였습니다.
    그래서 페이지 증가 조회에 대한 애니메이션을 전부 제거하였습니다.
  2. 컨텐츠 목록, 컨텐츠 검색에도 페이징 재조회 이슈가 있어서, 아예 재조회 로직을 만들었습니다.
    저희 페이징 조회는 똑같은 size로만 조회를 해야하기 때문에, 0부터 현제 페이지 까지 순차적으로 조회를 해야했습니다.
    물론 사용자가 목록에서 목록 아이템에 대한 수정, 추가, 삭제 시 해당 사항들을 반영하기 위해, 목록을 초기화 시키고 재조회 해도 되지만...
    그렇게 하면 사용자가 스크롤한 위치를 잃어버리고, 안좋은 사용자 경험으로 이어지기 때문에... 이를 방지하고자,
    아래와 같은 로직으로 페이징을 재조회 하는 방식을 택했습니다.
    특히 @ITlearning 님께서 요모저모 노션에 적어주신 Concurrency 정말 많이 도움이 됐습니다.. Concurrency공부 그걸로 하고 있어요 ㅎㅎ (AsyncStream에 대한 내용은 없지만)
case .페이징_재조회:
    return .run { [
        pageable = state.domain.pageable,
        contentType = state.contentType
    ] send in
        let stream = AsyncThrowingStream<BaseContentListInquiry, Error> { continuation in
            Task {
                for page in 0...pageable.page {
                    let paeagableRequest = BasePageableRequest(
                        page: page,
                        size: pageable.size,
                        sort: pageable.sort
                    )
                    switch contentType {
                    case .favorite:
                        let contentList = try await remindClient.즐겨찾기_링크모음_조회(
                            paeagableRequest
                        ).toDomain()
                        continuation.yield(contentList)
                    case .unread:
                        let contentList = try await remindClient.읽지않음_컨텐츠_조회(
                            paeagableRequest
                        ).toDomain()
                        continuation.yield(contentList)
                    }
                }
                continuation.finish()
            }
        }
        var contentItems: BaseContentListInquiry? = nil
        for try await contentList in stream {
            let items = contentItems?.data ?? []
            let newItems = contentList.data ?? []
            contentItems = contentList
            contentItems?.data = items + newItems
        }
        guard let contentItems else { return }
        await send(.inner(.컨텐츠_목록_갱신(contentItems)), animation: .pokitSpring)
    }
  • 페이징 조회는 비동기적으로 작동하기 때문에, 이를 순차적으로 동작시키기 위하여 AsyncThrowingStream을 사용하여 0부터 현재 페이지 까지의 조회 스트림을 만들어 줍니다.
let stream = AsyncThrowingStream<BaseContentListInquiry, Error> { continuation in
...
  • 0부터 현재 페이지 까지 페이징 값에 대한 for-in loop를 돌면서 스트림에 api가 반환해준 값을 순차적으로 넣습니다.
Task {
    for page in 0...pageable.page {
        let paeagableRequest = BasePageableRequest(
            page: page,
            size: pageable.size,
            sort: pageable.sort
        )
        switch contentType {
        case .favorite:
            let contentList = try await remindClient.즐겨찾기_링크모음_조회(
                paeagableRequest
            ).toDomain()
            continuation.yield(contentList)
        case .unread:
            let contentList = try await remindClient.읽지않음_컨텐츠_조회(
                paeagableRequest
            ).toDomain()
            continuation.yield(contentList)
        }
    }
...
  • 스트림에 값을 다 넣은 후 명시적으로 스트림을 종료시켜줍니다
continuation.finish()
  • 스트림을 for-try-await-in loop를 사용하여 값을 꺼네옵니다.
var contentItems: BaseContentListInquiry? = nil
for try await contentList in stream {
    let items = contentItems?.data ?? []
    let newItems = contentList.data ?? []
    contentItems = contentList
    contentItems?.data = items + newItems
}
  1. 뭔가 페이징 로직이 좀 더 복잡해지는거 같아서.. 로직을 재사용가능하게 만들어서 Util단으로 빼거나 Modifier로 빼고 싶은데... 좀 더 고민해봐야 할 것 같아요...ㅎㅎ
  2. 작업하면서 PM QA 반영과 자잘한 버그들 수정하였습니다. (작업과 좀 겹치는 부분이 있어서..)

close 이슈번호

@ShapeKim98 ShapeKim98 added Fix 기능 수정 Bug 🔫 현재 발견된 버그를 수정하기 위함 labels Sep 16, 2024
@ShapeKim98 ShapeKim98 requested a review from stealmh September 16, 2024 13:12
@ShapeKim98 ShapeKim98 self-assigned this Sep 16, 2024
Copy link
Member

@stealmh stealmh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

추석연휴에 고생많으셨읍니다~~
AsyncStream으로 바꾸게 된 이유와 해결점이 명확해서 저도 하나배워갑니다. 다만 액션이 너무 거대해지고 있다는 생각이 들지 않으신가요?? 함수로 빼는 고민을 해보시는 것도 좋아보입니다.

DateFormat쪽 이름을 단순하게 가져가고 싶은데요
굳이 이름억지로 짓는거보다 case1,2,3처럼 두고 주석에서 어떤형식인지 알려주는게 적합하다고 생각이듭니다. (단순제안입니다)

스크린샷 2024-09-17 오전 11 17 03

나머지는 크게 문제 될 것 없어보여 approve 하겠습니다

@ShapeKim98
Copy link
Contributor Author

추석연휴에 고생많으셨읍니다~~ AsyncStream으로 바꾸게 된 이유와 해결점이 명확해서 저도 하나배워갑니다. 다만 액션이 너무 거대해지고 있다는 생각이 들지 않으신가요?? 함수로 빼는 고민을 해보시는 것도 좋아보입니다.

액션이 너무 거대해진다는 것... 너무 공감합니다.. 어떻게 분리할지 고민을 좀 해봐야 할 것 같아요...

DateFormat쪽 이름을 단순하게 가져가고 싶은데요 굳이 이름억지로 짓는거보다 case1,2,3처럼 두고 주석에서 어떤형식인지 알려주는게 적합하다고 생각이듭니다. (단순제안입니다)

스크린샷 2024-09-17 오전 11 17 03

전 좋습니다!

나머지는 크게 문제 될 것 없어보여 approve 하겠습니다

@ShapeKim98 ShapeKim98 merged commit 5afac01 into develop Sep 17, 2024
1 check passed
@stealmh stealmh linked an issue Oct 29, 2024 that may be closed by this pull request
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🔫 현재 발견된 버그를 수정하기 위함 Fix 기능 수정
Projects
None yet
Development

Successfully merging this pull request may close these issues.

release 1.0.3 준비작업
2 participants