From d1ed52b82528cbb7b8bbdab4145a83778e3f848f Mon Sep 17 00:00:00 2001 From: psavarmattas Date: Sat, 15 Aug 2026 00:36:28 +0100 Subject: [PATCH] redesign(settings): reuse the real sidebar instead of a mini one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Settings now swaps the actual app sidebar's content (search field, collections tree, account footer) for a section list, instead of SettingsView drawing its own nested sidebar inside the detail pane. Done, wherever it's triggered from, restores the collections tree and detail content exactly as they were. AppNavigation gains selectedSettingsSection (and the SettingsSection enum moves there, shared by the new list and the sidebar host) so the sidebar's list and the detail pane agree on which section is showing. Also deferred the profile menu's "Settings…" action by one runloop tick — setting isShowingSettings synchronously in the same call that dismisses the popover was very likely the source of the AppKit "CA commit" transaction warnings in the console. --- Outpost/Features/Account/AccountFooter.swift | 7 +- .../Account/SettingsSidebarList.swift | 41 +++++++++ Outpost/Features/Account/SettingsView.swift | 92 ++++--------------- .../Collections/ContentView_macOS.swift | 45 ++++++--- Outpost/Root/AppNavigation.swift | 36 +++++++- 5 files changed, 127 insertions(+), 94 deletions(-) create mode 100644 Outpost/Features/Account/SettingsSidebarList.swift diff --git a/Outpost/Features/Account/AccountFooter.swift b/Outpost/Features/Account/AccountFooter.swift index 4aac7fc..79722ee 100644 --- a/Outpost/Features/Account/AccountFooter.swift +++ b/Outpost/Features/Account/AccountFooter.swift @@ -100,7 +100,12 @@ struct AccountFooter: View { .padding(.vertical, 4) menuItem("Profile…") { isShowingProfile = true } - menuItem("Settings…") { navigation.isShowingSettings = true } + // Deferred a tick: setting this synchronously in the same call + // that dismisses this popover collides two AppKit window/layer + // transactions in the same runloop turn (visible in the console + // as "Invalid attempt to open a new transaction during CA + // commit") — letting the popover's dismissal finish first avoids it. + menuItem("Settings…") { Task { @MainActor in navigation.isShowingSettings = true } } Divider() diff --git a/Outpost/Features/Account/SettingsSidebarList.swift b/Outpost/Features/Account/SettingsSidebarList.swift new file mode 100644 index 0000000..bb0f99c --- /dev/null +++ b/Outpost/Features/Account/SettingsSidebarList.swift @@ -0,0 +1,41 @@ +#if os(macOS) +import SwiftUI + +/// Swapped into the real sidebar's content slot (search field, collections +/// tree, account footer) while Settings is open — same sidebar, different +/// content, rather than a separate mini sidebar nested inside a page. "Done" +/// clears `AppNavigation.isShowingSettings`, which puts the collections tree +/// back. +struct SettingsSidebarList: View { + @Binding var selection: SettingsSection? + let onDone: () -> Void + + var body: some View { + VStack(spacing: 0) { + HStack { + Text("Settings") + .font(.headline) + Spacer() + } + .padding(.horizontal, 16) + .padding(.vertical, 12) + + Divider() + + List(SettingsSection.allCases, selection: $selection) { section in + Label(section.title, systemImage: section.icon) + .tag(section) + } + .listStyle(.sidebar) + + Divider() + + Button("Done", action: onDone) + .keyboardShortcut(.cancelAction) + .buttonStyle(.borderedProminent) + .frame(maxWidth: .infinity) + .padding(12) + } + } +} +#endif diff --git a/Outpost/Features/Account/SettingsView.swift b/Outpost/Features/Account/SettingsView.swift index 20d4a4c..6b63636 100644 --- a/Outpost/Features/Account/SettingsView.swift +++ b/Outpost/Features/Account/SettingsView.swift @@ -2,49 +2,18 @@ import SwiftUI import OutlineKit -private enum SettingsSection: String, CaseIterable, Identifiable, Hashable { - case appearance, account, offlineSync, about - - var id: String { rawValue } - - var title: String { - switch self { - case .appearance: return "Appearance" - case .account: return "Account" - case .offlineSync: return "Offline & Sync" - case .about: return "About" - } - } - - var icon: String { - switch self { - case .appearance: return "paintbrush" - case .account: return "person.crop.circle" - case .offlineSync: return "arrow.triangle.2.circlepath" - case .about: return "info.circle" - } - } -} - -/// Full-page Settings — swapped into `ContentView_macOS`'s detail pane (see -/// `AppNavigation`), alongside the sidebar, not a separate popup window. -/// Replaces the old `Settings {}` scene / `PreferencesView` and folds in -/// what used to be the standalone "About Outpost" window's content too, so -/// everything about the app lives in one place. -/// -/// Own section list + single-section detail (same shape as macOS System -/// Settings) rather than a page of stacked/gridded cards — cards of visibly -/// different heights never sit next to each other for comparison this way, -/// and the layout holds up at any window size or aspect ratio without -/// needing to reflow a grid. +/// Settings *detail* content for one section — the section list itself now +/// lives in `ContentView_macOS`'s real sidebar (swapped in over the +/// collections tree while `AppNavigation.isShowingSettings` is set, not a +/// separate mini sidebar of its own), so this view only ever renders +/// whichever section is currently selected. struct SettingsView: View { - let onDone: () -> Void + let section: SettingsSection @Environment(SessionStore.self) private var session @AppStorage("outpost.appearance") private var appearance: AppAppearance = .system @AppStorage(CachingOutlineAPIClient.offlineModeDefaultsKey) private var isOfflineModeEnabled = false @AppStorage("outpost.fullLocalSyncEnabled") private var isFullLocalSyncEnabled = false - @State private var selectedSection: SettingsSection? = .appearance @State private var isShowingLogoutConfirmation = false @State private var storageSummary: CacheStorageSummary? @State private var pendingOperations: [PendingOperationSummary] = [] @@ -54,20 +23,10 @@ struct SettingsView: View { @State private var lastFlushSummary: SyncFlushSummary? var body: some View { - VStack(spacing: 0) { - header - Divider() - HStack(spacing: 0) { - sectionList - .frame(width: 190) - Divider() - ScrollView { - sectionDetail(selectedSection ?? .appearance) - .padding(28) - .frame(maxWidth: .infinity, alignment: .leading) - } - .frame(maxWidth: .infinity, maxHeight: .infinity) - } + ScrollView { + sectionDetail + .padding(28) + .frame(maxWidth: .infinity, alignment: .leading) } .frame(maxWidth: .infinity, maxHeight: .infinity) .background(.background) @@ -75,27 +34,8 @@ struct SettingsView: View { .logoutConfirmationDialog(isPresented: $isShowingLogoutConfirmation, session: session) } - private var header: some View { - HStack { - Text("Settings") - .font(.title2.bold()) - Spacer() - Button("Done", action: onDone) - .keyboardShortcut(.cancelAction) - } - .padding(20) - } - - private var sectionList: some View { - List(SettingsSection.allCases, selection: $selectedSection) { section in - Label(section.title, systemImage: section.icon) - .tag(section) - } - .listStyle(.sidebar) - } - @ViewBuilder - private func sectionDetail(_ section: SettingsSection) -> some View { + private var sectionDetail: some View { switch section { case .appearance: appearanceDetail case .account: accountDetail @@ -104,7 +44,7 @@ struct SettingsView: View { } } - private func sectionHeader(_ section: SettingsSection) -> some View { + private var sectionHeader: some View { Text(section.title) .font(.title.bold()) } @@ -113,7 +53,7 @@ struct SettingsView: View { private var appearanceDetail: some View { VStack(alignment: .leading, spacing: 16) { - sectionHeader(.appearance) + sectionHeader Picker("Appearance", selection: $appearance) { ForEach(AppAppearance.allCases) { option in Text(option.label).tag(option) @@ -129,7 +69,7 @@ struct SettingsView: View { private var accountDetail: some View { VStack(alignment: .leading, spacing: 16) { - sectionHeader(.account) + sectionHeader VStack(alignment: .leading, spacing: 10) { labeledRow("Signed in as", session.userName ?? "—") if let email = session.userEmail { @@ -151,7 +91,7 @@ struct SettingsView: View { private var offlineSyncDetail: some View { VStack(alignment: .leading, spacing: 20) { - sectionHeader(.offlineSync) + sectionHeader VStack(alignment: .leading, spacing: 6) { Toggle("Offline Mode", isOn: $isOfflineModeEnabled) @@ -312,7 +252,7 @@ struct SettingsView: View { private var aboutDetail: some View { VStack(alignment: .leading, spacing: 16) { - sectionHeader(.about) + sectionHeader AboutInfoView() .frame(maxWidth: 420, alignment: .leading) } diff --git a/Outpost/Features/Collections/ContentView_macOS.swift b/Outpost/Features/Collections/ContentView_macOS.swift index bbcb029..e568151 100644 --- a/Outpost/Features/Collections/ContentView_macOS.swift +++ b/Outpost/Features/Collections/ContentView_macOS.swift @@ -27,17 +27,37 @@ struct ContentView_macOS: View { globalSearchQuery.trimmingCharacters(in: .whitespacesAndNewlines) } + /// `@Environment(AppNavigation.self)` doesn't hand out `$`-bindings on its + /// own (that's `@Bindable`'s job, and introducing one in `body` would + /// mean restructuring it away from a single implicit-return expression) + /// — a manually-built `Binding` over the same reference is simpler here. + private var selectedSettingsSectionBinding: Binding { + Binding( + get: { navigation.selectedSettingsSection }, + set: { navigation.selectedSettingsSection = $0 } + ) + } + var body: some View { NavigationSplitView { - VStack(spacing: 0) { - SidebarSearchField(text: $globalSearchQuery) - Divider() - if !session.networkMonitor.isOnline { - OfflineBanner() - Divider() + Group { + if navigation.isShowingSettings { + SettingsSidebarList( + selection: selectedSettingsSectionBinding, + onDone: { navigation.isShowingSettings = false } + ) + } else { + VStack(spacing: 0) { + SidebarSearchField(text: $globalSearchQuery) + Divider() + if !session.networkMonitor.isOnline { + OfflineBanner() + Divider() + } + sidebar + AccountFooter() + } } - sidebar - AccountFooter() } .navigationSplitViewColumnWidth(min: 220, ideal: 260) } detail: { @@ -268,10 +288,11 @@ struct ContentView_macOS: View { private var detail: some View { if navigation.isShowingSettings { // Deliberately not a `NavigationStack` destination or a separate - // window — it's its own page swapped into the same detail pane - // Home/collections use, so the sidebar (and the ability to just - // click something else in it) stays available while it's open. - SettingsView(onDone: { navigation.isShowingSettings = false }) + // 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 { // A fresh NavigationStack per collection (or when entering/leaving // search), so switching either also clears any pushed document. diff --git a/Outpost/Root/AppNavigation.swift b/Outpost/Root/AppNavigation.swift index 6fcd885..8d31750 100644 --- a/Outpost/Root/AppNavigation.swift +++ b/Outpost/Root/AppNavigation.swift @@ -1,12 +1,38 @@ import Observation -/// Cross-cutting UI state that doesn't belong to any one screen — currently -/// just "is Settings showing." Lives at `RootView` and is read wherever -/// something needs to open Settings (the profile menu) or render it (RootView -/// itself, as a full-window overlay rather than a separate popup window — -/// `openSettings()`'s `Settings {}` scene doesn't offer that). +enum SettingsSection: String, CaseIterable, Identifiable, Hashable { + case appearance, account, offlineSync, about + + var id: String { rawValue } + + var title: String { + switch self { + case .appearance: return "Appearance" + case .account: return "Account" + case .offlineSync: return "Offline & Sync" + case .about: return "About" + } + } + + var icon: String { + switch self { + case .appearance: return "paintbrush" + case .account: return "person.crop.circle" + case .offlineSync: return "arrow.triangle.2.circlepath" + case .about: return "info.circle" + } + } +} + +/// Cross-cutting UI state that doesn't belong to any one screen. Lives at +/// `OutpostApp` (so both `RootView`'s content and its `⌘,` command can reach +/// it) and is read wherever something needs to open Settings (the profile +/// menu) or render it — as a swap of the *existing* sidebar/detail panes in +/// `ContentView_macOS`, not a separate popup window or an overlay that hides +/// the sidebar. @Observable @MainActor final class AppNavigation { var isShowingSettings = false + var selectedSettingsSection: SettingsSection? = .appearance }