Skip to content

Conversation

@clxxrlove
Copy link
Member

@clxxrlove clxxrlove commented Sep 4, 2025

🔗 관련 이슈

📘 작업 유형

  • ✨ Feature (기능 추가)
  • 🐞 Bugfix (버그 수정)
  • 🔧 Refactor (코드 리팩토링)
  • ⚙️ Chore (환경 설정)
  • 📝 Docs (문서 작성 및 수정)
  • ✅ Test (기능 테스트)
  • 🎨 style (코드 스타일 수정)

📙 작업 내역

  • 로딩 화면의 타이밍 조정 (0ms -> 500ms)

🧪 테스트 내역

  • 브라우저/기기에서 동작 확인
  • 엣지 케이스 테스트 완료
  • 기존 기능 영향 없음

🎨 스크린샷 또는 시연 영상 (선택)

Simulator.Screen.Recording.-.iPhone.SE.3rd.generation.-.2025-09-04.at.09.13.57.mp4

서버 로딩이 너무 빨라서 인디케이터가 보기 쉽지 않네요.
로딩 시간이 500ms를 넘어가야만 로딩 인디케이터가 나옵니다.

Summary by CodeRabbit

  • 신기능
    • 로딩 인디케이터가 기본 0.5초 지연 후 표시되어 매우 빠른 작업에서 깜빡임이 감소하고, 지연 시간 내 완료 시 자동 취소되어 흐름이 매끄러워집니다.
    • 오류 처리 UI가 시스템 알림에서 통일된 대화상자(BKDialog)로 변경되어 사용자에게 일관된 오류 메시지와 회복 동작을 제공합니다.

@coderabbitai
Copy link

coderabbitai bot commented Sep 4, 2025

Walkthrough

로딩 인디케이터가 지연 표시되도록 show(delay:) API가 추가되어 DispatchWorkItem으로 표시를 예약하고 중복 예약을 방지합니다. hide()는 예약 취소 후 인디케이터를 제거합니다. 또한 NoteCompletionViewController의 오류 알림이 UIAlertController에서 BKDialog 기반 presentErrorDialog()로 대체되었습니다.

Changes

Cohort / File(s) Change Summary
Loading Indicator 지연 표시 도입
src/Projects/BKDesign/Sources/Components/BKLoading/BKLoadingIndicator.swift
public static func show()public static func show(delay: TimeInterval = 0.5)로 변경. 표시를 DispatchWorkItem으로 지연 예약하고 delayedWorkItem로 중복 예약 차단. 예약된 항목을 실행해 로딩 뷰 생성·추가·애니메이션 시작. hide()는 예약 취소(delayedWorkItem?.cancel()), 참조 해제 및 기존 인디케이터 제거. import 순서 소폭 변경.
오류 표시를 BKDialog로 교체
src/Projects/BKPresentation/Sources/MainFlow/NoteCompletion/View/NoteCompletionViewController.swift
오류 처리 경로에서 인라인 UIAlertController 제거. _로 에러 무시 후 presentErrorDialog() 호출하도록 변경. presentErrorDialog() 추가: 제목 "오류", 좌측 버튼 텍스트 "기록을 불러올 수 없습니다.", 버튼 액션은 다이얼로그 닫기 및 뷰모델에 .errorHandled 전송.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor User
    participant VC as UIViewController
    participant LI as BKLoadingIndicator
    participant Win as Key Window

    User->>VC: showLoading()
    VC->>LI: show(delay=0.5)
    note right of LI #E6F7FF: delayedWorkItem 생성 및 예약\n중복 예약 있으면 반환
    LI-->>LI: (0.5s 후) 예약된 작업 실행
    LI->>Win: 로딩 뷰 생성/태그/추가
    LI->>LI: 애니메이션 시작

    User->>VC: hideLoading()
    VC->>LI: hide()
    LI->>LI: delayedWorkItem?.cancel() 및 해제
    LI->>Win: 로딩 뷰 제거
Loading
sequenceDiagram
    autonumber
    participant VC as NoteCompletionViewController
    participant VM as ViewModel
    participant Dialog as BKDialog

    VM-->>VC: error 이벤트
    VC->>VC: presentErrorDialog()
    VC->>Dialog: 생성(title:"오류", leftBtn:"기록을 불러올 수 없습니다.")
    Dialog->>VC: 버튼액션
    VC->>VM: .errorHandled
    VC->>VC: 다이얼로그 닫기
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Assessment against linked issues

