fix: address code-review findings from Apr 22 commits

Three follow-ups from reviewing 1989fee (sidebar-width persist) and
4163595 (default server on launch):

- `SplitViewAutosaveFinder` hardcoded `"ScarfMainSidebar"` for every
  window. Since Scarf's `WindowGroup` spawns one window per `ServerID`,
  all windows shared the same `NSSplitView.autosaveName` — AppKit
  documents that name as required-unique, and in practice per-window
  widths collapsed onto a single UserDefaults key. Thread the window's
  `ServerContext` in through `@Environment(\.serverContext)` (already
  wired at `WindowGroup` construction) and suffix the name with the
  server UUID.
- `setDefaultServer` fired `onEntriesChanged`, whose sole consumer is
  `ServerLiveStatusRegistry.rebuild()` for menu-bar fanout. Flipping a
  default flag doesn't change the set of servers; the callback was
  semantic noise. Drop the call — SwiftUI views still redraw on the
  flag flip via `@Observable`'s tracking of `entries`.
- The filled-yellow star in `ManageServersView` had a no-op action
  inside `if !isDefault { ... }` but still animated its pressed state
  on click. Replace the conditional with `.disabled(isDefault)` so the
  row is visually inert when it already is the default.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Alan Wizemann
2026-04-23 00:59:58 +02:00
parent 7311320bfd
commit 7a99547b22
3 changed files with 12 additions and 6 deletions
@@ -82,6 +82,11 @@ final class ServerRegistry {
/// Flip the default server to `id`. Passing `ServerContext.local.id` /// Flip the default server to `id`. Passing `ServerContext.local.id`
/// clears the flag on every remote entry, making Local the implicit /// clears the flag on every remote entry, making Local the implicit
/// default. Passing an unknown ID is a no-op. Persisted on return. /// default. Passing an unknown ID is a no-op. Persisted on return.
///
/// Intentionally doesn't fire `onEntriesChanged` that hook means "the
/// set of servers changed" and drives the menu-bar fanout rebuild. A
/// default-flag flip doesn't change the set; SwiftUI views reading
/// `defaultServerID` redraw via `@Observable`'s tracking of `entries`.
func setDefaultServer(_ id: ServerID) { func setDefaultServer(_ id: ServerID) {
var changed = false var changed = false
for idx in entries.indices { for idx in entries.indices {
@@ -93,7 +98,6 @@ final class ServerRegistry {
} }
if changed { if changed {
save() save()
onEntriesChanged?()
} }
} }
@@ -143,19 +143,20 @@ struct ManageServersView: View {
} }
/// A star button that marks the open-on-launch default. Filled + yellow /// A star button that marks the open-on-launch default. Filled + yellow
/// on the current default row (and non-interactive clicking it is a /// on the current default row (disabled, since clicking would be a
/// no-op since the flag is already set); outline + secondary elsewhere, /// no-op); outline + secondary elsewhere, clicking promotes that row
/// clicking promotes that row to default. /// to default.
@ViewBuilder @ViewBuilder
private func defaultStar(for id: ServerID, currentDefault: ServerID) -> some View { private func defaultStar(for id: ServerID, currentDefault: ServerID) -> some View {
let isDefault = id == currentDefault let isDefault = id == currentDefault
Button { Button {
if !isDefault { registry.setDefaultServer(id) } registry.setDefaultServer(id)
} label: { } label: {
Image(systemName: isDefault ? "star.fill" : "star") Image(systemName: isDefault ? "star.fill" : "star")
.foregroundStyle(isDefault ? .yellow : .secondary) .foregroundStyle(isDefault ? .yellow : .secondary)
} }
.buttonStyle(.borderless) .buttonStyle(.borderless)
.disabled(isDefault)
.help(isDefault ? "Opens on launch" : "Set as default — open this server when Scarf launches.") .help(isDefault ? "Opens on launch" : "Set as default — open this server when Scarf launches.")
} }
+2 -1
View File
@@ -2,6 +2,7 @@ import SwiftUI
struct SidebarView: View { struct SidebarView: View {
@Environment(AppCoordinator.self) private var coordinator @Environment(AppCoordinator.self) private var coordinator
@Environment(\.serverContext) private var serverContext
var body: some View { var body: some View {
@Bindable var coordinator = coordinator @Bindable var coordinator = coordinator
@@ -59,6 +60,6 @@ struct SidebarView: View {
} }
.listStyle(.sidebar) .listStyle(.sidebar)
.navigationTitle("Scarf") .navigationTitle("Scarf")
.splitViewAutosaveName("ScarfMainSidebar") .splitViewAutosaveName("ScarfMainSidebar.\(serverContext.id)")
} }
} }