From d03791be1ecabb14c8ed848da9cab592f77a3e15 Mon Sep 17 00:00:00 2001 From: Prachi Gauriar Date: Sun, 29 Mar 2026 16:13:10 -0400 Subject: [PATCH] Require the consumer to provide a dismiss closure to the editor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rename ConfigVariableEditor.onSave to dismiss - If no dismiss is provided, default to dismissing the view - Remove requiresRelaunch metadata key, as we don’t want to encourage requiring relaunch vs. responding to the change live --- App/Sources/App/ContentView.swift | 3 +- CLAUDE.md | 2 +- .../ConfigVariableListView.swift | 8 +- .../ConfigVariableListViewModel.swift | 36 ++++++-- .../ConfigVariableListViewModeling.swift | 22 +++-- .../Editor/ConfigVariableEditor.swift | 39 +++++---- .../RequiresRelaunchMetadataKey.swift | 26 ------ .../Resources/Localizable.xcstrings | 10 --- .../ConfigVariableDetailViewModelTests.swift | 1 - .../ConfigVariableListViewModelTests.swift | 87 ++++++++++++++++--- .../RequiresRelaunchMetadataKeyTests.swift | 42 --------- 11 files changed, 149 insertions(+), 127 deletions(-) delete mode 100644 Sources/DevConfiguration/Metadata/RequiresRelaunchMetadataKey.swift delete mode 100644 Tests/DevConfigurationTests/Unit Tests/Metadata/RequiresRelaunchMetadataKeyTests.swift diff --git a/App/Sources/App/ContentView.swift b/App/Sources/App/ContentView.swift index 588cba9..17b6b43 100644 --- a/App/Sources/App/ContentView.swift +++ b/App/Sources/App/ContentView.swift @@ -33,8 +33,9 @@ struct ContentView: View { Button("Do something", role: .destructive) { print("Did something!") } - } onSave: { variables in + } dismiss: { variables in print(variables) + isPresentingConfigEditor = false } } } diff --git a/CLAUDE.md b/CLAUDE.md index 85232ae..1de11d5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -42,7 +42,7 @@ metadata. `ConfigVariableContent`, `CodableValueRepresentation`, `RegisteredConfigVariable`, and `ConfigVariableSecrecy` - **Sources/DevConfiguration/Metadata/**: `ConfigVariableMetadata` and metadata key types - (`DisplayNameMetadataKey`, `RequiresRelaunchMetadataKey`) + (`DisplayNameMetadataKey`) - **Sources/DevConfiguration/Access Reporting/**: EventBus-based access and decoding events ### Key Documents diff --git a/Sources/DevConfiguration/Editor/Config Variable List/ConfigVariableListView.swift b/Sources/DevConfiguration/Editor/Config Variable List/ConfigVariableListView.swift index 5e2c1b5..080039f 100644 --- a/Sources/DevConfiguration/Editor/Config Variable List/ConfigVariableListView.swift +++ b/Sources/DevConfiguration/Editor/Config Variable List/ConfigVariableListView.swift @@ -71,13 +71,12 @@ struct ConfigVariableListView Void + /// An optional closure to call when the editor is dismissed. + /// + /// It receives the registered variables whose overrides changed, or an empty array if the user dismissed without + /// saving. + private let dismiss: (([RegisteredConfigVariable]) -> Void)? var searchText = "" var isShowingSaveAlert = false @@ -32,10 +35,12 @@ final class ConfigVariableListViewModel: ConfigVariableListViewModeling { /// /// - Parameters: /// - document: The editor document. - /// - onSave: A closure called with the registered variables whose overrides changed when the user saves. - init(document: EditorDocument, onSave: @escaping ([RegisteredConfigVariable]) -> Void) { + /// - dismiss: An optional closure called when the editor is dismissed. It receives the registered variables whose + /// overrides changed, or an empty array if the user dismissed without saving. If `nil`, the view's environment + /// dismiss action is used instead. + init(document: EditorDocument, dismiss: (([RegisteredConfigVariable]) -> Void)?) { self.document = document - self.onSave = onSave + self.dismiss = dismiss } @@ -95,15 +100,30 @@ final class ConfigVariableListViewModel: ConfigVariableListViewModeling { if isDirty { isShowingSaveAlert = true } else { - dismiss() + dismissWithoutSaving(dismiss) } } - func save() { + func saveAndDismiss(_ dismiss: () -> Void) { let changedKeys = document.changedKeys document.save() - onSave(changedKeys.compactMap { document.registeredVariables[$0] }) + let changedVariables = changedKeys.compactMap { document.registeredVariables[$0] } + + if let dismiss = self.dismiss { + dismiss(changedVariables) + } else { + dismiss() + } + } + + + func dismissWithoutSaving(_ dismiss: () -> Void) { + if let dismiss = self.dismiss { + dismiss([]) + } else { + dismiss() + } } diff --git a/Sources/DevConfiguration/Editor/Config Variable List/ConfigVariableListViewModeling.swift b/Sources/DevConfiguration/Editor/Config Variable List/ConfigVariableListViewModeling.swift index fcbf612..86a03af 100644 --- a/Sources/DevConfiguration/Editor/Config Variable List/ConfigVariableListViewModeling.swift +++ b/Sources/DevConfiguration/Editor/Config Variable List/ConfigVariableListViewModeling.swift @@ -43,14 +43,26 @@ protocol ConfigVariableListViewModeling: Observable { /// Requests dismissal of the editor. /// - /// If the working copy has unsaved changes, this presents the save alert. Otherwise, it calls the dismiss closure - /// immediately. + /// If the working copy has unsaved changes, this presents the save alert. Otherwise, it dismisses without saving. /// - /// - Parameter dismiss: A closure that dismisses the editor view. + /// - Parameter dismiss: A closure that dismisses the editor view using the environment's dismiss action. func requestDismiss(_ dismiss: () -> Void) - /// Saves the working copy to the editor override provider. - func save() + /// Saves the working copy and dismisses the editor. + /// + /// This saves the document and calls the consumer's dismiss closure with the changed variables. If no consumer + /// dismiss closure was provided, it calls the provided `dismiss` closure instead. + /// + /// - Parameter dismiss: A closure that dismisses the editor view using the environment's dismiss action. + func saveAndDismiss(_ dismiss: () -> Void) + + /// Dismisses the editor without saving. + /// + /// This calls the consumer's dismiss closure with an empty array. If no consumer dismiss closure was provided, it + /// calls the provided `dismiss` closure instead. + /// + /// - Parameter dismiss: A closure that dismisses the editor view using the environment's dismiss action. + func dismissWithoutSaving(_ dismiss: () -> Void) /// Requests clearing all overrides by presenting the clear confirmation alert. func requestClearAllOverrides() diff --git a/Sources/DevConfiguration/Editor/ConfigVariableEditor.swift b/Sources/DevConfiguration/Editor/ConfigVariableEditor.swift index 6b9893c..8ba8e57 100644 --- a/Sources/DevConfiguration/Editor/ConfigVariableEditor.swift +++ b/Sources/DevConfiguration/Editor/ConfigVariableEditor.swift @@ -12,13 +12,14 @@ import SwiftUI /// A SwiftUI view that presents the configuration variable editor. /// /// `ConfigVariableEditor` is initialized with a ``ConfigVariableReader`` that has editor support enabled and an -/// `onSave` closure that receives the registered variables whose overrides changed. +/// optional `dismiss` closure that receives the registered variables whose overrides changed. /// -/// The consumer is responsible for presentation (sheet, full-screen cover, navigation push, etc.). +/// The consumer is responsible for presentation (sheet, full-screen cover, navigation push, etc.). If no `dismiss` +/// closure is provided, the editor uses the environment's dismiss action. /// /// .sheet(isPresented: $isEditorPresented) { /// ConfigVariableEditor(reader: reader) { changedVariables in -/// // Handle changed variables +/// // Handle changed variables and dismiss /// } /// } /// @@ -29,8 +30,8 @@ import SwiftUI /// customSectionTitle: "Actions" /// ) { /// Button("Reset All") { … } -/// } onSave: { changedVariables in -/// // Handle changed variables +/// } dismiss: { changedVariables in +/// // Handle changed variables and dismiss /// } public struct ConfigVariableEditor: View { /// The list view model created from the reader. @@ -50,18 +51,20 @@ public struct ConfigVariableEditor: View { /// `true`, the view is empty. /// - customSectionTitle: The title for the custom section. /// - customSection: A view builder that produces custom content to display in a section at the top of the list. - /// - onSave: A closure called with the registered variables whose overrides changed when the user saves. + /// - dismiss: An optional closure called when the editor is dismissed. It receives the registered variables whose + /// overrides changed, or an empty array if dismissed without saving. If `nil`, the environment's dismiss action + /// is used. public init( reader: ConfigVariableReader, customSectionTitle: LocalizedStringKey, @ViewBuilder customSection: () -> CustomSection, - onSave: @escaping ([RegisteredConfigVariable]) -> Void, + dismiss: (([RegisteredConfigVariable]) -> Void)? = nil, ) { self.init( reader: reader, customSectionTitle: Text(customSectionTitle), customSection: customSection, - onSave: onSave, + dismiss: dismiss, ) } @@ -73,16 +76,18 @@ public struct ConfigVariableEditor: View { /// `true`, the view is empty. /// - customSectionTitle: A `Text` view to use as the title for the custom section. /// - customSection: A view builder that produces custom content to display in a section at the top of the list. - /// - onSave: A closure called with the registered variables whose overrides changed when the user saves. + /// - dismiss: An optional closure called when the editor is dismissed. It receives the registered variables whose + /// overrides changed, or an empty array if dismissed without saving. If `nil`, the environment's dismiss action + /// is used. public init( reader: ConfigVariableReader, customSectionTitle: Text, @ViewBuilder customSection: () -> CustomSection, - onSave: @escaping ([RegisteredConfigVariable]) -> Void, + dismiss: (([RegisteredConfigVariable]) -> Void)? = nil, ) { self.customSectionTitle = customSectionTitle self.customSection = customSection() - self._viewModel = Self.makeViewModel(reader: reader, onSave: onSave) + self._viewModel = Self.makeViewModel(reader: reader, dismiss: dismiss) } @@ -99,7 +104,7 @@ public struct ConfigVariableEditor: View { private static func makeViewModel( reader: ConfigVariableReader, - onSave: @escaping ([RegisteredConfigVariable]) -> Void, + dismiss: (([RegisteredConfigVariable]) -> Void)?, ) -> State { guard let editorOverrideProvider = reader.editorOverrideProvider else { return State(initialValue: nil) @@ -117,7 +122,7 @@ public struct ConfigVariableEditor: View { undoManager: UndoManager(), ) - return State(initialValue: ConfigVariableListViewModel(document: document, onSave: onSave)) + return State(initialValue: ConfigVariableListViewModel(document: document, dismiss: dismiss)) } } @@ -128,14 +133,16 @@ extension ConfigVariableEditor where CustomSection == EmptyView { /// - Parameters: /// - reader: The configuration variable reader. If the reader was not created with `isEditorEnabled` set to /// `true`, the view is empty. - /// - onSave: A closure called with the registered variables whose overrides changed when the user saves. + /// - dismiss: An optional closure called when the editor is dismissed. It receives the registered variables whose + /// overrides changed, or an empty array if dismissed without saving. If `nil`, the environment's dismiss action + /// is used. public init( reader: ConfigVariableReader, - onSave: @escaping ([RegisteredConfigVariable]) -> Void, + dismiss: (([RegisteredConfigVariable]) -> Void)? = nil, ) { self.customSectionTitle = Text(verbatim: "") self.customSection = EmptyView() - self._viewModel = Self.makeViewModel(reader: reader, onSave: onSave) + self._viewModel = Self.makeViewModel(reader: reader, dismiss: dismiss) } } diff --git a/Sources/DevConfiguration/Metadata/RequiresRelaunchMetadataKey.swift b/Sources/DevConfiguration/Metadata/RequiresRelaunchMetadataKey.swift deleted file mode 100644 index 7f0bdfa..0000000 --- a/Sources/DevConfiguration/Metadata/RequiresRelaunchMetadataKey.swift +++ /dev/null @@ -1,26 +0,0 @@ -// -// RequiresRelaunchMetadataKey.swift -// DevConfiguration -// -// Created by Prachi Gauriar on 3/7/2026. -// - -import Foundation - -/// The metadata key indicating that changes to a variable require an app relaunch to take effect. -private struct RequiresRelaunchMetadataKey: ConfigVariableMetadataKey { - static let defaultValue = false - static let keyDisplayText = localizedString("requiresRelaunchMetadata.keyDisplayText") -} - - -extension ConfigVariableMetadata { - /// Whether changes to the configuration variable require an app relaunch to take effect. - /// - /// When `true`, the editor UI communicates this to consumers via the `onSave` closure so they can prompt the user - /// to relaunch the app. Defaults to `false`. - public var requiresRelaunch: Bool { - get { self[RequiresRelaunchMetadataKey.self] } - set { self[RequiresRelaunchMetadataKey.self] = newValue } - } -} diff --git a/Sources/DevConfiguration/Resources/Localizable.xcstrings b/Sources/DevConfiguration/Resources/Localizable.xcstrings index 9c241b1..9dadce1 100644 --- a/Sources/DevConfiguration/Resources/Localizable.xcstrings +++ b/Sources/DevConfiguration/Resources/Localizable.xcstrings @@ -370,16 +370,6 @@ } } } - }, - "requiresRelaunchMetadata.keyDisplayText" : { - "localizations" : { - "en" : { - "stringUnit" : { - "state" : "translated", - "value" : "Requires Relaunch" - } - } - } } }, "version" : "1.0" diff --git a/Tests/DevConfigurationTests/Unit Tests/Editor/Config Variable Detail/ConfigVariableDetailViewModelTests.swift b/Tests/DevConfigurationTests/Unit Tests/Editor/Config Variable Detail/ConfigVariableDetailViewModelTests.swift index 60f9950..4ce11ae 100644 --- a/Tests/DevConfigurationTests/Unit Tests/Editor/Config Variable Detail/ConfigVariableDetailViewModelTests.swift +++ b/Tests/DevConfigurationTests/Unit Tests/Editor/Config Variable Detail/ConfigVariableDetailViewModelTests.swift @@ -60,7 +60,6 @@ struct ConfigVariableDetailViewModelTests: RandomValueGenerating { // set up var metadata = ConfigVariableMetadata() metadata.displayName = randomAlphanumericString() - metadata.requiresRelaunch = randomBool() metadata.isEditable = randomBool() let destinationTypeName = randomAlphanumericString() let isSecret = randomBool() diff --git a/Tests/DevConfigurationTests/Unit Tests/Editor/Config Variable List/ConfigVariableListViewModelTests.swift b/Tests/DevConfigurationTests/Unit Tests/Editor/Config Variable List/ConfigVariableListViewModelTests.swift index 1b6a157..c59d5db 100644 --- a/Tests/DevConfigurationTests/Unit Tests/Editor/Config Variable List/ConfigVariableListViewModelTests.swift +++ b/Tests/DevConfigurationTests/Unit Tests/Editor/Config Variable List/ConfigVariableListViewModelTests.swift @@ -21,7 +21,7 @@ struct ConfigVariableListViewModelTests: RandomValueGenerating { var workingCopyDisplayName: String! let undoManager = UndoManager() - nonisolated(unsafe) var onSaveStub: Stub<[RegisteredConfigVariable], Void>! + nonisolated(unsafe) var dismissStub: Stub<[RegisteredConfigVariable], Void>! init() { @@ -29,7 +29,7 @@ struct ConfigVariableListViewModelTests: RandomValueGenerating { userDefaults.removeObject(forKey: "editorOverrides") editorOverrideProvider = EditorOverrideProvider(userDefaults: userDefaults) workingCopyDisplayName = randomAlphanumericString() - onSaveStub = Stub() + dismissStub = Stub() } @@ -50,7 +50,7 @@ struct ConfigVariableListViewModelTests: RandomValueGenerating { func makeViewModel(document: EditorDocument) -> ConfigVariableListViewModel { - ConfigVariableListViewModel(document: document, onSave: { self.onSaveStub($0) }) + ConfigVariableListViewModel(document: document, dismiss: { self.dismissStub($0) }) } @@ -303,14 +303,29 @@ struct ConfigVariableListViewModelTests: RandomValueGenerating { // MARK: - requestDismiss @Test - mutating func requestDismissCallsDismissWhenClean() async { + mutating func requestDismissCallsDismissClosureWhenClean() { // set up with no overrides let document = makeDocument() let viewModel = makeViewModel(document: document) // exercise - await confirmation { dismissed in - viewModel.requestDismiss { dismissed() } + viewModel.requestDismiss {} + + // expect the dismiss closure was called with an empty array and save alert is not showing + #expect(dismissStub.callArguments.map { $0.map(\.key) } == [[]]) + #expect(!viewModel.isShowingSaveAlert) + } + + + @Test + mutating func requestDismissCallsFallbackWhenCleanAndNoDismissClosure() async { + // set up with no overrides and no dismiss closure + let document = makeDocument() + let viewModel = ConfigVariableListViewModel(document: document, dismiss: nil) + + // exercise + await confirmation { fallbackDismissed in + viewModel.requestDismiss { fallbackDismissed() } } // expect save alert is not showing @@ -335,10 +350,10 @@ struct ConfigVariableListViewModelTests: RandomValueGenerating { } - // MARK: - save + // MARK: - saveAndDismiss @Test - mutating func saveCallsOnSaveWithChangedVariables() throws { + mutating func saveAndDismissCallsDismissClosureWithChangedVariables() async throws { // set up with an override let variable = randomRegisteredVariable(defaultContent: .string(randomAlphanumericString())) let document = makeDocument(registeredVariables: [variable]) @@ -347,15 +362,63 @@ struct ConfigVariableListViewModelTests: RandomValueGenerating { document.setOverride(.string(randomAlphanumericString()), forKey: variable.key) // exercise - viewModel.save() + viewModel.saveAndDismiss {} + + // expect dismiss closure was called with the changed variable and document is no longer dirty + let dismissedVariables = try #require(dismissStub.callArguments.first) + #expect(dismissedVariables.map(\.key) == [variable.key]) + #expect(!viewModel.isDirty) + } + + + @Test + mutating func saveAndDismissCallsFallbackWhenNoDismissClosure() async { + // set up with an override and no dismiss closure + let variable = randomRegisteredVariable(defaultContent: .string(randomAlphanumericString())) + let document = makeDocument(registeredVariables: [variable]) + let viewModel = ConfigVariableListViewModel(document: document, dismiss: nil) + + document.setOverride(.string(randomAlphanumericString()), forKey: variable.key) + + // exercise + await confirmation { fallbackDismissed in + viewModel.saveAndDismiss { fallbackDismissed() } + } - // expect onSave was called with the changed variable and document is no longer dirty - let savedVariables = try #require(onSaveStub.callArguments.first) - #expect(savedVariables.map(\.key) == [variable.key]) + // expect document is no longer dirty #expect(!viewModel.isDirty) } + // MARK: - dismissWithoutSaving + + @Test + mutating func dismissWithoutSavingCallsDismissClosureWithEmptyArray() { + // set up + let document = makeDocument() + let viewModel = makeViewModel(document: document) + + // exercise + viewModel.dismissWithoutSaving {} + + // expect dismiss closure was called with an empty array + #expect(dismissStub.callArguments.map { $0.map(\.key) } == [[]]) + } + + + @Test + mutating func dismissWithoutSavingCallsFallbackWhenNoDismissClosure() async { + // set up with no dismiss closure + let document = makeDocument() + let viewModel = ConfigVariableListViewModel(document: document, dismiss: nil) + + // exercise + await confirmation { fallbackDismissed in + viewModel.dismissWithoutSaving { fallbackDismissed() } + } + } + + // MARK: - requestClearAllOverrides @Test diff --git a/Tests/DevConfigurationTests/Unit Tests/Metadata/RequiresRelaunchMetadataKeyTests.swift b/Tests/DevConfigurationTests/Unit Tests/Metadata/RequiresRelaunchMetadataKeyTests.swift deleted file mode 100644 index 75c0a1f..0000000 --- a/Tests/DevConfigurationTests/Unit Tests/Metadata/RequiresRelaunchMetadataKeyTests.swift +++ /dev/null @@ -1,42 +0,0 @@ -// -// RequiresRelaunchMetadataKeyTests.swift -// DevConfiguration -// -// Created by Prachi Gauriar on 3/7/2026. -// - -import Testing - -@testable import DevConfiguration - -struct RequiresRelaunchMetadataKeyTests { - @Test - func requiresRelaunchDefaultsToFalseAndStoresAndRetrievesValue() { - // set up - var metadata = ConfigVariableMetadata() - - // expect that unset requiresRelaunch returns false - #expect(!metadata.requiresRelaunch) - - // exercise - metadata.requiresRelaunch = true - - // expect that the value is stored and retrieved correctly - #expect(metadata.requiresRelaunch) - } - - - @Test - func requiresRelaunchDisplayTextShowsValue() throws { - // set up - var metadata = ConfigVariableMetadata() - - // exercise - metadata.requiresRelaunch = true - - // expect that displayTextEntries contains the requires relaunch entry with a localized key - let entries = metadata.displayTextEntries - let entry = try #require(entries.first { $0.value == "true" }) - #expect(entry.key != "requiresRelaunchMetadata.keyDisplayText") - } -}