mirror of
https://github.com/awizemann/scarf.git
synced 2026-05-10 10:36:35 +00:00
fix: address code-review findings from Apr 22 commits
Three follow-ups from reviewing1989fee(sidebar-width persist) and4163595(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:
@@ -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,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)")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user