rasmuslos / AmpFin

Native Jellyfin music player for iOS & iPadOS
Other
107 stars 10 forks source link

Minor fixes #42

Closed gnattu closed 1 month ago

gnattu commented 1 month ago

This PR:

Additionally, you need to change the build target to Mac Catalyst and run Archive to get a macOS-compatible app in .app. The previous builds only released an iOS app in .ipa.

I'm busy with bugfixing the server-side issue of 10.9 recently, if you have faced any server-side issue please ask and let me see what we can do on the server side.

rasmuslos commented 1 month ago

Thanks, the buffering thing has been annoying me for a while now! I am currently on vacation but will look into this when I have some spare time and a stable WiFi connection.

rasmuslos commented 1 month ago

I tried to build the catalyst app after you added support but was unable to export it because apparently you cannot distribute an app that contains the Siri entitlement outside of the AppStore. I will have to look into that again...

gnattu commented 1 month ago

Rebased to current main branch and pushed more fix: corrects the navigation for side bar view. The NavigationLink without a selected tab (like in search view) will break and use the dummy provider. I fixed that by forcing it to use the navigation notification but as previously discussed this makes it impossible to back to the search view. A better approach to this probably is to make a dedicated navigation manager that stores the path and provider of the current stack.

rasmuslos commented 1 month ago

I also don't really like the approach of using notifications, you can just pass a binding to the search view and the sidebar updates the data provider,

I files a bug report for the NavigationStack thing, but haven't heard back. This can hopefully be improved once Apple fixes it.

If its fine with you I would just push this fix onto the main branch:

