diff --git a/Thaw/MenuBar/IceBar/IceBar.swift b/Thaw/MenuBar/IceBar/IceBar.swift index 3a06b490..beb9b352 100644 --- a/Thaw/MenuBar/IceBar/IceBar.swift +++ b/Thaw/MenuBar/IceBar/IceBar.swift @@ -360,7 +360,7 @@ private struct IceBarContentView: View { } private var itemSpacing: CGFloat { - let offset = displaySettings.configuration(for: screen.displayID).itemSpacingOffset + let offset = appState.settings.general.itemSpacingOffset return max(0, CGFloat(offset).rounded()) } diff --git a/Thaw/MenuBar/MenuBarItems/MenuBarItemManager.swift b/Thaw/MenuBar/MenuBarItems/MenuBarItemManager.swift index 950b9e2f..55bdcef7 100644 --- a/Thaw/MenuBar/MenuBarItems/MenuBarItemManager.swift +++ b/Thaw/MenuBar/MenuBarItems/MenuBarItemManager.swift @@ -924,9 +924,9 @@ final class MenuBarItemManager: ObservableObject { // Boot race: a cold (performSetup) or expected-set settling must // not be torn down by a transient preflight that the boot path - // also kicks off (DisplaySettingsManager.applyActiveDisplaySpacing, - // ProfileManager.layoutTask). Preserve the merged expected set so - // a later non-preflight call still has it; otherwise return. + // also kicks off (ProfileManager.layoutTask). Preserve the merged + // expected set so a later non-preflight call still has it; otherwise + // return. if let existing = settlingKind, incomingKind == .preflight, existing == .cold || existing == .expectedSet diff --git a/Thaw/MenuBar/Spacing/MenuBarItemSpacingManager.swift b/Thaw/MenuBar/Spacing/MenuBarItemSpacingManager.swift index dc31cadd..c49d7a5d 100644 --- a/Thaw/MenuBar/Spacing/MenuBarItemSpacingManager.swift +++ b/Thaw/MenuBar/Spacing/MenuBarItemSpacingManager.swift @@ -79,9 +79,9 @@ final class MenuBarItemSpacingManager { /// immediately, even though the first call is still mid-relaunch /// wave. With the semaphore the second caller queues behind the /// first; by the time it runs, the first call has completed and - /// applyActiveDisplaySpacing has already started a settling - /// period, so a subsequent applyProfileLayout correctly waits - /// for items to stabilize before moving them. + /// the spacing caller has already started a settling period, so a + /// subsequent applyProfileLayout correctly waits for items to + /// stabilize before moving them. private let applyOffsetSemaphore = SimpleSemaphore(value: 1) /// Runs a command with the given arguments. diff --git a/Thaw/Settings/Models/DisplayIceBarConfiguration.swift b/Thaw/Settings/Models/DisplayIceBarConfiguration.swift index d0afe2d0..06d611cc 100644 --- a/Thaw/Settings/Models/DisplayIceBarConfiguration.swift +++ b/Thaw/Settings/Models/DisplayIceBarConfiguration.swift @@ -29,21 +29,13 @@ struct DisplayIceBarConfiguration: Codable, Equatable { /// Valid range is 2 through 10. let gridColumns: Int - /// The menu bar item spacing offset to apply when this display is the - /// active menu bar display. Range is -16 to +16. The OS reads - /// NSStatusItemSpacing as a single system-wide value, so this is the - /// value that gets written + relaunched whenever this display becomes - /// (or remains) the active menu bar display. - let itemSpacingOffset: Double - /// Default configuration (disabled, dynamic location, horizontal layout). static let defaultConfiguration = DisplayIceBarConfiguration( useIceBar: false, iceBarLocation: .dynamic, alwaysShowHiddenItems: false, iceBarLayout: .horizontal, - gridColumns: 4, - itemSpacingOffset: 0 + gridColumns: 4 ) /// Returns a new configuration with the `useIceBar` flag replaced. @@ -53,8 +45,7 @@ struct DisplayIceBarConfiguration: Codable, Equatable { iceBarLocation: iceBarLocation, alwaysShowHiddenItems: alwaysShowHiddenItems, iceBarLayout: iceBarLayout, - gridColumns: gridColumns, - itemSpacingOffset: itemSpacingOffset + gridColumns: gridColumns ) } @@ -65,8 +56,7 @@ struct DisplayIceBarConfiguration: Codable, Equatable { iceBarLocation: value, alwaysShowHiddenItems: alwaysShowHiddenItems, iceBarLayout: iceBarLayout, - gridColumns: gridColumns, - itemSpacingOffset: itemSpacingOffset + gridColumns: gridColumns ) } @@ -77,8 +67,7 @@ struct DisplayIceBarConfiguration: Codable, Equatable { iceBarLocation: iceBarLocation, alwaysShowHiddenItems: value, iceBarLayout: iceBarLayout, - gridColumns: gridColumns, - itemSpacingOffset: itemSpacingOffset + gridColumns: gridColumns ) } @@ -89,8 +78,7 @@ struct DisplayIceBarConfiguration: Codable, Equatable { iceBarLocation: iceBarLocation, alwaysShowHiddenItems: alwaysShowHiddenItems, iceBarLayout: value, - gridColumns: gridColumns, - itemSpacingOffset: itemSpacingOffset + gridColumns: gridColumns ) } @@ -103,22 +91,7 @@ struct DisplayIceBarConfiguration: Codable, Equatable { iceBarLocation: iceBarLocation, alwaysShowHiddenItems: alwaysShowHiddenItems, iceBarLayout: iceBarLayout, - gridColumns: Swift.max(2, Swift.min(value, 10)), - itemSpacingOffset: itemSpacingOffset - ) - } - - /// Returns a new configuration with the itemSpacingOffset replaced. - /// - /// Values are clamped to the range -16 through 16. - func withItemSpacingOffset(_ value: Double) -> DisplayIceBarConfiguration { - DisplayIceBarConfiguration( - useIceBar: useIceBar, - iceBarLocation: iceBarLocation, - alwaysShowHiddenItems: alwaysShowHiddenItems, - iceBarLayout: iceBarLayout, - gridColumns: gridColumns, - itemSpacingOffset: Swift.max(-16, Swift.min(value, 16)) + gridColumns: Swift.max(2, Swift.min(value, 10)) ) } @@ -139,8 +112,7 @@ struct DisplayIceBarConfiguration: Codable, Equatable { iceBarLocation: location, alwaysShowHiddenItems: false, iceBarLayout: .horizontal, - gridColumns: 4, - itemSpacingOffset: 0 + gridColumns: 4 ) } return configs @@ -156,7 +128,6 @@ extension DisplayIceBarConfiguration { case alwaysShowHiddenItems case iceBarLayout case gridColumns - case itemSpacingOffset } init(from decoder: Decoder) throws { @@ -167,7 +138,5 @@ extension DisplayIceBarConfiguration { self.iceBarLayout = try container.decodeIfPresent(IceBarLayout.self, forKey: .iceBarLayout) ?? .horizontal let decodedGridColumns = try container.decodeIfPresent(Int.self, forKey: .gridColumns) ?? 4 self.gridColumns = Swift.max(2, Swift.min(decodedGridColumns, 10)) - let decodedSpacing = try container.decodeIfPresent(Double.self, forKey: .itemSpacingOffset) ?? 0 - self.itemSpacingOffset = Swift.max(-16, Swift.min(decodedSpacing, 16)) } } diff --git a/Thaw/Settings/Models/DisplaySettingsManager.swift b/Thaw/Settings/Models/DisplaySettingsManager.swift index b062609f..9e5d9bb2 100644 --- a/Thaw/Settings/Models/DisplaySettingsManager.swift +++ b/Thaw/Settings/Models/DisplaySettingsManager.swift @@ -36,13 +36,8 @@ final class DisplaySettingsManager: ObservableObject { /// JSON decoder for persistence. private let decoder = JSONDecoder() - /// Reference to AppState for driving spacingManager and itemManager from - /// active-display configuration changes. Held weakly to avoid retain cycles. - private weak var appState: AppState? - /// Performs the initial setup of the manager. - func performSetup(with appState: AppState) { - self.appState = appState + func performSetup(with _: AppState) { loadInitialState() configureCancellables() captureCurrentlyConnectedDisplays() @@ -143,18 +138,10 @@ final class DisplaySettingsManager: ObservableObject { } .store(in: &c) - // Listen for display connect/disconnect to log changes, refresh the - // known-display cache, and re-derive the active display's spacing. - // - // Debounced because didChangeScreenParametersNotification fires - // repeatedly during a single user action: docking, lid close, - // monitor sleep/wake, KVM switch, Sidecar handshake, and external - // display flicker can each post several notifications within a - // few hundred milliseconds. Without the debounce, every flap - // could trigger a relaunch wave (the no-op guard catches the - // common case but does not cover oscillating values during the - // flap window). One second coalesces a single docking event into - // one apply. + // Listen for display connect/disconnect to refresh the known-display + // cache. Debounced because didChangeScreenParametersNotification fires + // repeatedly during docking, lid close, monitor sleep/wake, KVM + // switches, Sidecar handshakes, and external display flicker. NotificationCenter.default .publisher(for: NSApplication.didChangeScreenParametersNotification) .debounce(for: .seconds(1), scheduler: DispatchQueue.main) @@ -162,20 +149,6 @@ final class DisplaySettingsManager: ObservableObject { guard let self else { return } diagLog.info("Screen parameters changed — \(NSScreen.screens.count) screen(s) connected") captureCurrentlyConnectedDisplays() - applyActiveDisplaySpacing(reason: "screenParametersChanged") - } - .store(in: &c) - - // Whenever per-display configurations change (user edit, profile - // load), re-derive what the active display's spacing should be and - // apply it. The no-op guard inside applyOffset() makes this free - // when on-disk already matches. - $configurations - .dropFirst() - .removeDuplicates() - .receive(on: DispatchQueue.main) - .sink { [weak self] _ in - self?.applyActiveDisplaySpacing(reason: "configurationsChanged") } .store(in: &c) @@ -191,51 +164,6 @@ final class DisplaySettingsManager: ObservableObject { cancellables = c } - /// Reads the active display's spacing offset, syncs it into - /// spacingManager.offset, and triggers applyOffset. The no-op guard - /// inside applyOffset skips when on-disk values already match, so this - /// is safe to call on every configurations change. On a real relaunch - /// wave, kicks off a settling period so a subsequent applyProfileLayout - /// (e.g. from a profile switch) waits for items to re-attach before - /// moving them. - private func applyActiveDisplaySpacing(reason: String) { - guard let appState else { return } - let desired = Int(configurationForActiveDisplay().itemSpacingOffset.rounded()) - appState.spacingManager.offset = desired - Task { [weak self] in - guard let self else { return } - // Preflight settling so intermediate late-arriver re-sorts and - // restore logic are suppressed while the wave runs. Cancelled - // below if applyOffset turns out to be a no-op. - appState.itemManager.startSettlingPeriod(reason: "spacingRelaunch:\(reason):preflight") - do { - let outcome = try await appState.spacingManager.applyOffset() - if outcome.didRelaunch { - appState.itemManager.startSettlingPeriod( - reason: "spacingRelaunch:\(reason)", - expectedBundleIDs: outcome.recoveredBundleIDs - ) - // The relaunched apps reattach at OS-default positions. - // Drive the active profile's layout pass so they end up - // in the saved order. Auto-switch doesn't fire when the - // associated profile is unchanged, so without this call - // the post-settle path would only run cross-section - // restore and leave within-section ordering untouched. - appState.profileManager.reapplyActiveProfile() - } else { - appState.itemManager.cancelSettlingPeriod( - reason: "spacingRelaunch:\(reason):noOp" - ) - } - } catch { - appState.itemManager.cancelSettlingPeriod( - reason: "spacingRelaunch:\(reason):error" - ) - diagLog.error("applyActiveDisplaySpacing(\(reason)) failed: \(error)") - } - } - } - /// Handles per-display settings changed externally via Settings URI scheme. private func handleExternalPerDisplaySettingsChange(_ notification: Notification) { guard let key = notification.userInfo?["key"] as? String, diff --git a/Thaw/Settings/Models/GeneralSettings.swift b/Thaw/Settings/Models/GeneralSettings.swift index c05e0564..d1351362 100644 --- a/Thaw/Settings/Models/GeneralSettings.swift +++ b/Thaw/Settings/Models/GeneralSettings.swift @@ -71,6 +71,7 @@ final class GeneralSettings: ObservableObject { @Published var showOnScroll = Defaults.DefaultValue.showOnScroll // The offset to apply to the menu bar item spacing and padding. + @Published var itemSpacingOffset = Defaults.DefaultValue.itemSpacingOffset /// A Boolean value that indicates whether the hidden section /// should automatically rehide. @@ -113,6 +114,9 @@ final class GeneralSettings: ObservableObject { Defaults.ifPresent(key: .showOnDoubleClick, assign: &showOnDoubleClick) Defaults.ifPresent(key: .showOnHover, assign: &showOnHover) Defaults.ifPresent(key: .showOnScroll, assign: &showOnScroll) + if let rawSpacing = Defaults.object(forKey: .itemSpacingOffset) as? NSNumber { + itemSpacingOffset = Self.clampItemSpacingOffset(rawSpacing.doubleValue) + } Defaults.ifPresent(key: .autoRehide, assign: &autoRehide) Defaults.ifPresent(key: .rehideInterval, assign: &rehideInterval) @@ -174,6 +178,22 @@ final class GeneralSettings: ObservableObject { $showOnDoubleClick.persistToDefaults(key: .showOnDoubleClick, in: &c) $showOnHover.persistToDefaults(key: .showOnHover, in: &c) $showOnScroll.persistToDefaults(key: .showOnScroll, in: &c) + + // itemSpacingOffset has side effect on appState - keep manual + $itemSpacingOffset + .removeDuplicates() + .receive(on: DispatchQueue.main) + .sink { [weak appState, weak self] offset in + let clamped = Self.clampItemSpacingOffset(offset) + if clamped != offset { + self?.diagLog.warning("itemSpacingOffset \(offset) clamped to \(clamped)") + self?.itemSpacingOffset = clamped + } + Defaults.set(clamped, forKey: .itemSpacingOffset) + appState?.spacingManager.offset = Int(clamped.rounded()) + } + .store(in: &c) + $autoRehide.persistToDefaults(key: .autoRehide, in: &c) $rehideStrategy.persistToDefaults(key: .rehideStrategy, transform: \.rawValue, in: &c) $rehideInterval.persistToDefaults(key: .rehideInterval, in: &c) @@ -190,6 +210,10 @@ final class GeneralSettings: ObservableObject { cancellables = c } + private static func clampItemSpacingOffset(_ value: Double) -> Double { + max(-16, min(16, value)) + } + /// Handles settings changed externally via Settings URI scheme. private func handleExternalSettingsChange(_ notification: Notification) { guard let key = notification.userInfo?["key"] as? String else { diff --git a/Thaw/Settings/Models/Profile.swift b/Thaw/Settings/Models/Profile.swift index f4dacf06..08ccf6c4 100644 --- a/Thaw/Settings/Models/Profile.swift +++ b/Thaw/Settings/Models/Profile.swift @@ -38,10 +38,47 @@ struct GeneralSettingsSnapshot: Codable { var showOnDoubleClick: Bool var showOnHover: Bool var showOnScroll: Bool + var itemSpacingOffset: Double var autoRehide: Bool var rehideStrategyRawValue: Int var rehideInterval: TimeInterval + init( + showIceIcon: Bool, + iceIcon: ControlItemImageSet, + lastCustomIceIcon: ControlItemImageSet?, + customIceIconIsTemplate: Bool, + useIceBar: Bool, + useIceBarOnlyOnNotchedDisplay: Bool, + iceBarLocation: IceBarLocation, + iceBarLocationOnHotkey: Bool, + showOnClick: Bool, + showOnDoubleClick: Bool, + showOnHover: Bool, + showOnScroll: Bool, + itemSpacingOffset: Double = Defaults.DefaultValue.itemSpacingOffset, + autoRehide: Bool, + rehideStrategyRawValue: Int, + rehideInterval: TimeInterval + ) { + self.showIceIcon = showIceIcon + self.iceIcon = iceIcon + self.lastCustomIceIcon = lastCustomIceIcon + self.customIceIconIsTemplate = customIceIconIsTemplate + self.useIceBar = useIceBar + self.useIceBarOnlyOnNotchedDisplay = useIceBarOnlyOnNotchedDisplay + self.iceBarLocation = iceBarLocation + self.iceBarLocationOnHotkey = iceBarLocationOnHotkey + self.showOnClick = showOnClick + self.showOnDoubleClick = showOnDoubleClick + self.showOnHover = showOnHover + self.showOnScroll = showOnScroll + self.itemSpacingOffset = Self.clampItemSpacingOffset(itemSpacingOffset) + self.autoRehide = autoRehide + self.rehideStrategyRawValue = rehideStrategyRawValue + self.rehideInterval = rehideInterval + } + @MainActor static func capture(from settings: GeneralSettings) -> GeneralSettingsSnapshot { GeneralSettingsSnapshot( @@ -57,6 +94,7 @@ struct GeneralSettingsSnapshot: Codable { showOnDoubleClick: settings.showOnDoubleClick, showOnHover: settings.showOnHover, showOnScroll: settings.showOnScroll, + itemSpacingOffset: settings.itemSpacingOffset, autoRehide: settings.autoRehide, rehideStrategyRawValue: settings.rehideStrategy.rawValue, rehideInterval: settings.rehideInterval @@ -77,12 +115,59 @@ struct GeneralSettingsSnapshot: Codable { settings.showOnDoubleClick = showOnDoubleClick settings.showOnHover = showOnHover settings.showOnScroll = showOnScroll + settings.itemSpacingOffset = itemSpacingOffset settings.autoRehide = autoRehide if let strategy = RehideStrategy(rawValue: rehideStrategyRawValue) { settings.rehideStrategy = strategy } settings.rehideInterval = rehideInterval } + + private static func clampItemSpacingOffset(_ value: Double) -> Double { + Swift.max(-16, Swift.min(value, 16)) + } + + enum CodingKeys: String, CodingKey { + case showIceIcon + case iceIcon + case lastCustomIceIcon + case customIceIconIsTemplate + case useIceBar + case useIceBarOnlyOnNotchedDisplay + case iceBarLocation + case iceBarLocationOnHotkey + case showOnClick + case showOnDoubleClick + case showOnHover + case showOnScroll + case itemSpacingOffset + case autoRehide + case rehideStrategyRawValue + case rehideInterval + } + + init(from decoder: Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + self.init( + showIceIcon: try container.decode(Bool.self, forKey: .showIceIcon), + iceIcon: try container.decode(ControlItemImageSet.self, forKey: .iceIcon), + lastCustomIceIcon: try container.decodeIfPresent(ControlItemImageSet.self, forKey: .lastCustomIceIcon), + customIceIconIsTemplate: try container.decode(Bool.self, forKey: .customIceIconIsTemplate), + useIceBar: try container.decode(Bool.self, forKey: .useIceBar), + useIceBarOnlyOnNotchedDisplay: try container.decode(Bool.self, forKey: .useIceBarOnlyOnNotchedDisplay), + iceBarLocation: try container.decode(IceBarLocation.self, forKey: .iceBarLocation), + iceBarLocationOnHotkey: try container.decode(Bool.self, forKey: .iceBarLocationOnHotkey), + showOnClick: try container.decode(Bool.self, forKey: .showOnClick), + showOnDoubleClick: try container.decode(Bool.self, forKey: .showOnDoubleClick), + showOnHover: try container.decode(Bool.self, forKey: .showOnHover), + showOnScroll: try container.decode(Bool.self, forKey: .showOnScroll), + itemSpacingOffset: try container.decodeIfPresent(Double.self, forKey: .itemSpacingOffset) + ?? Defaults.DefaultValue.itemSpacingOffset, + autoRehide: try container.decode(Bool.self, forKey: .autoRehide), + rehideStrategyRawValue: try container.decode(Int.self, forKey: .rehideStrategyRawValue), + rehideInterval: try container.decode(TimeInterval.self, forKey: .rehideInterval) + ) + } } // MARK: - AdvancedSettingsSnapshot @@ -267,6 +352,7 @@ struct Profile: Codable, Identifiable { showOnDoubleClick: Defaults.DefaultValue.showOnDoubleClick, showOnHover: Defaults.DefaultValue.showOnHover, showOnScroll: Defaults.DefaultValue.showOnScroll, + itemSpacingOffset: Defaults.DefaultValue.itemSpacingOffset, autoRehide: Defaults.DefaultValue.autoRehide, rehideStrategyRawValue: Defaults.DefaultValue.rehideStrategy.rawValue, rehideInterval: Defaults.DefaultValue.rehideInterval diff --git a/Thaw/Settings/Models/ProfileManager.swift b/Thaw/Settings/Models/ProfileManager.swift index fff8d5cd..9cc47697 100644 --- a/Thaw/Settings/Models/ProfileManager.swift +++ b/Thaw/Settings/Models/ProfileManager.swift @@ -224,15 +224,16 @@ final class ProfileManager: ObservableObject { /// Applies a profile's settings to the running app state. /// - /// The menu bar item spacing offset is applied after the snapshot is - /// pushed, driving the per-profile spacing behaviour. The no-op guard - /// inside applyOffset skips the relaunch when the on-disk values + /// The menu bar item spacing offset is global, but profiles can still + /// restore it as part of their General settings snapshot. The no-op + /// guard inside applyOffset skips the relaunch when the on-disk values /// already match, so identical-offset switches cost nothing. func applyProfile(_ profile: Profile, to appState: AppState) { diagLog.debug( "applyProfile entered: name=\(profile.name)" ) profile.generalSettings.apply(to: appState.settings.general) + appState.spacingManager.offset = Int(profile.generalSettings.itemSpacingOffset.rounded()) profile.advancedSettings.apply(to: appState.settings.advanced) // Apply hotkeys @@ -731,14 +732,11 @@ final class ProfileManager: ObservableObject { /// Re-applies the currently active profile, driving its layout pass /// without changing which profile is active. /// - /// Used by DisplaySettingsManager.applyActiveDisplaySpacing after it - /// fires a relaunch wave whose menu bar items reattach at OS-default - /// positions: the auto-switch path doesn't fire when the active display - /// keeps the same associated profile, so without an explicit re-apply - /// the layout would never run and the items would stay where macOS put - /// them. The applyOffset inside layoutTask no-ops (the on-disk values - /// were just written), and the subsequent applyProfileLayout awaits - /// the in-flight expected-set settling before running. + /// Used after spacing-related relaunch waves whose menu bar items + /// reattach at OS-default positions. The applyOffset inside layoutTask + /// no-ops when the on-disk values were just written, and the subsequent + /// applyProfileLayout awaits the in-flight expected-set settling before + /// running. func reapplyActiveProfile() { guard let appState else { return } guard let activeID = activeProfileID else { return } diff --git a/Thaw/Settings/Models/SettingsResetter.swift b/Thaw/Settings/Models/SettingsResetter.swift index e7c991a0..dec410b7 100644 --- a/Thaw/Settings/Models/SettingsResetter.swift +++ b/Thaw/Settings/Models/SettingsResetter.swift @@ -40,6 +40,7 @@ extension AppSettings { general.showOnDoubleClick = Defaults.DefaultValue.showOnDoubleClick general.showOnHover = Defaults.DefaultValue.showOnHover general.showOnScroll = Defaults.DefaultValue.showOnScroll + general.itemSpacingOffset = Defaults.DefaultValue.itemSpacingOffset general.autoRehide = Defaults.DefaultValue.autoRehide general.rehideStrategy = Defaults.DefaultValue.rehideStrategy general.rehideInterval = Defaults.DefaultValue.rehideInterval diff --git a/Thaw/Settings/SettingsPanes/DisplaySettingsPane.swift b/Thaw/Settings/SettingsPanes/DisplaySettingsPane.swift index b232be7e..5e5c9d0b 100644 --- a/Thaw/Settings/SettingsPanes/DisplaySettingsPane.swift +++ b/Thaw/Settings/SettingsPanes/DisplaySettingsPane.swift @@ -12,11 +12,6 @@ struct DisplaySettingsPane: View { @ObservedObject var displaySettings: DisplaySettingsManager @State private var maxSliderLabelWidth: CGFloat = 0 - /// Per-display draft of the spacing slider, keyed by display UUID. - /// Until the user clicks Apply, dragging the slider only updates this - /// dictionary — it does not touch the saved configuration or trigger - /// any relaunches. - @State private var draftSpacing: [String: CGFloat] = [:] var body: some View { IceForm { @@ -172,67 +167,5 @@ struct DisplaySettingsPane: View { .annotation("Maximum number of items per row in the grid layout.") } } - - spacingRow(for: display) - } - - @ViewBuilder - private func spacingRow(for display: DisplaySettingsManager.DisplayInfo) -> some View { - let savedOffset = displaySettings.configuration(forUUID: display.id).itemSpacingOffset - let draft = draftSpacing[display.id] ?? CGFloat(savedOffset) - let canApply = draft != CGFloat(savedOffset) - - let sliderBinding = Binding( - get: { draftSpacing[display.id] ?? CGFloat(savedOffset) }, - set: { draftSpacing[display.id] = $0 } - ) - - let labelKey: LocalizedStringKey = switch draft { - case -16: "none" - case 0: "default" - case 16: "max" - default: LocalizedStringKey(draft.formatted()) - } - - LabeledContent { - IceSlider( - labelKey, - value: sliderBinding, - in: -16 ... 16, - step: 2 - ) - } label: { - LabeledContent { - Button("Apply") { - displaySettings.updateConfiguration(forDisplayUUID: display.id) { config in - config.withItemSpacingOffset(Double(draft)) - } - } - .help(Text("Apply the spacing for this display")) - .disabled(!canApply) - - Button { - draftSpacing[display.id] = 0 - displaySettings.updateConfiguration(forDisplayUUID: display.id) { config in - config.withItemSpacingOffset(0) - } - } label: { - Image(systemName: "arrow.counterclockwise.circle.fill") - } - .buttonStyle(.borderless) - .help(Text("Reset to the default spacing")) - .disabled(savedOffset == 0 && draft == 0) - } label: { - Text("Menu bar item spacing") - } - } - .annotation( - "Apply briefly relaunches apps with menu bar items so they pick up the new spacing. Setting takes effect when this display is the active menu bar display." - ) - .onChange(of: savedOffset) { _, newValue in - // Sync draft when the saved value changes externally - // (profile load, URI scheme, etc.). - draftSpacing[display.id] = CGFloat(newValue) - } } } diff --git a/Thaw/Settings/SettingsPanes/GeneralSettingsPane.swift b/Thaw/Settings/SettingsPanes/GeneralSettingsPane.swift index aa6d0dd7..d3cb8376 100644 --- a/Thaw/Settings/SettingsPanes/GeneralSettingsPane.swift +++ b/Thaw/Settings/SettingsPanes/GeneralSettingsPane.swift @@ -15,6 +15,17 @@ struct GeneralSettingsPane: View { @State private var isImportingCustomIceIcon = false @State private var isPresentingError = false @State private var presentedError: LocalizedErrorWrapper? + @State private var isApplyingItemSpacingOffset = false + @State private var tempItemSpacingOffset: CGFloat = 0 + + private var itemSpacingOffsetKey: LocalizedStringKey { + switch tempItemSpacingOffset { + case -16: "none" + case 0: "default" + case 16: "max" + default: LocalizedStringKey(tempItemSpacingOffset.formatted()) + } + } private var rehideIntervalKey: LocalizedStringKey { let count = Int(settings.rehideInterval) @@ -35,6 +46,9 @@ struct GeneralSettingsPane: View { IceSection { rehideOptions } + IceSection { + spacingOptions + } } } @@ -211,4 +225,78 @@ struct GeneralSettingsPane: View { } } } + + // MARK: Spacing Options + + private var spacingOptions: some View { + LabeledContent { + IceSlider( + itemSpacingOffsetKey, + value: $tempItemSpacingOffset, + in: -16 ... 16, + step: 2 + ) + .disabled(isApplyingItemSpacingOffset) + } label: { + LabeledContent { + Button("Apply") { + applyTempItemSpacingOffset() + } + .help(Text("Apply the current spacing")) + .disabled(isApplyingItemSpacingOffset || tempItemSpacingOffset == settings.itemSpacingOffset) + + if isApplyingItemSpacingOffset { + ProgressView() + .progressViewStyle(.circular) + .scaleEffect(0.5) + .frame(width: 15, height: 15) + } else { + Button { + tempItemSpacingOffset = 0 + applyTempItemSpacingOffset() + } label: { + Image(systemName: "arrow.counterclockwise.circle.fill") + } + .buttonStyle(.borderless) + .help(Text("Reset to the default spacing")) + .disabled(isApplyingItemSpacingOffset || settings.itemSpacingOffset == 0) + } + } label: { + Text("Menu bar item spacing") + } + } + .annotation( + "Applying this setting will relaunch all apps with menu bar items. Some apps may need to be manually relaunched.", + spacing: 2 + ) + .annotation(spacing: 10) { + CalloutBox( + "Note: You may need to log out and back in for this setting to apply properly.", + systemImage: "exclamationmark.circle" + ) + } + .onAppear { + tempItemSpacingOffset = settings.itemSpacingOffset + } + .onChange(of: settings.itemSpacingOffset) { _, newValue in + tempItemSpacingOffset = newValue + } + } + + private func applyTempItemSpacingOffset() { + isApplyingItemSpacingOffset = true + let previousOffset = settings.itemSpacingOffset + settings.itemSpacingOffset = tempItemSpacingOffset + Task { + do { + try await appState.spacingManager.applyOffset() + } catch { + settings.itemSpacingOffset = previousOffset + tempItemSpacingOffset = previousOffset + let alert = NSAlert(error: error) + alert.runModal() + } + isApplyingItemSpacingOffset = false + } + } } diff --git a/Thaw/Utilities/Defaults.swift b/Thaw/Utilities/Defaults.swift index b1165b71..1a10da31 100644 --- a/Thaw/Utilities/Defaults.swift +++ b/Thaw/Utilities/Defaults.swift @@ -154,6 +154,7 @@ extension Defaults { static let showOnDoubleClick = true static let showOnHover = false static let showOnScroll = true + static let itemSpacingOffset: Double = 0 static let autoRehide = true static let rehideStrategy: RehideStrategy = .smart static let rehideInterval: TimeInterval = 15 @@ -218,6 +219,7 @@ extension Defaults { case autoRehide = "AutoRehide" case rehideStrategy = "RehideStrategy" case rehideInterval = "RehideInterval" + case itemSpacingOffset = "ItemSpacingOffset" case displayIceBarConfigurations = "DisplayIceBarConfigurations" case knownDisplays = "KnownDisplays" diff --git a/ThawTests/DisplayIceBarConfigurationTests.swift b/ThawTests/DisplayIceBarConfigurationTests.swift index 6391ff9f..c04b63e5 100644 --- a/ThawTests/DisplayIceBarConfigurationTests.swift +++ b/ThawTests/DisplayIceBarConfigurationTests.swift @@ -20,7 +20,6 @@ final class DisplayIceBarConfigurationTests: XCTestCase { XCTAssertFalse(config.alwaysShowHiddenItems) XCTAssertEqual(config.iceBarLayout, .horizontal) XCTAssertEqual(config.gridColumns, 4) - XCTAssertEqual(config.itemSpacingOffset, 0) } // MARK: - Initialization Tests @@ -31,8 +30,7 @@ final class DisplayIceBarConfigurationTests: XCTestCase { iceBarLocation: .mousePointer, alwaysShowHiddenItems: true, iceBarLayout: .grid, - gridColumns: 6, - itemSpacingOffset: 5.0 + gridColumns: 6 ) XCTAssertTrue(config.useIceBar) @@ -40,7 +38,6 @@ final class DisplayIceBarConfigurationTests: XCTestCase { XCTAssertTrue(config.alwaysShowHiddenItems) XCTAssertEqual(config.iceBarLayout, .grid) XCTAssertEqual(config.gridColumns, 6) - XCTAssertEqual(config.itemSpacingOffset, 5.0) } // MARK: - With Methods Tests @@ -148,52 +145,6 @@ final class DisplayIceBarConfigurationTests: XCTestCase { XCTAssertEqual(original.gridColumns, 4) } - func testWithItemSpacingOffset() { - let original = DisplayIceBarConfiguration.defaultConfiguration - let modified = original.withItemSpacingOffset(8) - - XCTAssertEqual(modified.itemSpacingOffset, 8) - XCTAssertEqual(modified.useIceBar, original.useIceBar) - XCTAssertEqual(modified.iceBarLocation, original.iceBarLocation) - XCTAssertEqual(modified.alwaysShowHiddenItems, original.alwaysShowHiddenItems) - XCTAssertEqual(modified.iceBarLayout, original.iceBarLayout) - XCTAssertEqual(modified.gridColumns, original.gridColumns) - } - - func testWithItemSpacingOffsetNegative() { - let modified = DisplayIceBarConfiguration.defaultConfiguration - .withItemSpacingOffset(-10) - - XCTAssertEqual(modified.itemSpacingOffset, -10) - } - - func testWithItemSpacingOffsetFractional() { - let modified = DisplayIceBarConfiguration.defaultConfiguration - .withItemSpacingOffset(2.5) - - XCTAssertEqual(modified.itemSpacingOffset, 2.5, accuracy: 0.001) - } - - func testWithItemSpacingOffsetClamping() { - let original = DisplayIceBarConfiguration.defaultConfiguration - - let tooLow = original.withItemSpacingOffset(-100) - XCTAssertEqual(tooLow.itemSpacingOffset, -16) - - let tooHigh = original.withItemSpacingOffset(100) - XCTAssertEqual(tooHigh.itemSpacingOffset, 16) - - let inRange = original.withItemSpacingOffset(7.5) - XCTAssertEqual(inRange.itemSpacingOffset, 7.5, accuracy: 0.001) - } - - func testWithItemSpacingOffsetDoesNotMutateOriginal() { - let original = DisplayIceBarConfiguration.defaultConfiguration - _ = original.withItemSpacingOffset(10) - - XCTAssertEqual(original.itemSpacingOffset, 0) - } - // MARK: - Chained With Methods func testChainedWithMethods() { @@ -203,14 +154,12 @@ final class DisplayIceBarConfigurationTests: XCTestCase { .withAlwaysShowHiddenItems(true) .withIceBarLayout(.grid) .withGridColumns(5) - .withItemSpacingOffset(-3) XCTAssertTrue(config.useIceBar) XCTAssertEqual(config.iceBarLocation, .iceIcon) XCTAssertTrue(config.alwaysShowHiddenItems) XCTAssertEqual(config.iceBarLayout, .grid) XCTAssertEqual(config.gridColumns, 5) - XCTAssertEqual(config.itemSpacingOffset, -3) } // MARK: - Equatable Tests @@ -221,16 +170,14 @@ final class DisplayIceBarConfigurationTests: XCTestCase { iceBarLocation: .mousePointer, alwaysShowHiddenItems: false, iceBarLayout: .vertical, - gridColumns: 3, - itemSpacingOffset: -4 + gridColumns: 3 ) let config2 = DisplayIceBarConfiguration( useIceBar: true, iceBarLocation: .mousePointer, alwaysShowHiddenItems: false, iceBarLayout: .vertical, - gridColumns: 3, - itemSpacingOffset: -4 + gridColumns: 3 ) XCTAssertEqual(config1, config2) @@ -271,13 +218,6 @@ final class DisplayIceBarConfigurationTests: XCTestCase { XCTAssertNotEqual(config1, config2) } - func testEquatableDifferentItemSpacingOffset() { - let config1 = DisplayIceBarConfiguration.defaultConfiguration - let config2 = config1.withItemSpacingOffset(5) - - XCTAssertNotEqual(config1, config2) - } - // MARK: - Codable Tests func testEncodeDecode() throws { @@ -286,8 +226,7 @@ final class DisplayIceBarConfigurationTests: XCTestCase { iceBarLocation: .iceIcon, alwaysShowHiddenItems: true, iceBarLayout: .grid, - gridColumns: 6, - itemSpacingOffset: 2.5 + gridColumns: 6 ) let encoder = JSONEncoder() @@ -349,7 +288,6 @@ final class DisplayIceBarConfigurationTests: XCTestCase { XCTAssertFalse(decoded.alwaysShowHiddenItems) XCTAssertEqual(decoded.iceBarLayout, .horizontal) XCTAssertEqual(decoded.gridColumns, 4) - XCTAssertEqual(decoded.itemSpacingOffset, 0) } func testDecodeOldJSONWithInvalidGridColumns() throws { @@ -369,7 +307,7 @@ final class DisplayIceBarConfigurationTests: XCTestCase { XCTAssertEqual(decoded.gridColumns, 10) } - func testDecodeJSONWithItemSpacingOffset() throws { + func testDecodeLegacyJSONWithItemSpacingOffsetIgnoresField() throws { let json = """ { "useIceBar": false, @@ -384,25 +322,8 @@ final class DisplayIceBarConfigurationTests: XCTestCase { let decoder = JSONDecoder() let decoded = try decoder.decode(DisplayIceBarConfiguration.self, from: json) - XCTAssertEqual(decoded.itemSpacingOffset, -7.5, accuracy: 0.001) - } - - func testDecodeJSONClampsOutOfRangeItemSpacingOffset() throws { - let json = """ - { - "useIceBar": false, - "iceBarLocation": 0, - "alwaysShowHiddenItems": false, - "iceBarLayout": 1, - "gridColumns": 4, - "itemSpacingOffset": 99 - } - """.data(using: .utf8)! - - let decoder = JSONDecoder() - let decoded = try decoder.decode(DisplayIceBarConfiguration.self, from: json) - - XCTAssertEqual(decoded.itemSpacingOffset, 16) + XCTAssertFalse(decoded.useIceBar) + XCTAssertEqual(decoded.gridColumns, 4) } // MARK: - All Locations Tests diff --git a/ThawTests/GeneralSettingsSnapshotTests.swift b/ThawTests/GeneralSettingsSnapshotTests.swift index e21e381e..e8f3b8a9 100644 --- a/ThawTests/GeneralSettingsSnapshotTests.swift +++ b/ThawTests/GeneralSettingsSnapshotTests.swift @@ -41,6 +41,7 @@ final class GeneralSettingsSnapshotTests: XCTestCase { showOnDoubleClick: false, showOnHover: false, showOnScroll: false, + itemSpacingOffset: 0, autoRehide: true, rehideStrategyRawValue: 0, rehideInterval: 15 @@ -65,6 +66,7 @@ final class GeneralSettingsSnapshotTests: XCTestCase { showOnDoubleClick: true, showOnHover: true, showOnScroll: true, + itemSpacingOffset: -8, autoRehide: false, rehideStrategyRawValue: 2, rehideInterval: 30 @@ -87,6 +89,7 @@ final class GeneralSettingsSnapshotTests: XCTestCase { XCTAssertFalse(snapshot.showOnDoubleClick) XCTAssertFalse(snapshot.showOnHover) XCTAssertFalse(snapshot.showOnScroll) + XCTAssertEqual(snapshot.itemSpacingOffset, 0) XCTAssertTrue(snapshot.autoRehide) XCTAssertEqual(snapshot.rehideStrategyRawValue, 0) XCTAssertEqual(snapshot.rehideInterval, 15) @@ -106,6 +109,7 @@ final class GeneralSettingsSnapshotTests: XCTestCase { XCTAssertTrue(snapshot.showOnDoubleClick) XCTAssertTrue(snapshot.showOnHover) XCTAssertTrue(snapshot.showOnScroll) + XCTAssertEqual(snapshot.itemSpacingOffset, -8) XCTAssertFalse(snapshot.autoRehide) XCTAssertEqual(snapshot.rehideStrategyRawValue, 2) XCTAssertEqual(snapshot.rehideInterval, 30) @@ -124,6 +128,7 @@ final class GeneralSettingsSnapshotTests: XCTestCase { XCTAssertEqual(decoded.useIceBar, original.useIceBar) XCTAssertEqual(decoded.iceBarLocation, original.iceBarLocation) XCTAssertEqual(decoded.showOnClick, original.showOnClick) + XCTAssertEqual(decoded.itemSpacingOffset, original.itemSpacingOffset) XCTAssertEqual(decoded.autoRehide, original.autoRehide) XCTAssertEqual(decoded.rehideStrategyRawValue, original.rehideStrategyRawValue) XCTAssertEqual(decoded.rehideInterval, original.rehideInterval) @@ -145,6 +150,7 @@ final class GeneralSettingsSnapshotTests: XCTestCase { XCTAssertEqual(decoded.showOnDoubleClick, true) XCTAssertEqual(decoded.showOnHover, true) XCTAssertEqual(decoded.showOnScroll, true) + XCTAssertEqual(decoded.itemSpacingOffset, -8) XCTAssertEqual(decoded.autoRehide, false) XCTAssertEqual(decoded.rehideStrategyRawValue, 2) XCTAssertEqual(decoded.rehideInterval, 30) @@ -170,6 +176,28 @@ final class GeneralSettingsSnapshotTests: XCTestCase { XCTAssertNotNil(decoded.lastCustomIceIcon) } + func testDecodeWithoutItemSpacingOffsetUsesDefault() throws { + let data = try encoder.encode(makeDefaultSnapshot()) + var json = try XCTUnwrap(JSONSerialization.jsonObject(with: data) as? [String: Any]) + json.removeValue(forKey: "itemSpacingOffset") + + let legacyData = try JSONSerialization.data(withJSONObject: json) + let decoded = try decoder.decode(GeneralSettingsSnapshot.self, from: legacyData) + + XCTAssertEqual(decoded.itemSpacingOffset, Defaults.DefaultValue.itemSpacingOffset) + } + + func testDecodeClampsItemSpacingOffset() throws { + let data = try encoder.encode(makeDefaultSnapshot()) + var json = try XCTUnwrap(JSONSerialization.jsonObject(with: data) as? [String: Any]) + json["itemSpacingOffset"] = 99 + + let legacyData = try JSONSerialization.data(withJSONObject: json) + let decoded = try decoder.decode(GeneralSettingsSnapshot.self, from: legacyData) + + XCTAssertEqual(decoded.itemSpacingOffset, 16) + } + // MARK: - IceBarLocation Tests func testAllIceBarLocations() throws {