diff --git a/README.md b/README.md index c9bb9748..f454a4c7 100644 --- a/README.md +++ b/README.md @@ -26,7 +26,7 @@ A native macOS client for YouTube Music and YouTube, built with Swift and SwiftU - ⌨️ **[Keyboard Shortcuts](docs/keyboard-shortcuts.md)** — Full keyboard control for playback, navigation, and more - 🧭 **Explore** — Discover new releases, charts, and moods & genres - 🎙️ **Podcasts** — Browse and listen to podcasts with episode progress tracking -- 📚 **Library Access** — Browse playlists, liked songs, and subscribed podcasts; create playlists, add songs to playlists, and delete your own playlists +- 📚 **Library Access** — Browse playlists, liked songs, and subscribed podcasts; create playlists, add songs to playlists, sort playlist tracks by title, artist, or duration, and delete your own playlists - 🕓 **History** — Revisit recently played tracks - 🔍 **Search** — Find songs, albums, artists, playlists, and podcasts - 🌍 **Localized** — Available in English, French, Korean, Indonesian, Turkish, and Arabic diff --git a/Sources/Kaset/ViewModels/PlaylistDetailViewModel.swift b/Sources/Kaset/ViewModels/PlaylistDetailViewModel.swift index feb676f9..2c7dfcb3 100644 --- a/Sources/Kaset/ViewModels/PlaylistDetailViewModel.swift +++ b/Sources/Kaset/ViewModels/PlaylistDetailViewModel.swift @@ -2,6 +2,18 @@ import Foundation import Observation import os +// MARK: - PlaylistTrackSortOption + +/// Session-only sort options for the playlist detail track list. +enum PlaylistTrackSortOption: CaseIterable { + case custom + case title + case artist + case duration +} + +// MARK: - PlaylistDetailViewModel + /// View model for the PlaylistDetailView. @MainActor @Observable @@ -28,6 +40,49 @@ final class PlaylistDetailViewModel { /// Whether more tracks are available to load. private(set) var hasMore: Bool = false + /// Current track sort option (session-only, not persisted). + private(set) var sortOption: PlaylistTrackSortOption = .custom + + /// Current sort direction; ignored when `sortOption` is `.custom`. + private(set) var sortAscending: Bool = true + + /// A playlist track paired with a stable identity that survives re-sorting, so the list moves + /// rows in place instead of rebuilding them — rebuilding reloads row artwork and like state. + struct IdentifiedTrack: Identifiable { + let id: String + let song: Song + } + + /// Tracks in the selected sort order, each carrying a stable identity (song id + occurrence, + /// numbered in playlist order so duplicate tracks stay distinct). Reordering these only changes + /// their positions, so SwiftUI animates moves rather than swapping row contents. + var displayedTrackRows: [IdentifiedTrack] { + guard let tracks = self.playlistDetail?.tracks else { return [] } + + var occurrences: [String: Int] = [:] + let identified = tracks.map { song -> IdentifiedTrack in + let occurrence = occurrences[song.id, default: 0] + occurrences[song.id] = occurrence + 1 + return IdentifiedTrack(id: "\(song.id)#\(occurrence)", song: song) + } + + switch self.sortOption { + case .custom: + return identified + case .title: + return self.sortedByStandardCompare(identified) { $0.song.title } + case .artist: + return self.sortedByStandardCompare(identified, tieBreaker: { $0.song.title }, key: { $0.song.artistsDisplay }) + case .duration: + return self.sortedByDuration(identified) + } + } + + /// The playlist tracks in the currently selected sort order. + var displayedTracks: [Song] { + self.displayedTrackRows.map(\.song) + } + private let playlist: Playlist /// The API client (exposed for add to library action). let client: any YTMusicClientProtocol @@ -480,6 +535,45 @@ final class PlaylistDetailViewModel { await self.load(restartingInFlightLoad: true) } + /// Selects a track sort option. Selecting the already-active option toggles the sort + /// direction; selecting a different option resets to ascending. + func selectSort(_ option: PlaylistTrackSortOption) { + if self.sortOption == option { + self.sortAscending.toggle() + } else { + self.sortOption = option + self.sortAscending = true + } + } + + private func sortedByStandardCompare( + _ tracks: [Element], + tieBreaker: ((Element) -> String)? = nil, + key: (Element) -> String + ) -> [Element] { + tracks.sorted { lhs, rhs in + var result = key(lhs).localizedStandardCompare(key(rhs)) + if result == .orderedSame, let tieBreaker { + result = tieBreaker(lhs).localizedStandardCompare(tieBreaker(rhs)) + } + guard result != .orderedSame else { return false } + return self.sortAscending ? result == .orderedAscending : result == .orderedDescending + } + } + + private func sortedByDuration(_ tracks: [IdentifiedTrack]) -> [IdentifiedTrack] { + tracks.sorted { lhs, rhs in + switch (lhs.song.duration, rhs.song.duration) { + case let (lhsDuration?, rhsDuration?): + self.sortAscending ? lhsDuration < rhsDuration : lhsDuration > rhsDuration + case (nil, nil), (nil, _): + false + case (_, nil): + true + } + } + } + private func cancelFullLoadTask() { self.fullLoadTask?.cancel() self.fullLoadTask = nil diff --git a/Sources/Kaset/Views/PlaylistDetailView+HeaderActions.swift b/Sources/Kaset/Views/PlaylistDetailView+HeaderActions.swift index 6da0d29b..8b2b1d03 100644 --- a/Sources/Kaset/Views/PlaylistDetailView+HeaderActions.swift +++ b/Sources/Kaset/Views/PlaylistDetailView+HeaderActions.swift @@ -16,7 +16,7 @@ extension PlaylistDetailView { func headerButtons(_ detail: PlaylistDetail) -> some View { let fallbackAlbum = self.makeFallbackAlbum(from: detail) let playableTracks = self.playableTracks( - detail.tracks, + self.viewModel.displayedTracks, fallbackArtist: detail.author?.name, fallbackAlbum: fallbackAlbum ) @@ -68,7 +68,7 @@ extension PlaylistDetailView { ) -> some View { Button { self.playAll( - detail.tracks, fallbackArtist: detail.author?.name, + self.viewModel.displayedTracks, fallbackArtist: detail.author?.name, fallbackAlbum: fallbackAlbum ) } label: { diff --git a/Sources/Kaset/Views/PlaylistDetailView+TrackSort.swift b/Sources/Kaset/Views/PlaylistDetailView+TrackSort.swift new file mode 100644 index 00000000..55899c82 --- /dev/null +++ b/Sources/Kaset/Views/PlaylistDetailView+TrackSort.swift @@ -0,0 +1,57 @@ +import SwiftUI + +@available(macOS 26.0, *) +extension PlaylistDetailView { + @ViewBuilder + func trackSortControl(_ detail: PlaylistDetail) -> some View { + if detail.tracks.count >= 2 { + Menu { + ForEach(PlaylistTrackSortOption.allCases, id: \.self) { option in + Button { + self.viewModel.selectSort(option) + } label: { + HStack { + Text(self.sortOptionTitle(option)) + if self.viewModel.sortOption == option { + Spacer() + Image(systemName: "checkmark") + if option != .custom { + Image(systemName: self.viewModel.sortAscending ? "chevron.up" : "chevron.down") + } + } + } + } + } + } label: { + self.trackSortMenuLabel + } + .menuStyle(.borderlessButton) + .fixedSize() + .foregroundStyle(.secondary) + .help(String(localized: "Sort tracks")) + .accessibilityLabel(String(localized: "Sort tracks")) + } + } + + private var trackSortMenuLabel: some View { + HStack(spacing: 4) { + Image(systemName: "arrow.up.arrow.down") + if self.viewModel.sortOption != .custom { + Text(self.sortOptionTitle(self.viewModel.sortOption)) + } + } + } + + private func sortOptionTitle(_ option: PlaylistTrackSortOption) -> String { + switch option { + case .custom: + String(localized: "Default") + case .title: + String(localized: "Title") + case .artist: + String(localized: "Artist") + case .duration: + String(localized: "Duration") + } + } +} diff --git a/Sources/Kaset/Views/PlaylistDetailView.swift b/Sources/Kaset/Views/PlaylistDetailView.swift index a28ba179..20fb33eb 100644 --- a/Sources/Kaset/Views/PlaylistDetailView.swift +++ b/Sources/Kaset/Views/PlaylistDetailView.swift @@ -136,7 +136,7 @@ struct PlaylistDetailView: View { trackCount: detail.trackCount ?? detail.tracks.count ) self.tracksView( - detail.tracks, isAlbum: detail.isAlbum, author: detail.author?.name, + self.viewModel.displayedTrackRows, isAlbum: detail.isAlbum, author: detail.author?.name, fallbackAlbum: fallbackAlbum ) } @@ -189,7 +189,11 @@ struct PlaylistDetailView: View { Spacer(minLength: 24) - self.headerButtons(detail) + HStack(spacing: 16) { + self.headerButtons(detail) + Spacer(minLength: 16) + self.trackSortControl(detail) + } } .frame(maxWidth: .infinity, alignment: .leading) } @@ -240,22 +244,23 @@ struct PlaylistDetailView: View { } private func tracksView( - _ tracks: [Song], isAlbum: Bool, author: String?, fallbackAlbum: Album? = nil + _ rows: [PlaylistDetailViewModel.IdentifiedTrack], isAlbum: Bool, author: String?, fallbackAlbum: Album? = nil ) -> some View { - LazyVStack(spacing: 0) { - ForEach(Array(tracks.enumerated()), id: \.offset) { index, track in + let tracks = rows.map(\.song) + return LazyVStack(spacing: 0) { + ForEach(Array(rows.enumerated()), id: \.element.id) { index, row in self.trackRow( - track, index: index, tracks: tracks, isAlbum: isAlbum, author: author, + row.song, index: index, tracks: tracks, isAlbum: isAlbum, author: author, fallbackAlbum: fallbackAlbum ) .onAppear { // Load more when reaching the last few items - if index >= tracks.count - 3, self.viewModel.hasMore { + if index >= rows.count - 3, self.viewModel.hasMore { Task { await self.viewModel.loadMore() } } } - if index < tracks.count - 1 { + if index < rows.count - 1 { Divider() // For albums: 28 (index) + 12 (spacing) // For playlists: 28 (index) + 12 (spacing) + 40 (thumbnail) + 16 (spacing) @@ -490,7 +495,7 @@ struct PlaylistDetailView: View { initial cleanedTracks: [Song], startingAt index: Int, fallbackArtist: String?, fallbackAlbum: Album? ) { - let initiallyLoadedCount = cleanedTracks.count + let alreadyQueuedVideoIds = Set(cleanedTracks.map(\.videoId)) Task { @MainActor in let willDeferLoad = self.viewModel.hasMore let loadGeneration = await self.playerService.playQueue( @@ -505,11 +510,13 @@ struct PlaylistDetailView: View { // such as removing a track keep the same load generation, so loading continues.) guard self.playerService.isCurrentQueueLoad(loadGeneration) else { return } + // Diffed by videoId (not index) so a re-sort while pagination was still in flight + // can't duplicate or drop tracks the fixed-count prefix would have assumed unchanged. let fullTracks = self.playableTracks( - self.viewModel.playlistDetail?.tracks ?? [], + self.viewModel.displayedTracks, fallbackArtist: fallbackArtist, fallbackAlbum: fallbackAlbum ) - let remaining = Array(fullTracks.dropFirst(initiallyLoadedCount)) + let remaining = fullTracks.filter { !alreadyQueuedVideoIds.contains($0.videoId) } self.playerService.appendOriginalTracks(remaining) await self.playerService.endQueueLoading(loadGeneration) } diff --git a/Tests/KasetTests/NotificationServiceTests.swift b/Tests/KasetTests/NotificationServiceTests.swift index 71b8558c..71db7d57 100644 --- a/Tests/KasetTests/NotificationServiceTests.swift +++ b/Tests/KasetTests/NotificationServiceTests.swift @@ -23,6 +23,20 @@ struct NotificationServiceTests { try? await Task.sleep(for: .milliseconds(50)) } + /// Polls until `condition` holds, bounded by a deadline that tolerates slow CI + /// runners — a fixed wait races with Observation delivery and the service's + /// loading-resolution poll loop. + private func waitForObservationDelivery(until condition: @autoclosure @MainActor () -> Bool) async { + let clock = ContinuousClock() + let deadline = clock.now.advanced(by: .seconds(2)) + while !condition(), clock.now < deadline { + for _ in 0 ..< 5 { + await Task.yield() + } + try? await Task.sleep(for: .milliseconds(10)) + } + } + // MARK: - Observation Lifecycle @Test("observation is active after init") @@ -43,7 +57,7 @@ struct NotificationServiceTests { self.playerService.currentTrack = TestFixtures.makeSong(id: "song-1", title: "First Song") self.playerService.state = .playing - await self.waitForObservationDelivery() + await self.waitForObservationDelivery(until: self.notificationService.lastNotifiedTrackId == "song-1") #expect(self.notificationService.lastNotifiedTrackId == "song-1") } @@ -55,7 +69,7 @@ struct NotificationServiceTests { playerService.state = .playing let notificationService = NotificationService(playerService: playerService) - await self.waitForObservationDelivery() + await self.waitForObservationDelivery(until: notificationService.lastNotifiedTrackId == "initial-song") #expect(notificationService.lastNotifiedTrackId == "initial-song") notificationService.stopObserving() @@ -65,11 +79,11 @@ struct NotificationServiceTests { func detectsMultipleTrackChanges() async { self.playerService.currentTrack = TestFixtures.makeSong(id: "song-1", title: "First Song") self.playerService.state = .playing - await self.waitForObservationDelivery() + await self.waitForObservationDelivery(until: self.notificationService.lastNotifiedTrackId == "song-1") #expect(self.notificationService.lastNotifiedTrackId == "song-1") self.playerService.currentTrack = TestFixtures.makeSong(id: "song-2", title: "Second Song") - await self.waitForObservationDelivery() + await self.waitForObservationDelivery(until: self.notificationService.lastNotifiedTrackId == "song-2") #expect(self.notificationService.lastNotifiedTrackId == "song-2") } @@ -93,7 +107,7 @@ struct NotificationServiceTests { self.playerService.state = .playing - await self.waitForObservationDelivery() + await self.waitForObservationDelivery(until: self.notificationService.lastNotifiedTrackId == "song-1") #expect(self.notificationService.lastNotifiedTrackId == "song-1") } @@ -101,12 +115,12 @@ struct NotificationServiceTests { func doesNotNotifyForSameTrackTwice() async { self.playerService.currentTrack = TestFixtures.makeSong(id: "song-1", title: "First Song") self.playerService.state = .playing - await self.waitForObservationDelivery() + await self.waitForObservationDelivery(until: self.notificationService.lastNotifiedTrackId == "song-1") #expect(self.notificationService.lastNotifiedTrackId == "song-1") // Set a different track, then back to the same one self.playerService.currentTrack = TestFixtures.makeSong(id: "song-2", title: "Second Song") - await self.waitForObservationDelivery() + await self.waitForObservationDelivery(until: self.notificationService.lastNotifiedTrackId == "song-2") // The lastNotifiedTrackId should now be song-2, meaning song-1 wasn't skipped #expect(self.notificationService.lastNotifiedTrackId == "song-2") @@ -133,7 +147,7 @@ struct NotificationServiceTests { self.playerService.currentTrack = TestFixtures.makeSong(id: "song-1", title: "Real Song") self.playerService.state = .playing - await self.waitForObservationDelivery() + await self.waitForObservationDelivery(until: self.notificationService.lastNotifiedTrackId == "song-1") #expect(self.notificationService.lastNotifiedTrackId == "song-1") } @@ -146,7 +160,7 @@ struct NotificationServiceTests { self.playerService.currentTrack = TestFixtures.makeSong(id: "song-1", title: "Real Song") - await self.waitForObservationDelivery() + await self.waitForObservationDelivery(until: self.notificationService.lastNotifiedTrackId == "song-1") #expect(self.notificationService.lastNotifiedTrackId == "song-1") } @@ -162,7 +176,7 @@ struct NotificationServiceTests { func stopObservingPreventsFutureNotifications() async { self.playerService.currentTrack = TestFixtures.makeSong(id: "song-1", title: "First Song") self.playerService.state = .playing - await self.waitForObservationDelivery() + await self.waitForObservationDelivery(until: self.notificationService.lastNotifiedTrackId == "song-1") #expect(self.notificationService.lastNotifiedTrackId == "song-1") self.notificationService.stopObserving() @@ -181,7 +195,7 @@ struct NotificationServiceTests { // And still detects changes without a polling delay. self.playerService.currentTrack = TestFixtures.makeSong(id: "late-song", title: "Late Song") self.playerService.state = .playing - await self.waitForObservationDelivery() + await self.waitForObservationDelivery(until: self.notificationService.lastNotifiedTrackId == "late-song") #expect(self.notificationService.lastNotifiedTrackId == "late-song") } } diff --git a/Tests/KasetTests/PlaylistDetailViewModelTests.swift b/Tests/KasetTests/PlaylistDetailViewModelTests.swift index 03479f21..1b9a847e 100644 --- a/Tests/KasetTests/PlaylistDetailViewModelTests.swift +++ b/Tests/KasetTests/PlaylistDetailViewModelTests.swift @@ -1140,4 +1140,118 @@ struct PlaylistDetailViewModelTests { #expect(viewModel.playlistDetail?.author?.subtitle == "123 subscribers") #expect(viewModel.playlistDetail?.author?.profileKind == .profile) } + + // MARK: - Sort Tests + + private func makeSortTestViewModel() async -> PlaylistDetailViewModel { + let tracks = [ + TestFixtures.makeSong(id: "1", title: "Banana", artistName: "Bob", duration: 200), + TestFixtures.makeSong(id: "2", title: "Apple", artistName: "Amy", duration: 100), + TestFixtures.makeSong(id: "3", title: "Cherry", artistName: "Amy", duration: nil), + ] + let playlist = TestFixtures.makePlaylist(id: "VL-sort-test") + let mockClient = MockYTMusicClient() + mockClient.playlistDetails[playlist.id] = PlaylistDetail( + playlist: playlist, + tracks: tracks, + duration: nil + ) + let viewModel = PlaylistDetailViewModel(playlist: playlist, client: mockClient) + await viewModel.load() + return viewModel + } + + @Test("Custom sort keeps original playlist order") + func customSortKeepsOriginalOrder() async { + let viewModel = await self.makeSortTestViewModel() + + #expect(viewModel.sortOption == .custom) + #expect(viewModel.displayedTracks.map(\.videoId) == ["1", "2", "3"]) + } + + @Test("Title sort orders ascending then toggles descending") + func titleSortTogglesDirection() async { + let viewModel = await self.makeSortTestViewModel() + + viewModel.selectSort(.title) + #expect(viewModel.sortOption == .title) + #expect(viewModel.sortAscending == true) + #expect(viewModel.displayedTracks.map(\.videoId) == ["2", "1", "3"]) + + viewModel.selectSort(.title) + #expect(viewModel.sortAscending == false) + #expect(viewModel.displayedTracks.map(\.videoId) == ["3", "1", "2"]) + } + + @Test("Artist sort breaks ties by title") + func artistSortBreaksTiesByTitle() async { + let viewModel = await self.makeSortTestViewModel() + + viewModel.selectSort(.artist) + + // Amy (2 tracks) sorts before Bob; her tracks break the tie by title (Apple < Cherry). + #expect(viewModel.displayedTracks.map(\.videoId) == ["2", "3", "1"]) + } + + @Test("Duration sort keeps nil durations last regardless of direction") + func durationSortKeepsNilDurationsLast() async { + let viewModel = await self.makeSortTestViewModel() + + viewModel.selectSort(.duration) + #expect(viewModel.displayedTracks.map(\.videoId) == ["2", "1", "3"]) + + viewModel.selectSort(.duration) + #expect(viewModel.sortAscending == false) + #expect(viewModel.displayedTracks.map(\.videoId) == ["1", "2", "3"]) + } + + @Test("Selecting a different sort option resets to ascending") + func selectingNewSortOptionResetsAscending() async { + let viewModel = await self.makeSortTestViewModel() + + viewModel.selectSort(.title) + viewModel.selectSort(.title) + #expect(viewModel.sortAscending == false) + + viewModel.selectSort(.duration) + #expect(viewModel.sortOption == .duration) + #expect(viewModel.sortAscending == true) + } + + @Test("Row identities stay stable across sort changes so rows move instead of rebuilding") + func rowIdentitiesStableAcrossSort() async { + let viewModel = await self.makeSortTestViewModel() + + let identityByVideoId = Dictionary( + uniqueKeysWithValues: viewModel.displayedTrackRows.map { ($0.song.videoId, $0.id) } + ) + + viewModel.selectSort(.title) + + // Order changes, but each song keeps the same row identity so SwiftUI moves the row instead + // of swapping a slot's contents (which would reload artwork and like state). + #expect(viewModel.displayedTrackRows.map(\.song.videoId) == ["2", "1", "3"]) + for row in viewModel.displayedTrackRows { + #expect(row.id == identityByVideoId[row.song.videoId]) + } + } + + @Test("Duplicate tracks receive distinct stable row identities") + func duplicateTracksGetDistinctIdentities() async { + let dup = TestFixtures.makeSong(id: "dup", title: "Same", artistName: "X", duration: 100) + let other = TestFixtures.makeSong(id: "other", title: "Other", artistName: "Y", duration: 200) + let playlist = TestFixtures.makePlaylist(id: "VL-dup-test") + let mockClient = MockYTMusicClient() + mockClient.playlistDetails[playlist.id] = PlaylistDetail( + playlist: playlist, + tracks: [dup, other, dup], + duration: nil + ) + let viewModel = PlaylistDetailViewModel(playlist: playlist, client: mockClient) + await viewModel.load() + + let ids = viewModel.displayedTrackRows.map(\.id) + #expect(ids == ["dup#0", "other#0", "dup#1"]) + #expect(Set(ids).count == 3, "duplicate tracks must still get unique row identities") + } }