From 5a28837b9c9c008e6ab8416dbc375b2e02c59434 Mon Sep 17 00:00:00 2001 From: Daniel Andersson Date: Sat, 11 Jul 2026 02:04:28 +0200 Subject: [PATCH 1/3] feat: sort playlist tracks by title, artist, or duration --- README.md | 2 +- .../ViewModels/PlaylistDetailViewModel.swift | 73 ++++++++++++++++++ .../PlaylistDetailView+HeaderActions.swift | 4 +- .../Views/PlaylistDetailView+TrackSort.swift | 61 +++++++++++++++ Sources/Kaset/Views/PlaylistDetailView.swift | 12 ++- .../PlaylistDetailViewModelTests.swift | 77 +++++++++++++++++++ 6 files changed, 222 insertions(+), 7 deletions(-) create mode 100644 Sources/Kaset/Views/PlaylistDetailView+TrackSort.swift 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..4947173c 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,28 @@ 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 + + /// The playlist tracks in the currently selected sort order. + var displayedTracks: [Song] { + guard let tracks = self.playlistDetail?.tracks else { return [] } + + switch self.sortOption { + case .custom: + return tracks + case .title: + return self.sortedByStandardCompare(tracks) { $0.title } + case .artist: + return self.sortedByStandardCompare(tracks, tieBreaker: { $0.title }, key: { $0.artistsDisplay }) + case .duration: + return self.sortedByDuration(tracks) + } + } + private let playlist: Playlist /// The API client (exposed for add to library action). let client: any YTMusicClientProtocol @@ -480,6 +514,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: [Song], + tieBreaker: ((Song) -> String)? = nil, + key: (Song) -> String + ) -> [Song] { + 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: [Song]) -> [Song] { + tracks.sorted { lhs, rhs in + switch (lhs.duration, rhs.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..b4d7d4f0 --- /dev/null +++ b/Sources/Kaset/Views/PlaylistDetailView+TrackSort.swift @@ -0,0 +1,61 @@ +import SwiftUI + +@available(macOS 26.0, *) +extension PlaylistDetailView { + @ViewBuilder + func trackSortMenu(_ detail: PlaylistDetail) -> some View { + if detail.tracks.count >= 2 { + HStack { + Spacer() + + 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: "Custom order") + 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..de538323 100644 --- a/Sources/Kaset/Views/PlaylistDetailView.swift +++ b/Sources/Kaset/Views/PlaylistDetailView.swift @@ -135,8 +135,10 @@ struct PlaylistDetailView: View { year: nil, trackCount: detail.trackCount ?? detail.tracks.count ) + self.trackSortMenu(detail) + self.tracksView( - detail.tracks, isAlbum: detail.isAlbum, author: detail.author?.name, + self.viewModel.displayedTracks, isAlbum: detail.isAlbum, author: detail.author?.name, fallbackAlbum: fallbackAlbum ) } @@ -490,7 +492,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 +507,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/PlaylistDetailViewModelTests.swift b/Tests/KasetTests/PlaylistDetailViewModelTests.swift index 03479f21..74e0ef42 100644 --- a/Tests/KasetTests/PlaylistDetailViewModelTests.swift +++ b/Tests/KasetTests/PlaylistDetailViewModelTests.swift @@ -1140,4 +1140,81 @@ 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) + } } From 4dae7d8042c1642c6aeea418901ba1654c628bf5 Mon Sep 17 00:00:00 2001 From: Daniel Andersson Date: Sat, 11 Jul 2026 01:30:39 +0200 Subject: [PATCH 2/3] test: deflake NotificationService tests by polling with a deadline --- .../KasetTests/NotificationServiceTests.swift | 36 +++++++++++++------ 1 file changed, 25 insertions(+), 11 deletions(-) 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") } } From f9f1a6bbabb738acec73fce9a439657aa2ea6666 Mon Sep 17 00:00:00 2001 From: tsibog Date: Sun, 12 Jul 2026 22:27:40 +0200 Subject: [PATCH 3/3] refine playlist track sort: header placement, Default label, stable rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Move the sort control into the header action row, right-aligned after the Play/Queue buttons, instead of a separate row above the track list. - Rename the 'Custom order' option to 'Default'. - Give each track row a stable identity (song id + occurrence, numbered in playlist order to keep duplicates distinct) so re-sorting moves rows in place instead of swapping slot contents — the latter reloaded row artwork and like/dislike state, causing the blink and lag on sort changes. - displayedTracks now derives from the identified rows so playback order stays in sync with the displayed order. --- .../ViewModels/PlaylistDetailViewModel.swift | 47 ++++++++++++++----- .../Views/PlaylistDetailView+TrackSort.swift | 46 +++++++++--------- Sources/Kaset/Views/PlaylistDetailView.swift | 23 +++++---- .../PlaylistDetailViewModelTests.swift | 37 +++++++++++++++ 4 files changed, 105 insertions(+), 48 deletions(-) diff --git a/Sources/Kaset/ViewModels/PlaylistDetailViewModel.swift b/Sources/Kaset/ViewModels/PlaylistDetailViewModel.swift index 4947173c..2c7dfcb3 100644 --- a/Sources/Kaset/ViewModels/PlaylistDetailViewModel.swift +++ b/Sources/Kaset/ViewModels/PlaylistDetailViewModel.swift @@ -46,22 +46,43 @@ final class PlaylistDetailViewModel { /// Current sort direction; ignored when `sortOption` is `.custom`. private(set) var sortAscending: Bool = true - /// The playlist tracks in the currently selected sort order. - var displayedTracks: [Song] { + /// 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 tracks + return identified case .title: - return self.sortedByStandardCompare(tracks) { $0.title } + return self.sortedByStandardCompare(identified) { $0.song.title } case .artist: - return self.sortedByStandardCompare(tracks, tieBreaker: { $0.title }, key: { $0.artistsDisplay }) + return self.sortedByStandardCompare(identified, tieBreaker: { $0.song.title }, key: { $0.song.artistsDisplay }) case .duration: - return self.sortedByDuration(tracks) + 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 @@ -525,11 +546,11 @@ final class PlaylistDetailViewModel { } } - private func sortedByStandardCompare( - _ tracks: [Song], - tieBreaker: ((Song) -> String)? = nil, - key: (Song) -> String - ) -> [Song] { + 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 { @@ -540,9 +561,9 @@ final class PlaylistDetailViewModel { } } - private func sortedByDuration(_ tracks: [Song]) -> [Song] { + private func sortedByDuration(_ tracks: [IdentifiedTrack]) -> [IdentifiedTrack] { tracks.sorted { lhs, rhs in - switch (lhs.duration, rhs.duration) { + switch (lhs.song.duration, rhs.song.duration) { case let (lhsDuration?, rhsDuration?): self.sortAscending ? lhsDuration < rhsDuration : lhsDuration > rhsDuration case (nil, nil), (nil, _): diff --git a/Sources/Kaset/Views/PlaylistDetailView+TrackSort.swift b/Sources/Kaset/Views/PlaylistDetailView+TrackSort.swift index b4d7d4f0..55899c82 100644 --- a/Sources/Kaset/Views/PlaylistDetailView+TrackSort.swift +++ b/Sources/Kaset/Views/PlaylistDetailView+TrackSort.swift @@ -3,37 +3,33 @@ import SwiftUI @available(macOS 26.0, *) extension PlaylistDetailView { @ViewBuilder - func trackSortMenu(_ detail: PlaylistDetail) -> some View { + func trackSortControl(_ detail: PlaylistDetail) -> some View { if detail.tracks.count >= 2 { - HStack { - Spacer() - - 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") - } + 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")) + } label: { + self.trackSortMenuLabel } + .menuStyle(.borderlessButton) + .fixedSize() + .foregroundStyle(.secondary) + .help(String(localized: "Sort tracks")) + .accessibilityLabel(String(localized: "Sort tracks")) } } @@ -49,7 +45,7 @@ extension PlaylistDetailView { private func sortOptionTitle(_ option: PlaylistTrackSortOption) -> String { switch option { case .custom: - String(localized: "Custom order") + String(localized: "Default") case .title: String(localized: "Title") case .artist: diff --git a/Sources/Kaset/Views/PlaylistDetailView.swift b/Sources/Kaset/Views/PlaylistDetailView.swift index de538323..20fb33eb 100644 --- a/Sources/Kaset/Views/PlaylistDetailView.swift +++ b/Sources/Kaset/Views/PlaylistDetailView.swift @@ -135,10 +135,8 @@ struct PlaylistDetailView: View { year: nil, trackCount: detail.trackCount ?? detail.tracks.count ) - self.trackSortMenu(detail) - self.tracksView( - self.viewModel.displayedTracks, isAlbum: detail.isAlbum, author: detail.author?.name, + self.viewModel.displayedTrackRows, isAlbum: detail.isAlbum, author: detail.author?.name, fallbackAlbum: fallbackAlbum ) } @@ -191,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) } @@ -242,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) diff --git a/Tests/KasetTests/PlaylistDetailViewModelTests.swift b/Tests/KasetTests/PlaylistDetailViewModelTests.swift index 74e0ef42..1b9a847e 100644 --- a/Tests/KasetTests/PlaylistDetailViewModelTests.swift +++ b/Tests/KasetTests/PlaylistDetailViewModelTests.swift @@ -1217,4 +1217,41 @@ struct PlaylistDetailViewModelTests { #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") + } }