From a9db3e1412779ce82104c79ea518190f03783235 Mon Sep 17 00:00:00 2001 From: psavarmattas Date: Sat, 15 Aug 2026 01:13:28 +0100 Subject: [PATCH] fix(settings): make Settings its own top-level branch, not swapped content MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The .id() fix on the detail pane wasn't enough — still reported as the document staying visible with only the sidebar switching. NavigationSplitView bridges to NSSplitViewController on macOS, and apparently doesn't reliably replace already-mounted detail content (a NavigationStack with real push history) for something unrelated inside one persisting split view instance, identity hints or not. Restructured so `navigation.isShowingSettings` picks between two entirely separate NavigationSplitView instances (settingsContent / mainContent) at the top of body, instead of branching on content inside a single one. A different top-level view hierarchy is a guaranteed full teardown of whatever AppKit was holding onto — no split-view instance persists across the switch for it to get confused about reusing. Also dropped the now-redundant `isShowingSettings` guards scattered through mainContent's toolbar/leadingToolbarContent — moot once mainContent only ever renders while Settings isn't showing. --- .../Collections/ContentView_macOS.swift | 104 ++++++++++-------- 1 file changed, 58 insertions(+), 46 deletions(-) diff --git a/Outpost/Features/Collections/ContentView_macOS.swift b/Outpost/Features/Collections/ContentView_macOS.swift index 5cefa75..ad42a31 100644 --- a/Outpost/Features/Collections/ContentView_macOS.swift +++ b/Outpost/Features/Collections/ContentView_macOS.swift @@ -40,40 +40,67 @@ struct ContentView_macOS: View { } var body: some View { + // A totally separate top-level branch, not content swapped inside + // one persistent `NavigationSplitView` — that was tried first (an + // `if/else` inside a single split view, then an explicit `.id()` on + // just the detail pane) and neither reliably replaced the detail + // pane's content on macOS: `NavigationSplitView` bridges to + // `NSSplitViewController`, and swapping a `NavigationStack` with + // real push history for a plain view inside one persisting instance + // doesn't propagate the way plain SwiftUI identity rules would + // suggest. A different `if` branch is a genuinely different view + // hierarchy, so there's no existing split view instance for AppKit + // to get confused about reusing. + if navigation.isShowingSettings { + settingsContent + } else { + mainContent + } + } + + private var settingsContent: some View { NavigationSplitView { - Group { - if navigation.isShowingSettings { - SettingsSidebarList( - selection: selectedSettingsSectionBinding, - onDone: { navigation.isShowingSettings = false } - ) - } else { - VStack(spacing: 0) { - SidebarSearchField(text: $globalSearchQuery) - Divider() - if isOfflineModeEnabled { - OfflineBanner(isManual: true) - Divider() - } else if !session.networkMonitor.isOnline { - OfflineConnectionPromptBanner(onEnableOfflineMode: { isOfflineModeEnabled = true }) - Divider() - } - sidebar - AccountFooter() - } + SettingsSidebarList( + selection: selectedSettingsSectionBinding, + onDone: { navigation.isShowingSettings = false } + ) + .navigationSplitViewColumnWidth(min: 220, ideal: 260) + } detail: { + SettingsView(section: navigation.selectedSettingsSection ?? .appearance) + } + .navigationTitle("") + .toolbar { + ToolbarItem(placement: .navigation) { + HStack(spacing: 6) { + Image(systemName: "gearshape.fill") + Text("Settings") } + .font(.headline) + .padding(.horizontal, 10) + .padding(.vertical, 4) + .background(.fill.tertiary, in: Capsule()) + } + } + } + + private var mainContent: some View { + NavigationSplitView { + VStack(spacing: 0) { + SidebarSearchField(text: $globalSearchQuery) + Divider() + if isOfflineModeEnabled { + OfflineBanner(isManual: true) + Divider() + } else if !session.networkMonitor.isOnline { + OfflineConnectionPromptBanner(onEnableOfflineMode: { isOfflineModeEnabled = true }) + Divider() + } + sidebar + AccountFooter() } .navigationSplitViewColumnWidth(min: 220, ideal: 260) } detail: { detail - // NavigationSplitView on macOS doesn't reliably tear down the - // detail pane's previous content when swapping between very - // different subtrees (a NavigationStack with a document - // pushed vs. plain SettingsView) off an implicit branch - // alone — an explicit identity change forces a real - // teardown/remount instead of it silently leaving the old - // document visible underneath. - .id(navigation.isShowingSettings) } // Empty rather than a real value: with one, the native title rendered // in the same toolbar row as the custom `.navigation` pill below, @@ -108,7 +135,7 @@ struct ContentView_macOS: View { // `CollectionOverviewView`, so on Home it was a dead end: a // user could click it, type, and nothing would happen. The // sidebar's global search already covers "search everything." - if !navigation.isShowingSettings && !(isShowingHome && documentPath.isEmpty) { + if !(isShowingHome && documentPath.isEmpty) { ToolbarItem(placement: .primaryAction) { contextualSearchField } @@ -178,12 +205,7 @@ struct ContentView_macOS: View { @ViewBuilder private var leadingToolbarContent: some View { Group { - if navigation.isShowingSettings { - HStack(spacing: 6) { - Image(systemName: "gearshape.fill") - Text("Settings") - } - } else if !trimmedGlobalQuery.isEmpty { + if !trimmedGlobalQuery.isEmpty { HStack(spacing: 6) { Image(systemName: "magnifyingglass") Text("Search") @@ -262,7 +284,6 @@ struct ContentView_macOS: View { globalSearchQuery = "" selectedCollection = collection isContextualSearchExpanded = true - navigation.isShowingSettings = false Task { @MainActor in isContextualSearchFocused = true } @@ -273,13 +294,11 @@ struct ContentView_macOS: View { /// hierarchy immediately rather than just the leaf. private func selectDocumentChain(_ collection: OutlineCollection, _ chain: [OutlineDocument]) { selectedCollection = collection - navigation.isShowingSettings = false replaceDocumentPath(with: chain) } /// Flat-list / search-result clicks: no known ancestors, single-level push. private func openDocument(_ document: OutlineDocument) { - navigation.isShowingSettings = false replaceDocumentPath(with: [document]) } @@ -298,14 +317,7 @@ struct ContentView_macOS: View { @ViewBuilder private var detail: some View { - if navigation.isShowingSettings { - // Deliberately not a `NavigationStack` destination or a separate - // window — it's swapped into the same detail pane Home/ - // collections use, alongside the real sidebar (now showing - // `SettingsSidebarList` instead of the collections tree) rather - // than a page with its own nested mini sidebar. - SettingsView(section: navigation.selectedSettingsSection ?? .appearance) - } else if let apiClient = session.apiClient { + if let apiClient = session.apiClient { // A fresh NavigationStack per collection (or when entering/leaving // search), so switching either also clears any pushed document. NavigationStack(path: $documentPath) {