diff --git a/Multiplatform/Common/SearchView.swift b/Multiplatform/Common/SearchView.swift
index d40fb5d..7449321 100644
--- a/Multiplatform/Common/SearchView.swift
+++ b/Multiplatform/Common/SearchView.swift
@@ -11,7 +11,7 @@ import AmpFinKit
 import AFPlayback

 internal struct SearchView: View {
-    @Default(.searchTab) private var searchTab
+    @Binding var searchTab: Tab

     @State private var search = ""
     @State private var task: Task<(), Never>? = nil
@@ -22,76 +22,73 @@ internal struct SearchView: View {
     @State private var playlists = [Playlist]()

     var body: some View {
-        NavigationStack {
-            List {
-                Picker("search.library", selection: $searchTab) {
-                    Text("search.jellyfin")
-                        .tag(Tab.online)
-                    Text("search.downloaded")
-                        .tag(Tab.offline)
-                }
-                .pickerStyle(.segmented)
-                .listRowSeparator(.hidden)
-                .listRowInsets(.init(top: 0, leading: 20, bottom: 0, trailing: 20))
-                
-                if !artists.isEmpty {
-                    Section("section.artists") {
-                        ArtistList(artists: artists)
-                            .padding(.horizontal, 20)
-                    }
+        List {
+            Picker("search.library", selection: $searchTab) {
+                Text("search.jellyfin")
+                    .tag(Tab.online)
+                Text("search.downloaded")
+                    .tag(Tab.offline)
+            }
+            .pickerStyle(.segmented)
+            .listRowSeparator(.hidden)
+            .listRowInsets(.init(top: 0, leading: 20, bottom: 0, trailing: 20))
+            
+            if !artists.isEmpty {
+                Section("section.artists") {
+                    ArtistList(artists: artists)
+                        .padding(.horizontal, 20)
                 }
-                
-                if !albums.isEmpty {
-                    Section("section.albums") {
-                        ForEach(albums) { album in
-                            NavigationLink(destination: AlbumView(album: album)) {
-                                AlbumListRow(album: album)
-                            }
-                            .listRowInsets(.init(top: 6, leading: 20, bottom: 6, trailing: 20))
+            }
+            
+            if !albums.isEmpty {
+                Section("section.albums") {
+                    ForEach(albums) { album in
+                        NavigationLink(destination: AlbumView(album: album)) {
+                            AlbumListRow(album: album)
                         }
+                        .listRowInsets(.init(top: 6, leading: 20, bottom: 6, trailing: 20))
                     }
                 }
-                
-                if !playlists.isEmpty {
-                    Section("section.playlists") {
-                        ForEach(playlists) { playlist in
-                            NavigationLink(destination: PlaylistView(playlist: playlist)) {
-                                PlaylistListRow(playlist: playlist)
-                            }
-                            .listRowInsets(.init(top: 6, leading: 20, bottom: 6, trailing: 20))
+            }
+            
+            if !playlists.isEmpty {
+                Section("section.playlists") {
+                    ForEach(playlists) { playlist in
+                        NavigationLink(destination: PlaylistView(playlist: playlist)) {
+                            PlaylistListRow(playlist: playlist)
                         }
+                        .listRowInsets(.init(top: 6, leading: 20, bottom: 6, trailing: 20))
                     }
                 }
-                
-                if !tracks.isEmpty {
-                    Section("section.tracks") {
-                        ForEach(Array(tracks.enumerated()), id: \.offset) { index, track in
-                            TrackListRow(track: track) {
-                                AudioPlayer.current.startPlayback(tracks: tracks, startIndex: index, shuffle: false, playbackInfo: .init(container: nil, search: search))
-                            }
-                            .listRowInsets(.init(top: 6, leading: 20, bottom: 6, trailing: 20))
+            }
+            
+            if !tracks.isEmpty {
+                Section("section.tracks") {
+                    ForEach(Array(tracks.enumerated()), id: \.offset) { index, track in
+                        TrackListRow(track: track) {
+                            AudioPlayer.current.startPlayback(tracks: tracks, startIndex: index, shuffle: false, playbackInfo: .init(container: nil, search: search))
                         }
+                        .listRowInsets(.init(top: 6, leading: 20, bottom: 6, trailing: 20))
                     }
                 }
             }
-            .listStyle(.plain)
-            .navigationTitle("title.search")
-            .searchable(text: $search, placement: .navigationBarDrawer(displayMode: .always), prompt: "search.placeholder")
-            .autocorrectionDisabled()
-            .textInputAutocapitalization(.never)
-            .modifier(NowPlaying.SafeAreaModifier())
-            .task(id: searchTab) {
-                fetchSearchResults(shouldReset: true)
-            }
-            .refreshable {
-                fetchSearchResults(shouldReset: true)
-            }
-            .onChange(of: search) {
-                fetchSearchResults(shouldReset: false)
-            }
-            .modifier(AccountToolbarButtonModifier(requiredSize: .compact))
         }
-        .environment(\.libraryDataProvider, searchTab.dataProvider)
+        .listStyle(.plain)
+        .navigationTitle("title.search")
+        .searchable(text: $search, placement: .navigationBarDrawer(displayMode: .always), prompt: "search.placeholder")
+        .autocorrectionDisabled()
+        .textInputAutocapitalization(.never)
+        .modifier(NowPlaying.SafeAreaModifier())
+        .task(id: searchTab) {
+            fetchSearchResults(shouldReset: true)
+        }
+        .refreshable {
+            fetchSearchResults(shouldReset: true)
+        }
+        .onChange(of: search) {
+            fetchSearchResults(shouldReset: false)
+        }
+        .modifier(AccountToolbarButtonModifier(requiredSize: .compact))
     }

     private func fetchSearchResults(shouldReset: Bool) {
@@ -131,7 +128,7 @@ internal extension SearchView {
         case offline
     }
 }
-private extension SearchView.Tab {
+internal extension SearchView.Tab {
     var dataProvider: LibraryDataProvider {
         switch self {
             case .online:
@@ -143,5 +140,5 @@ private extension SearchView.Tab {
 }

 #Preview {
-    SearchView()
+    SearchView(searchTab: .constant(.online))
 }
diff --git a/Multiplatform/Navigation/Sidebar/Sidebar+Selection.swift b/Multiplatform/Navigation/Sidebar/Sidebar+Selection.swift
index 68b188c..e0fa446 100644
--- a/Multiplatform/Navigation/Sidebar/Sidebar+Selection.swift
+++ b/Multiplatform/Navigation/Sidebar/Sidebar+Selection.swift
@@ -135,7 +135,7 @@ internal extension Sidebar.Panel {
                 ArtistsView(albumOnly: false)

             case .search:
-                SearchView()
+                EmptyView()

             case .playlist(let id):
                 PlaylistLoadView(playlistId: id)
diff --git a/Multiplatform/Navigation/Sidebar/Sidebar.swift b/Multiplatform/Navigation/Sidebar/Sidebar.swift
index eccb2b8..0dfab92 100644
--- a/Multiplatform/Navigation/Sidebar/Sidebar.swift
+++ b/Multiplatform/Navigation/Sidebar/Sidebar.swift
@@ -12,8 +12,39 @@ import AFOffline

 internal struct Sidebar: View {
     @Default(.sidebarSelection) private var sidebarSelection
+    @Default(.searchTab) private var searchTab

-    var provider: DataProvider?
+    var provider: DataProvider? = nil
+    
+    private var dataProvider: LibraryDataProvider {
+        if sidebarSelection?.panel == .search {
+            return searchTab.dataProvider
+        }
+        
+        return sidebarSelection?.provider?.libraryProvider ?? MockLibraryDataProvider()
+    }
+    
+    @ViewBuilder
+    private var content: some View {
+        if let sidebarSelection {
+            NavigationStack {
+                Group {
+                    if sidebarSelection.panel == .search {
+                        SearchView(searchTab: $searchTab)
+                    } else {
+                        sidebarSelection.panel.content
+                    }
+                }
+                .id(sidebarSelection.panel)
+                .id(provider)
+            }
+        } else {
+            ProgressView()
+                .onAppear {
+                    sidebarSelection = .init(provider: .online, panel: .tracks)
+                }
+        }
+    }

     var body: some View {
         NavigationSplitView {
@@ -43,20 +74,9 @@ internal struct Sidebar: View {
                 }
             }
         } detail: {
-            if let sidebarSelection = sidebarSelection {
-                NavigationStack {
-                    sidebarSelection.panel.content
-                        .id(sidebarSelection.panel)
-                        .id(sidebarSelection.provider)
-                }
-            } else {
-                ProgressView()
-                    .onAppear {
-                        sidebarSelection = .init(provider: .online, panel: .tracks)
-                    }
-            }
+            content
         }
-        .environment(\.libraryDataProvider, sidebarSelection?.provider?.libraryProvider ?? MockLibraryDataProvider())
+        .environment(\.libraryDataProvider, dataProvider)
         .modifier(NowPlayingBarModifier(visible: provider == nil))
         .modifier(Navigation.NotificationModifier(
             navigateAlbum: {
diff --git a/Multiplatform/Navigation/Tabs.swift b/Multiplatform/Navigation/Tabs.swift
index 6ab6e07..b957503 100644
--- a/Multiplatform/Navigation/Tabs.swift
+++ b/Multiplatform/Navigation/Tabs.swift
@@ -11,6 +11,7 @@ import AmpFinKit

 internal struct Tabs: View {
     @Default(.activeTab) private var activeTab
+    @Default(.searchTab) private var searchTab

     @State private var libraryPath = NavigationPath()
     @State private var downloadsPath = NavigationPath()
@@ -61,11 +62,13 @@ internal struct Tabs: View {

                 // MARK: Search

-                SearchView()
-                    .tag(Selection.search)
-                    .tabItem {
-                        Label("tab.search", systemImage: "magnifyingglass")
-                    }
+                NavigationStack {
+                    SearchView(searchTab: $searchTab)
+                }
+                .tag(Selection.search)
+                .tabItem {
+                    Label("tab.search", systemImage: "magnifyingglass")
+                }
             }
             .modifier(NowPlaying.CompactBarModifier())
         }
diff --git a/Multiplatform/Navigation/XRTabs.swift b/Multiplatform/Navigation/XRTabs.swift
index e82d3c4..edecdf4 100644
--- a/Multiplatform/Navigation/XRTabs.swift
+++ b/Multiplatform/Navigation/XRTabs.swift
@@ -6,8 +6,11 @@
 //

 import SwiftUI
+import Defaults

 struct XRTabs: View {
+    @Default(.searchTab) private var searchTab
+    
     var body: some View {
         TabView {
             Sidebar(provider: .online)
@@ -20,10 +23,12 @@ struct XRTabs: View {
                     Label("tab.downloads", systemImage: "arrow.down")
                 }

-            SearchView()
-                .tabItem {
-                    Label("tab.search", systemImage: "magnifyingglass")
-                }
+            NavigationStack {
+                SearchView(searchTab: $searchTab)
+            }
+            .tabItem {
+                Label("tab.search", systemImage: "magnifyingglass")
+            }
         }
     }
 }
gnattu commented 1 month ago

I tried it and made minor modifications and pushed.

I would prefer merging this to avoid even further conflicts.