From 340062e6dd66bddc971d29019ca6c5a2dd2f55dc Mon Sep 17 00:00:00 2001 From: Kyle Van Essen Date: Wed, 2 Sep 2026 20:56:19 -0700 Subject: [PATCH] Redesign region editing in Settings --- Where/WhereUI/README.md | 7 +- .../RegionsSettingsViewSnapshotTests.swift | 10 ++ .../regionsSettings.Overview_iPad.png | 3 + ...nsSettings.Overview_iPad_accessibility.png | 3 + .../regionsSettings.Overview_iPad_ax5.png | 3 + ...regionsSettings.Overview_iPad_contrast.png | 3 + .../regionsSettings.Overview_iPad_dark.png | 3 + .../regionsSettings.Overview_iPhone.png | 3 + ...Settings.Overview_iPhone_accessibility.png | 3 + .../regionsSettings.Overview_iPhone_ax5.png | 3 + ...gionsSettings.Overview_iPhone_contrast.png | 3 + .../regionsSettings.Overview_iPhone_dark.png | 3 + .../Sources/Regions/RegionsSettingsView.swift | 155 ++++++++++++++---- .../Sources/Resources/Localizable.xcstrings | 22 +++ .../PrimaryRegionSelectionModelTests.swift | 21 +++ 15 files changed, 206 insertions(+), 39 deletions(-) create mode 100644 Where/WhereUI/SnapshotTests/RegionsSettingsViewSnapshotTests.swift create mode 100644 Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPad.png create mode 100644 Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPad_accessibility.png create mode 100644 Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPad_ax5.png create mode 100644 Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPad_contrast.png create mode 100644 Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPad_dark.png create mode 100644 Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPhone.png create mode 100644 Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPhone_accessibility.png create mode 100644 Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPhone_ax5.png create mode 100644 Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPhone_contrast.png create mode 100644 Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPhone_dark.png diff --git a/Where/WhereUI/README.md b/Where/WhereUI/README.md index 844325690..277174d16 100644 --- a/Where/WhereUI/README.md +++ b/Where/WhereUI/README.md @@ -158,9 +158,10 @@ the feature [`Where/AGENTS.md`](../AGENTS.md) and this module's drain an obsolete outbox. `OnboardingImportRecoveryModel` owns that reconciliation rather than the process-wide `WhereModel`. Settings offers export only. - **`RegionPickerView` / `RegionCustomizeView`** β€” the shared primary-region - picker (segmented map/list) and per-region color/emoji/icon customization, - backed by `PrimaryRegionSelectionModel`. Reused by onboarding and the Settings - `RegionsSettingsView` editor. + picker (segmented map/list) and the stepped color/emoji/icon editor for onboarding. + `PrimaryRegionSelectionModel` contains their shared state. The `RegionsSettingsView` + screen opens on selected regions and edits one appearance at a time. It uses the shared + picker for membership changes. - **`DevicesSettingsView`** β€” Settings’ installation rows for local recording choice, synced nicknames, advisory activity/permission status, and irreversible removal. Only the current row can toggle recording. Remote rows can be renamed or removed while preserving their earlier diff --git a/Where/WhereUI/SnapshotTests/RegionsSettingsViewSnapshotTests.swift b/Where/WhereUI/SnapshotTests/RegionsSettingsViewSnapshotTests.swift new file mode 100644 index 000000000..07e933e08 --- /dev/null +++ b/Where/WhereUI/SnapshotTests/RegionsSettingsViewSnapshotTests.swift @@ -0,0 +1,10 @@ +import SnapshotKitTesting +import Testing +@testable import WhereUI + +@MainActor +struct RegionsSettingsViewSnapshotTests { + @Test func regionsSettings() async { + await assertSnapshots(of: RegionsSettingsView.self) + } +} diff --git a/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPad.png b/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPad.png new file mode 100644 index 000000000..6e7980826 --- /dev/null +++ b/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPad.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:5aafe9181d0c73192ebc5aeb8057116bec7eb7d2a4af564ade5336512e05a8f0 +size 294385 diff --git a/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPad_accessibility.png b/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPad_accessibility.png new file mode 100644 index 000000000..bd41b72e1 --- /dev/null +++ b/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPad_accessibility.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:c6609ace963a22ade919c8857f833222a265027612b95d4a94e6e7978e9c3995 +size 434575 diff --git a/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPad_ax5.png b/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPad_ax5.png new file mode 100644 index 000000000..672a4efe4 --- /dev/null +++ b/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPad_ax5.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:62603bf8626d40c60a437aa7b8eb82d7ba3cb038a12aad13087ca3b58d37ad75 +size 419872 diff --git a/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPad_contrast.png b/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPad_contrast.png new file mode 100644 index 000000000..c2a7731c6 --- /dev/null +++ b/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPad_contrast.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:061bdda9756d5992c5b8e1e91c122fed4361ccebcf4f07b2a353105458db300d +size 280222 diff --git a/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPad_dark.png b/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPad_dark.png new file mode 100644 index 000000000..83bf2dfdc --- /dev/null +++ b/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPad_dark.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:1955e79422e0997ae395c43adc5bb06541827d30db6aadfdffca6760e9b4fabe +size 276389 diff --git a/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPhone.png b/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPhone.png new file mode 100644 index 000000000..18d3d265b --- /dev/null +++ b/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPhone.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:cf05086a4eb28643159cc47f199ade622a696750d2b1be02f6ebb0f354d4457d +size 173302 diff --git a/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPhone_accessibility.png b/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPhone_accessibility.png new file mode 100644 index 000000000..d2a29c0c2 --- /dev/null +++ b/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPhone_accessibility.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:be6014d8e88d30ce70cb46230df576bf0ce8428c1abaa75bd9ef0c8a4e1fdbb2 +size 292316 diff --git a/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPhone_ax5.png b/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPhone_ax5.png new file mode 100644 index 000000000..ee3b64d52 --- /dev/null +++ b/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPhone_ax5.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:cacd885ba9a569243cd0b2206200a5c2a67c0d982dc5a5282c2467e7ac5ba66f +size 304732 diff --git a/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPhone_contrast.png b/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPhone_contrast.png new file mode 100644 index 000000000..82289489c --- /dev/null +++ b/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPhone_contrast.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:794cdfb78452aa9d935ccfa9daea4231a3c9dd914101d4949c74fd60d57a7680 +size 162038 diff --git a/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPhone_dark.png b/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPhone_dark.png new file mode 100644 index 000000000..0178ce3f7 --- /dev/null +++ b/Where/WhereUI/SnapshotTests/__Snapshots__/RegionsSettingsViewSnapshotTests/regionsSettings.Overview_iPhone_dark.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:e60926bd8803498239ee229ba01d42a26aa7ce917a15a55c9240fa9fd4febadf +size 156589 diff --git a/Where/WhereUI/Sources/Regions/RegionsSettingsView.swift b/Where/WhereUI/Sources/Regions/RegionsSettingsView.swift index a674bb62e..5fde424fa 100644 --- a/Where/WhereUI/Sources/Regions/RegionsSettingsView.swift +++ b/Where/WhereUI/Sources/Regions/RegionsSettingsView.swift @@ -1,12 +1,13 @@ import PeriscopeCore import RegionKit +import SFSafeSymbols +import SnapshotKit import SwiftUI import WhereCore -/// Settings screen for editing your primary regions after onboarding: reuses -/// the same picker and per-region customization the first run uses. Loads the -/// current picks, lets you add/remove (up to the cap) and re-style each, and -/// commits on Save. +/// Settings screen for editing primary regions after onboarding. It opens on +/// the current regions, lets one region's appearance be edited at a time, and +/// keeps add/remove work behind a separate route to the shared picker. struct RegionsSettingsView: View { /// Regions with days in the selected year, so the picker can surface a /// "used this year" group. Passed by `SettingsView` from the report. @@ -15,29 +16,31 @@ struct RegionsSettingsView: View { @Environment(WhereSession.self) private var session @Environment(\.dismiss) private var dismiss - /// The picker/customization model, built once the current picks load. + /// The shared membership and appearance draft, built once the current picks load. @State private var model: PrimaryRegionSelectionModel? - @State private var phase: Phase = .pick @State private var isSaving = false + @State private var saveError = SaveErrorAlertState() - private enum Phase: Hashable { - case pick - case customize + private enum Destination: Hashable { + case appearance(Region) + case manage } private static let logger = WhereLog.session(RegionsSettingsViewLog.self) var body: some View { + @Bindable var saveError = saveError + // Presented as a sheet from Settings, so it owns its navigation stack and // explicit Cancel/Done points β€” making the commit boundary clear. NavigationStack { Group { if let model { - content(model) + overview(model) } else { ProgressView() .frame(maxWidth: .infinity, maxHeight: .infinity) - .navigationTitle(String(localized: .regionsManageTitle)) + .navigationTitle(String(localized: .settingsRegionsSection)) .toolbar { ToolbarItem(placement: .cancellationAction) { Button(String(localized: .commonCancel)) { dismiss() } @@ -45,36 +48,103 @@ struct RegionsSettingsView: View { } } } + .navigationDestination(for: Destination.self) { destination in + if let model { + destinationView(destination, model: model) + } + } } .task { await loadIfNeeded() } + .interactiveDismissDisabled(isSaving) + .alert( + String(localized: .settingsRegionsSaveErrorTitle), + isPresented: $saveError.isPresented, + ) { + Button(String(localized: .commonOk), role: .cancel) {} + } message: { + if let message = saveError.message { + Text(message) + } + } // Log View Mode: reveal an inspect badge for the region-editor events. A // no-op in release. .debugLogInspectable(WhereLog.session(RegionsSettingsViewLog.self)) } - @ViewBuilder - private func content(_ model: PrimaryRegionSelectionModel) -> some View { - switch phase { - case .pick: - RegionPickerView(model: model) - .navigationTitle(String(localized: .regionsManageTitle)) - .toolbar { - ToolbarItem(placement: .cancellationAction) { - Button(String(localized: .commonCancel)) { dismiss() } - } - ToolbarItem(placement: .confirmationAction) { - Button(String(localized: .onboardingNext)) { phase = .customize } - .disabled(!model.hasSelection) + private func overview(_ model: PrimaryRegionSelectionModel) -> some View { + List { + Section(String(localized: .regionGroupYours)) { + if model.selectedRegions.isEmpty { + Text(String(localized: .settingsRegionsEmpty)) + .foregroundStyle(.secondary) + } else { + ForEach(model.selectedRegions, id: \.self) { region in + NavigationLink(value: Destination.appearance(region)) { + regionRow(region, model: model) } } - case .customize: - // `RegionCustomizeView` supplies its own Back/Done toolbar; Back - // returns to the pick phase, Done saves. - RegionCustomizeView( - model: model, - onBack: { phase = .pick }, - onFinish: { save(model) }, - ) + } + } + + Section { + NavigationLink(value: Destination.manage) { + Label( + String(localized: .settingsRegionsManage), + systemSymbol: .map, + ) + } + } + } + .disabled(isSaving) + .navigationTitle(String(localized: .settingsRegionsSection)) + .toolbar { + ToolbarItem(placement: .cancellationAction) { + Button(String(localized: .commonCancel)) { dismiss() } + .disabled(isSaving) + } + ToolbarItem(placement: .confirmationAction) { + if isSaving { + ProgressView() + .accessibilityLabel(String(localized: .commonSave)) + } else { + Button(String(localized: .commonDone)) { save(model) } + .disabled(!model.hasSelection) + } + } + } + } + + private func regionRow( + _ region: Region, + model: PrimaryRegionSelectionModel, + ) -> some View { + let appearance = model.appearance(for: region) + return HStack { + Text(appearance.emoji) + .accessibilityHidden(true) + Text(region.localizedName) + Spacer(minLength: 0) + Image(systemSymbol: appearance.symbolName.sfSymbol) + .foregroundStyle(appearance.color.color) + .accessibilityHidden(true) + } + .accessibilityElement(children: .combine) + } + + @ViewBuilder + private func destinationView( + _ destination: Destination, + model: PrimaryRegionSelectionModel, + ) -> some View { + switch destination { + case let .appearance(region): + RegionAppearanceEditor(model: model, region: region) + .navigationTitle(region.localizedName) + .navigationBarTitleDisplayMode(.inline) + case .manage: + RegionPickerView(model: model) + .navigationTitle(String(localized: .settingsRegionsManage)) + .navigationBarTitleDisplayMode(.inline) } } @@ -98,16 +168,19 @@ struct RegionsSettingsView: View { private func save(_ model: PrimaryRegionSelectionModel) { guard !isSaving else { return } + saveError.message = nil isSaving = true Task { do { try await model.commit(using: session) + dismiss() } catch { Self.logger(attachments: [.error(error, name: "save-error")]) { .primaryRegionsSaveFailed(description: error.localizedDescription) } + saveError.message = error.localizedDescription + isSaving = false } - dismiss() } } } @@ -135,10 +208,20 @@ extension RegionsSettingsView: SettingsSection { } #if DEBUG + extension RegionsSettingsView: SnapshotProviding { + static var snapshots: [SnapshotCase] { + whereSnapshot( + name: "Overview", + configurations: .fullContentScreenDefaults, + ) { + RegionsSettingsView() + .environment(PreviewSupport.loadedSession()) + } + } + } + #Preview { - RegionsSettingsView() - .environment(PreviewSupport.loadedSession()) - .whereBroadwayRoot() + RegionsSettingsView.snapshotPreviews } #endif diff --git a/Where/WhereUI/Sources/Resources/Localizable.xcstrings b/Where/WhereUI/Sources/Resources/Localizable.xcstrings index 6e4e0be3b..82a5c7079 100644 --- a/Where/WhereUI/Sources/Resources/Localizable.xcstrings +++ b/Where/WhereUI/Sources/Resources/Localizable.xcstrings @@ -8753,6 +8753,17 @@ } } }, + "settings.regions.manage" : { + "extractionState" : "manual", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "new", + "value" : "Manage Regions" + } + } + } + }, "settings.regions.row" : { "extractionState" : "manual", "localizations" : { @@ -8764,6 +8775,17 @@ } } }, + "settings.regions.saveError.title" : { + "extractionState" : "manual", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "new", + "value" : "Unable to Save Regions" + } + } + } + }, "settings.regions.section" : { "comment" : "Label for the settings option to manage your regions.", "extractionState" : "manual", diff --git a/Where/WhereUI/Tests/PrimaryRegionSelectionModelTests.swift b/Where/WhereUI/Tests/PrimaryRegionSelectionModelTests.swift index 40f5843fe..5f742d94a 100644 --- a/Where/WhereUI/Tests/PrimaryRegionSelectionModelTests.swift +++ b/Where/WhereUI/Tests/PrimaryRegionSelectionModelTests.swift @@ -128,6 +128,27 @@ struct PrimaryRegionSelectionModelTests { #expect(try await session.services.trackedRegions() == [.california]) } + @Test func editingOneAppearancePreservesOtherRegionsAndOrder() async throws { + let session = PreviewSupport.loadedSession() + let california = RegionAppearance(color: .orange, emoji: "🌴", symbolName: .sunMaxFill) + let newYork = RegionAppearance(color: .indigo, emoji: "πŸ—½", symbolName: .building2Fill) + try await session.services.setPrimaryRegions([ + PrimaryRegion(region: .california, appearance: california, order: 0), + PrimaryRegion(region: .newYork, appearance: newYork, order: 1), + ]) + + let existing = try await session.services.primaryRegions() + let model = PrimaryRegionSelectionModel(existing: existing) + model.setEmoji("πŸŒ‰", for: .california) + try await model.commit(using: session) + + let saved = try await session.services.primaryRegions() + #expect(saved.map(\.region) == [.california, .newYork]) + #expect(saved[0].appearance?.emoji == "πŸŒ‰") + #expect(saved[0].appearance?.color == california.color) + #expect(saved[1].appearance == newYork) + } + @Test func editingTheDefaultSetConvergesToUSOnly() async throws { // A fresh install has no stored rows, so `primaryRegions()` returns the // legacy default set (CA / NY / Canada / EU). Opening the editor drops