Objective Addressed Explanation
DelayLoadingIndicator 적용 [#208] 로딩 표시 지연(Delay) 기능이 show(delay:)로 도입되어 요구사항을 충족합니다.

Pre-merge checks (3 passed, 2 warnings)

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning 이 PR에는 NoteCompletionViewController.swift에서 오류 알림을 UIAlertController에서 BKDialog로 교체하는 관련 없는 수정이 포함되어 있어 연결된 이슈 #208에 정의된 DelayLoadingIndicator 목표와 일치하지 않습니다. NoteCompletionViewController.swift의 오류 대화 상자 변경을 별도의 PR로 분리하거나, 병합 전에 명시된 목표와 일치하도록 조정하세요.
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed 제목이 변경사항의 주요 기능 구현과 일치하며 로딩 인디케이터에 딜레이를 추가하는 핵심 변경을 간결하고 정확하게 설명하고 있습니다.
Linked Issues Check ✅ Passed PR은 show() API에 딜레이 매개변수와 예약 메커니즘을 도입하여 지연 로딩 인디케이터를 구현함으로써 연결된 이슈 #208의 핵심 요구사항인 DelayLoadingIndicator 적용을 충족합니다.
Description Check ✅ Passed 설명은 연결된 이슈, 기능 유형, 로딩 화면 타이밍을 0ms에서 500ms로 조정한 구체적인 변경 사항과 테스트 검증 단계를 명확하게 제시하여 변경 사항과 직접적으로 일치합니다.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
		  - name: "Undocumented Breaking Changes"
			  mode: "warning"
			  instructions: |
				  Flag potential breaking changes that are not documented:
				  1. Identify changes to public APIs/exports, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints (including removed/renamed items and changes to types, required params, return values, defaults, or behavior).
				  2. Ignore purely internal/private changes (e.g., code not exported from package entry points or marked internal).
				  3. Verify documentation exists: a "Breaking Change" section in the PR description and updates to CHANGELOG.md.

Please share your feedback with us on this Discord post.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch BOOK-307-fix/#208

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (2)
src/Projects/BKDesign/Sources/Components/BKLoading/BKLoadingIndicator.swift (2)

3-4: 불필요한 import(SnapKit) 제거 제안

이 파일에서 SnapKit을 사용하지 않습니다. 불필요한 의존성 노출을 줄이기 위해 import를 제거하는 편이 깔끔합니다.

-import SnapKit
 import UIKit

29-30: 지연 값 방어 코드(클램프) 추가 제안

음수 지연이 들어오면 즉시 실행되지만, 의도 명확화를 위해 0으로 클램프하는 편이 안전합니다.

-            DispatchQueue.main.asyncAfter(deadline: .now() + delay, execute: workItem)
+            DispatchQueue.main.asyncAfter(deadline: .now() + max(0, delay), execute: workItem)

추가로, 새 show(delay:)가 더 짧은 지연으로 호출되면 기존 예약을 취소하고 재예약하는 정책(옵션)을 고려해 보세요.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between eab1f20 and 2356b06.

📒 Files selected for processing (1)
  • src/Projects/BKDesign/Sources/Components/BKLoading/BKLoadingIndicator.swift (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build
🔇 Additional comments (3)
src/Projects/BKDesign/Sources/Components/BKLoading/BKLoadingIndicator.swift (3)

9-9: 지연 작업 핸들 보관 방식 OK

메인 큐에서만 읽고/쓰도록 보장되어 데이터 레이스 우려가 낮습니다. 네이밍과 접근 제어도 적절합니다.


16-16: 중복 예약 방지 로직 적절

지연 표시가 이미 예약된 경우 재예약을 막아 플리커/중복 표시를 예방합니다.


12-14: LoadingIndicator.show에 delay 기본값 추가로 인한 영향 없음 확인됨

  • 리포지토리 내 LoadingIndicator.show() 직접 호출은 showLoading() 내부 사용(파일: BKLoadingIndicator.swift)뿐이며, 외부 모듈에서는 showLoading() API만 사용 중이므로 바이너리 의존 모듈 재컴파일 불필요
  • 기본값 0.5초는 PR 의도에 부합
  • 선택: UIViewController 확장에서 delay 커스터마이징 가능한 오버로드 추가 고려

Copy link
Member

@doyeonk429 doyeonk429 left a comment

Choose a reason for hiding this comment

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

성과공유회 보고 iOS에도 도입하면 좋겠다고 생각했는데, 좋네요. combine에서 처리하면 더 간단한 로직으로 처리 가능할 것 같은데, 인디케이터 객체에 또 다는게 좋아보이기도 하네요.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/Projects/BKPresentation/Sources/MainFlow/NoteCompletion/View/NoteCompletionViewController.swift (1)

63-74: 로딩 딜레이 미적용—showLoading()이 즉시 표시 호출
BKLoadingIndicator.showLoading() 내부에서 LoadingIndicator.show()만 호출해 0.5초 기본 지연이 적용되지 않습니다. 지연 표시(BKLoadingIndicator.show(delay:))가 사용되도록 수정 필요합니다.

🧹 Nitpick comments (2)
src/Projects/BKPresentation/Sources/MainFlow/NoteCompletion/View/NoteCompletionViewController.swift (2)

79-81: 연속 에러 이벤트 시 중복 present 방지 가드 제안

빠르게 연속 에러가 발행되면 이미 모달이 떠 있는 상태에서 또 present 시도할 수 있습니다. 간단한 가드로 방지하는 편이 안전합니다.

다음과 같이 변경 제안:

-            .sink { [weak self] _ in
-                self?.presentErrorDialog()
-            }
+            .sink { [weak self] _ in
+                guard let self = self, self.presentedViewController == nil else { return }
+                self.presentErrorDialog()
+            }

167-181: BKDialog 카피/버튼 라벨 UX 보정

현재 버튼 라벨이 본문 메시지 역할을 하고 있어 UX가 어색합니다. 타이틀은 메시지, 버튼은 “확인”으로 정리하는 편이 자연스럽습니다. 중첩 [weak self]도 한 번으로 단순화 가능합니다.

-        let dialog = BKDialog(
-            title: "오류",
+        let dialog = BKDialog(
+            title: "기록을 불러올 수 없습니다.",
             config: .init(
-                leftButtonTitle: "기록을 불러올 수 없습니다.",
-                leftButtonAction: { [weak self] in
-                    self?.dismiss(animated: true) { [weak self] in
-                        self?.viewModel.send(.errorHandled)
-                    }
-                }
+                leftButtonTitle: "확인",
+                leftButtonAction: { [weak self] in
+                    guard let self else { return }
+                    self.dismiss(animated: true) {
+                        self.viewModel.send(.errorHandled)
+                    }
+                }
             )
         )

참고: 과거 러닝에 따라 에러 상세 카피는 추후로 미루셔도 OK입니다.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2356b06 and 1c3eba8.

📒 Files selected for processing (2)
  • src/Projects/BKDesign/Sources/Components/BKLoading/BKLoadingIndicator.swift (1 hunks)
  • src/Projects/BKPresentation/Sources/MainFlow/NoteCompletion/View/NoteCompletionViewController.swift (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/Projects/BKDesign/Sources/Components/BKLoading/BKLoadingIndicator.swift
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-01T20:35:39.806Z
Learnt from: clxxrlove
PR: YAPP-Github/Reed-iOS#134
File: src/Projects/BKPresentation/Sources/MainFlow/Note/ViewModel/NoteViewModel.swift:66-74
Timestamp: 2025-08-01T20:35:39.806Z
Learning: clxxrlove는 NoteViewModel에서 UI 에러 처리를 추후 작업으로 계획하며, 현재 단계에서는 기본 기능 구현에 집중하는 것을 선호한다고 명시했다.

Applied to files:

  • src/Projects/BKPresentation/Sources/MainFlow/NoteCompletion/View/NoteCompletionViewController.swift
📚 Learning: 2025-07-30T11:32:20.533Z
Learnt from: clxxrlove
PR: YAPP-Github/Reed-iOS#120
File: src/Projects/BKPresentation/Sources/AppCoordinator.swift:31-41
Timestamp: 2025-07-30T11:32:20.533Z
Learning: Reed-iOS 프로젝트의 OnboardingCheckUseCase.execute() 메서드는 AnyPublisher<Bool, Never>를 반환하므로 실패할 수 없도록 설계되었다. Never 타입은 에러가 발생하지 않음을 보장하므로 에러 처리 로직이 불필요하다.

Applied to files:

  • src/Projects/BKPresentation/Sources/MainFlow/NoteCompletion/View/NoteCompletionViewController.swift
🧬 Code graph analysis (1)
src/Projects/BKPresentation/Sources/MainFlow/NoteCompletion/View/NoteCompletionViewController.swift (1)
src/Projects/BKPresentation/Sources/MainFlow/NoteCompletion/ViewModel/NoteCompletionViewModel.swift (1)
  • send (51-55)

@clxxrlove clxxrlove merged commit 4585672 into develop Sep 15, 2025
3 checks passed
@clxxrlove clxxrlove deleted the BOOK-307-fix/#208 branch September 15, 2025 03:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BOOK-307/fix] DelayLoadingIndicator 적용

3 participants