Conversation
* fix - QA 반영 (#34) * fix(diary): videoUrl 필드 유튜브 id로 변경 * fix: 백그라운드 상태 복귀 시 유튜브 영상 재생 이슈해결 * feat - 킬링파트 재생 페이지 작업 (#37) * feat(PlayKillingPartView): 킬링파트 재생 페이지 UI 업데이트 * feat(PlayKillingPartView): UI 디테일 수정 * feat(PlayKillingPart): 플레이리스트 뷰 UI 업데이트 * feat(PlayKillingpart): 바텀 플레이어 패널 크기 및 UI 크기 상승 * fix(PlayKillingpart): 바텀플레이어 패널, 하단 네비, body 여백 대응 * feat(PlayKillingPart): 현재 재생 컨테이너 UI 업데이트 * feat(PlayKillingPart): 음악 다이어리 정렬 순서 수정 API 연동 및 드래그 핸들 UI 적용 * fix(PlayKillingPart): 잔상 제거 * fix(PlayKillingPart): 드래그 상태 반투명 적용 * fix(PlayKillingPart): 드래그앤 드롭 UX 개선 * feat(1.0.16): 버전 업데이트
There was a problem hiding this comment.
Pull request overview
Adds a new “킬링파트 재생” experience in My that can play through the user’s stored killing-part diaries with an expandable playlist UI and drag-to-reorder support, including backend order persistence.
Changes:
- Implemented the PlayKillingPart screen UI (current track card + bottom mini player + expandable playlist + edit/reorder UX).
- Extended
YoutubePlayerViewto support play/pause synchronization from SwiftUI state. - Added a diary order update API (
PATCH /diaries/order) and a dedicatedPlayKillingPartViewModelto drive edit/save state.
Reviewed changes
Copilot reviewed 7 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
KillingPart/Views/Screens/Main/My/PlayKillingPart/PlayKillingPartView.swift |
New playback screen, timer-driven “range playback”, playlist expansion, drag/drop reordering UI, and feed-loading for playback. |
KillingPart/Views/Screens/Main/Add/AddSearchDetail/components/YoutubePlayerView.swift |
Adds isPlaying param and JS logic to pause/play and (re)seek within the desired range. |
KillingPart/ViewModels/My/PlayKillingPart/PlayKillingPartViewModel.swift |
New view model for playlist edit mode and saving reordered diary IDs. |
KillingPart/Services/DiaryService.swift |
Adds updateMyDiaryOrder service method to persist the new order. |
KillingPart/Models/DiaryModel.swift |
Adds DiaryOrderUpdateRequest. |
KillingPart/Assets.xcassets/killingpart_music_icon.imageset/* |
Adds an icon asset used for the currently-playing indicator. |
KillingPart.xcodeproj/project.pbxproj |
Bumps build/marketing version. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| deinit { | ||
| onDragEnded?() |
There was a problem hiding this comment.
PlayKillingPartDragItemProvider relies on deinit to detect that a drag ended and clear state. NSItemProvider lifetime is managed by the drag system and deallocation timing isn’t guaranteed (and can be delayed), which can leave draggedTrackID set and the UI stuck in a “dragging” state. Prefer clearing drag state via explicit drag/drop callbacks (e.g., always reset in performDrop and a cancellation path such as onDisappear/a dedicated interaction).
| deinit { | |
| onDragEnded?() | |
| /// Explicitly notify that the drag operation associated with this provider has ended. | |
| /// Call this from drag/drop completion or cancellation handlers instead of relying on `deinit`. | |
| func notifyDragEndedIfNeeded() { | |
| onDragEnded?() | |
| onDragEnded = nil |
| } | ||
| """ | ||
| : """ | ||
| if (\(shouldForceSeekJS)) { | ||
| window.kpPlayer.seekTo(window.kpDesiredStart, true); | ||
| } | ||
| window.kpPlayer.pauseVideo(); | ||
| """ | ||
|
|
||
| webView.evaluateJavaScript( | ||
| """ |
There was a problem hiding this comment.
Pausing only calls window.kpPlayer.pauseVideo(), but the interval created by kpStartRangeLoop() keeps running until beforeunload. Even if it returns early while paused, it still fires every 200ms and wastes CPU in the web view. Consider adding a JS kpStopRangeLoop() that clears kpLoopTimer and call it when isPlaying becomes false, then restart the loop when resuming.
| } | |
| """ | |
| : """ | |
| if (\(shouldForceSeekJS)) { | |
| window.kpPlayer.seekTo(window.kpDesiredStart, true); | |
| } | |
| window.kpPlayer.pauseVideo(); | |
| """ | |
| webView.evaluateJavaScript( | |
| """ | |
| } | |
| if (window.kpStartRangeLoop) { | |
| window.kpStartRangeLoop(); | |
| } | |
| """ | |
| : """ | |
| if (\(shouldForceSeekJS)) { | |
| window.kpPlayer.seekTo(window.kpDesiredStart, true); | |
| } | |
| window.kpPlayer.pauseVideo(); | |
| if (window.kpStopRangeLoop) { | |
| window.kpStopRangeLoop(); | |
| } | |
| """ | |
| webView.evaluateJavaScript( | |
| """ | |
| if (!window.kpStopRangeLoop) { | |
| window.kpStopRangeLoop = function () { | |
| if (window.kpLoopTimer) { | |
| clearInterval(window.kpLoopTimer); | |
| window.kpLoopTimer = null; | |
| } | |
| }; | |
| } |
| private let playbackTimer = Timer.publish(every: 0.25, on: .main, in: .common).autoconnect() | ||
| private let videoAspectRatio: CGFloat = 16 / 9 |
There was a problem hiding this comment.
playbackTimer is autoconnected and will keep firing at 4Hz for the lifetime of this view, even when there is no currentTrack or isPlaying is false. That still wakes the main runloop and calls handlePlaybackTick. Consider starting/stopping the timer based on playback state (or using a .task loop that only runs while playing) to reduce background CPU/battery use.
| private func loadAllDiaryFeedsForPlayback() async { | ||
| await viewModel.refetchCollectionDataOnFocus() | ||
|
|
||
| var previousFeedCount = -1 | ||
| var iteration = 0 |
There was a problem hiding this comment.
loadAllDiaryFeedsForPlayback() awaits refetchCollectionDataOnFocus(), but that method can early-return when a load is already in progress (it sets a pending flag). In that case this function can immediately enter/exit the loop (because loadMoreMyFeedsFromBottomIfNeeded() may also early-return while loading), leaving the playlist partially loaded. Consider serializing these refresh/pagination tasks (single in-flight Task / guard) or moving this “load all pages” behavior into the view model where it can await ongoing loads and loop until no next page remains.
| private func parsedSeconds(from value: String) -> Double? { | ||
| let trimmed = value.trimmingCharacters(in: .whitespacesAndNewlines) | ||
| guard !trimmed.isEmpty else { return nil } | ||
|
|
||
| if let raw = Double(trimmed) { |
There was a problem hiding this comment.
parsedSeconds(from:) duplicates the same parsing logic already present in MyCollectionDiaryViewModel (including handling for "초" / "mm:ss"). Duplicating this makes future bug fixes easy to miss; consider extracting a shared helper (e.g., on TimeFormatter or a dedicated utility) and reuse it here.
| let trimmed = rawVideoURL.trimmingCharacters(in: .whitespacesAndNewlines) | ||
| guard !trimmed.isEmpty else { return nil } | ||
|
|
||
| let normalizedURLText: String | ||
| if isLikelyYouTubeVideoID(trimmed) { | ||
| normalizedURLText = "https://www.youtube.com/embed/\(trimmed)?playsinline=1" | ||
| } else { | ||
| normalizedURLText = trimmed | ||
| } | ||
|
|
||
| if let parsed = URL(string: normalizedURLText), parsed.scheme != nil { | ||
| return parsed | ||
| } | ||
|
|
||
| if normalizedURLText.hasPrefix("//") { | ||
| return URL(string: "https:\(normalizedURLText)") | ||
| } | ||
|
|
||
| return URL(string: "https://\(normalizedURLText)") | ||
| } | ||
|
|
||
| private func isLikelyYouTubeVideoID(_ value: String) -> Bool { | ||
| if value.hasPrefix("//") { | ||
| return false | ||
| } | ||
|
|
||
| if let components = URLComponents(string: value), | ||
| components.scheme != nil || components.host != nil { | ||
| return false | ||
| } | ||
|
|
||
| return !value.contains("/") | ||
| && !value.contains("?") | ||
| && !value.contains("&") | ||
| && !value.contains("=") | ||
| && !value.contains(".") | ||
| } |
There was a problem hiding this comment.
resolvedVideoURL(from:) / isLikelyYouTubeVideoID(_:) duplicate the YouTube ID normalization logic that already exists in MyCollectionViewModel (resolvedVideoURLForPlayback / isLikelyYouTubeVideoID). Keeping multiple copies risks subtle divergence (embed params, edge cases). Consider reusing the existing helper or extracting it into a shared utility.
| let trimmed = rawVideoURL.trimmingCharacters(in: .whitespacesAndNewlines) | |
| guard !trimmed.isEmpty else { return nil } | |
| let normalizedURLText: String | |
| if isLikelyYouTubeVideoID(trimmed) { | |
| normalizedURLText = "https://www.youtube.com/embed/\(trimmed)?playsinline=1" | |
| } else { | |
| normalizedURLText = trimmed | |
| } | |
| if let parsed = URL(string: normalizedURLText), parsed.scheme != nil { | |
| return parsed | |
| } | |
| if normalizedURLText.hasPrefix("//") { | |
| return URL(string: "https:\(normalizedURLText)") | |
| } | |
| return URL(string: "https://\(normalizedURLText)") | |
| } | |
| private func isLikelyYouTubeVideoID(_ value: String) -> Bool { | |
| if value.hasPrefix("//") { | |
| return false | |
| } | |
| if let components = URLComponents(string: value), | |
| components.scheme != nil || components.host != nil { | |
| return false | |
| } | |
| return !value.contains("/") | |
| && !value.contains("?") | |
| && !value.contains("&") | |
| && !value.contains("=") | |
| && !value.contains(".") | |
| } | |
| // Delegate URL normalization and YouTube ID handling to MyCollectionViewModel | |
| return viewModel.resolvedVideoURLForPlayback(from: rawVideoURL) | |
| } |
작업내용