From 1ced521d40aa2d3f7db3535fa14d71ed9183f08f Mon Sep 17 00:00:00 2001 From: Andreas Bauer Date: Tue, 20 Feb 2024 14:38:36 -0800 Subject: [PATCH] Support dynamic type sizes and longer timeout for required account modifier (#51) # Support dynamic type sizes and longer timeout for required account modifier ## :recycle: Current situation & Problem Currently, the `accountRequired` modifier provides 500ms for account details to be available, before it brings up the Account Sheet to enforce an account login. This time is to shorts and currently makes the Account Sheet pop up after fresh app starts for a few milliseconds. We increase the timeout to be more lenient. Further, this PR addresses #50 by using the new `ListRow` and `DynamicHStack` views for all list row contents optimizing SpeziAccount for larger dynamic type sizes. ## :gear: Release Notes * Fixed an issue where the account required sheet was popping up to early. * Optimize UI for larger dynamic type sizes. ## :books: Documentation -- ## :white_check_mark: Testing -- ## :pencil: Code of Conduct & Contributing Guidelines By submitting creating this pull request, you agree to follow our [Code of Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md) and [Contributing Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md): - [x] I agree to follow the [Code of Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md) and [Contributing Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md). --- .github/workflows/build-and-test.yml | 1 + Package.swift | 4 +- .../AccountValue/Keys/DateOfBirthKey.swift | 3 +- .../AccountValue/Keys/PersonNameKey.swift | 2 +- .../AccountValue/Keys/UserIdKey.swift | 3 +- .../Resources/Localizable.xcstrings | 23 ------- .../AccountRequiredModifier.swift | 2 +- .../LocalizableStringBasedDisplayView.swift | 7 ++- .../Views/DataDisplay/SimpleTextRow.swift | 43 ------------- .../DataDisplay/StringBasedDisplayView.swift | 7 ++- .../Views/Fields/DateOfBirthPicker.swift | 61 ++++++++++++------- .../Views/Fields/GenderIdentityPicker.swift | 14 ++--- Sources/SpeziAccount/Views/SignupForm.swift | 2 +- .../AccountTests/AccountTestsView.swift | 10 ++- .../AccountTests/TestAlertModifier.swift | 2 +- .../UITests/UITests.xcodeproj/project.pbxproj | 7 ++- .../xcshareddata/xcschemes/TestApp.xcscheme | 1 + 17 files changed, 77 insertions(+), 115 deletions(-) delete mode 100644 Sources/SpeziAccount/Views/DataDisplay/SimpleTextRow.swift diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 365e9889..68240fa4 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -31,6 +31,7 @@ jobs: runsonlabels: '["macOS", "self-hosted"]' path: 'Tests/UITests' scheme: TestApp + setupSimulators: true uploadcoveragereport: name: Upload Coverage Report needs: [buildandtest, buildandtestuitests] diff --git a/Package.swift b/Package.swift index 5e4e2e6e..d77bfd7d 100644 --- a/Package.swift +++ b/Package.swift @@ -22,8 +22,8 @@ let package = Package( ], dependencies: [ .package(url: "https://github.com/StanfordSpezi/SpeziFoundation.git", from: "1.0.0"), - .package(url: "https://github.com/StanfordSpezi/Spezi", from: "1.0.0"), - .package(url: "https://github.com/StanfordSpezi/SpeziViews", from: "1.0.0"), + .package(url: "https://github.com/StanfordSpezi/Spezi", from: "1.2.0"), + .package(url: "https://github.com/StanfordSpezi/SpeziViews", from: "1.3.0"), .package(url: "https://github.com/StanfordBDHG/XCTRuntimeAssertions", from: "1.0.0"), .package(url: "https://github.com/apple/swift-collections.git", from: "1.0.4") ], diff --git a/Sources/SpeziAccount/AccountValue/Keys/DateOfBirthKey.swift b/Sources/SpeziAccount/AccountValue/Keys/DateOfBirthKey.swift index ea2b17a7..e82b0541 100644 --- a/Sources/SpeziAccount/AccountValue/Keys/DateOfBirthKey.swift +++ b/Sources/SpeziAccount/AccountValue/Keys/DateOfBirthKey.swift @@ -6,6 +6,7 @@ // SPDX-License-Identifier: MIT // +import SpeziViews import SwiftUI @@ -58,7 +59,7 @@ extension DateOfBirthKey { } public var body: some View { - SimpleTextRow(name: Key.name) { + ListRow(Key.name) { Text(value.formatted(formatStyle)) } } diff --git a/Sources/SpeziAccount/AccountValue/Keys/PersonNameKey.swift b/Sources/SpeziAccount/AccountValue/Keys/PersonNameKey.swift index c7bd24fd..2a011a20 100644 --- a/Sources/SpeziAccount/AccountValue/Keys/PersonNameKey.swift +++ b/Sources/SpeziAccount/AccountValue/Keys/PersonNameKey.swift @@ -49,7 +49,7 @@ extension PersonNameKey { private let value: PersonNameComponents public var body: some View { - SimpleTextRow(name: Key.name) { + ListRow(Key.name) { Text(value.formatted(.name(style: .long))) } } diff --git a/Sources/SpeziAccount/AccountValue/Keys/UserIdKey.swift b/Sources/SpeziAccount/AccountValue/Keys/UserIdKey.swift index 6c1a8c27..1e447fe7 100644 --- a/Sources/SpeziAccount/AccountValue/Keys/UserIdKey.swift +++ b/Sources/SpeziAccount/AccountValue/Keys/UserIdKey.swift @@ -8,6 +8,7 @@ import SpeziFoundation import SpeziValidation +import SpeziViews import SwiftUI @@ -68,7 +69,7 @@ extension UserIdKey { @Environment(\.accountServiceConfiguration) private var configuration public var body: some View { - SimpleTextRow(name: configuration.userIdConfiguration.idType.localizedStringResource) { + ListRow(configuration.userIdConfiguration.idType.localizedStringResource) { Text(verbatim: value) } } diff --git a/Sources/SpeziAccount/Resources/Localizable.xcstrings b/Sources/SpeziAccount/Resources/Localizable.xcstrings index 38f358ec..148a73a3 100644 --- a/Sources/SpeziAccount/Resources/Localizable.xcstrings +++ b/Sources/SpeziAccount/Resources/Localizable.xcstrings @@ -882,29 +882,6 @@ } } }, - "Hello" : { - "comment" : "No need to translate, only used in Previews ...", - "localizations" : { - "de" : { - "stringUnit" : { - "state" : "translated", - "value" : "Hallo" - } - }, - "en" : { - "stringUnit" : { - "state" : "translated", - "value" : "Hello" - } - }, - "es" : { - "stringUnit" : { - "state" : "translated", - "value" : "Hola" - } - } - } - }, "MISSING_ACCOUNT_DETAILS" : { "localizations" : { "de" : { diff --git a/Sources/SpeziAccount/ViewModifier/AccountRequiredModifier.swift b/Sources/SpeziAccount/ViewModifier/AccountRequiredModifier.swift index c12bbdc8..0339c46a 100644 --- a/Sources/SpeziAccount/ViewModifier/AccountRequiredModifier.swift +++ b/Sources/SpeziAccount/ViewModifier/AccountRequiredModifier.swift @@ -30,7 +30,7 @@ struct AccountRequiredModifier: ViewModifier { } } .task { - try? await Task.sleep(for: .milliseconds(500)) + try? await Task.sleep(for: .seconds(2)) if !account.signedIn { presentingSheet = true } diff --git a/Sources/SpeziAccount/Views/DataDisplay/LocalizableStringBasedDisplayView.swift b/Sources/SpeziAccount/Views/DataDisplay/LocalizableStringBasedDisplayView.swift index b55f36f1..15c5ebb7 100644 --- a/Sources/SpeziAccount/Views/DataDisplay/LocalizableStringBasedDisplayView.swift +++ b/Sources/SpeziAccount/Views/DataDisplay/LocalizableStringBasedDisplayView.swift @@ -7,6 +7,7 @@ // import SpeziFoundation +import SpeziViews import SwiftUI @@ -17,7 +18,7 @@ public struct LocalizableStringBasedDisplayView: DataDisplayVie private let value: Key.Value public var body: some View { - SimpleTextRow(name: Key.name) { + ListRow(Key.name) { Text(value.localizedStringResource) } } @@ -37,8 +38,8 @@ extension Bool: CustomLocalizedStringResourceConvertible { #if DEBUG -struct LocalizableStringBasedDisplayView_Previews: PreviewProvider { - static var previews: some View { +#Preview { + Form { LocalizableStringBasedDisplayView(.preferNotToState) } } diff --git a/Sources/SpeziAccount/Views/DataDisplay/SimpleTextRow.swift b/Sources/SpeziAccount/Views/DataDisplay/SimpleTextRow.swift deleted file mode 100644 index 2cf80b34..00000000 --- a/Sources/SpeziAccount/Views/DataDisplay/SimpleTextRow.swift +++ /dev/null @@ -1,43 +0,0 @@ -// -// This source file is part of the Stanford Spezi open-source project -// -// SPDX-FileCopyrightText: 2023 Stanford University and the project authors (see CONTRIBUTORS.md) -// -// SPDX-License-Identifier: MIT -// - -import SwiftUI - - -struct SimpleTextRow: View { - private let name: LocalizedStringResource - private let value: Value - - - var body: some View { - HStack { - Text(name) - Spacer() - value - .foregroundColor(.secondary) - } - .accessibilityElement(children: .combine) - } - - - init(name: LocalizedStringResource, @ViewBuilder value: () -> Value) { - self.name = name - self.value = value() - } -} - - -#if DEBUG -struct SimpleTextRow_Previews: PreviewProvider { - static var previews: some View { - SimpleTextRow(name: LocalizedStringResource("Hello", comment: "No need to translate, only used in Previews ...")) { - Text(verbatim: "World") - } - } -} -#endif diff --git a/Sources/SpeziAccount/Views/DataDisplay/StringBasedDisplayView.swift b/Sources/SpeziAccount/Views/DataDisplay/StringBasedDisplayView.swift index d6250f85..e9d982ad 100644 --- a/Sources/SpeziAccount/Views/DataDisplay/StringBasedDisplayView.swift +++ b/Sources/SpeziAccount/Views/DataDisplay/StringBasedDisplayView.swift @@ -7,6 +7,7 @@ // import SpeziFoundation +import SpeziViews import SwiftUI @@ -15,7 +16,7 @@ public struct StringBasedDisplayView: DataDisplayView where Key private let value: Key.Value public var body: some View { - SimpleTextRow(name: Key.name) { + ListRow(Key.name) { Text(value) } } @@ -27,8 +28,8 @@ public struct StringBasedDisplayView: DataDisplayView where Key #if DEBUG -struct StringDataDisplayView_Previews: PreviewProvider { - static var previews: some View { +#Preview { + List { StringBasedDisplayView("andreas.bauer") } } diff --git a/Sources/SpeziAccount/Views/Fields/DateOfBirthPicker.swift b/Sources/SpeziAccount/Views/Fields/DateOfBirthPicker.swift index 63689b3c..5b13057e 100644 --- a/Sources/SpeziAccount/Views/Fields/DateOfBirthPicker.swift +++ b/Sources/SpeziAccount/Views/Fields/DateOfBirthPicker.swift @@ -6,6 +6,7 @@ // SPDX-License-Identifier: MIT // +import SpeziViews import SwiftUI @@ -21,6 +22,8 @@ public struct DateOfBirthPicker: DataEntryView { @Binding private var date: Date @State private var dateAdded = false + @State private var layout: DynamicLayout? + private var dateRange: ClosedRange { let calendar = Calendar.current let startDateComponents = DateComponents(year: 1800, month: 1, day: 1) @@ -38,7 +41,7 @@ public struct DateOfBirthPicker: DataEntryView { /// - The date is configured to be required. /// - We are NOT entering new date. Showing existing data the user might want to change. /// - If we are entering new data and the user pressed the add button. - private var showPicker: Bool { + @MainActor private var showPicker: Bool { account.configuration[Key.self]?.requirement == .required || viewType?.enteringNewData == false || dateAdded @@ -47,30 +50,40 @@ public struct DateOfBirthPicker: DataEntryView { public var body: some View { HStack { - Text(titleLocalization) - .multilineTextAlignment(.leading) - Spacer() - - if showPicker { - DatePicker( - selection: $date, - in: dateRange, - displayedComponents: .date - ) { - Text(titleLocalization) + DynamicHStack { + Text(titleLocalization) + .multilineTextAlignment(.leading) + + if layout == .horizontal { + Spacer() } + + if showPicker { + DatePicker( + selection: $date, + in: dateRange, + displayedComponents: .date + ) { + Text(titleLocalization) + } .labelsHidden() - } else { - Button(action: addDateAction) { - Text("ADD_DATE", bundle: .module) - .multilineTextAlignment(.center) - .foregroundColor(.primary) - .frame(width: 110, height: 34) - } + } else { + Button(action: addDateAction) { + Text("ADD_DATE", bundle: .module) + .multilineTextAlignment(.center) + .foregroundColor(.primary) + .padding([.leading, .trailing], 20) + .padding([.top, .bottom], 7) + } .background( RoundedRectangle(cornerRadius: 8) .fill(Color(uiColor: .tertiarySystemFill)) ) + } + } + + if layout == .vertical { + Spacer() } } .accessibilityRepresentation { @@ -86,6 +99,9 @@ public struct DateOfBirthPicker: DataEntryView { } } } + .onPreferenceChange(DynamicLayout.self) { value in + layout = value + } } @@ -118,11 +134,10 @@ struct DateOfBirthPicker_Previews: PreviewProvider { @State private var date = Date.now var body: some View { + Form { + DateOfBirthPicker(date: $date) + } VStack { - Form { - DateOfBirthPicker(date: $date) - } - .frame(height: 200) DateOfBirthPicker(date: $date) .padding(32) } diff --git a/Sources/SpeziAccount/Views/Fields/GenderIdentityPicker.swift b/Sources/SpeziAccount/Views/Fields/GenderIdentityPicker.swift index 64518db0..ccb2c107 100644 --- a/Sources/SpeziAccount/Views/Fields/GenderIdentityPicker.swift +++ b/Sources/SpeziAccount/Views/Fields/GenderIdentityPicker.swift @@ -49,18 +49,16 @@ struct GenderIdentityPicker_Previews: PreviewProvider { static var previews: some View { - VStack { - Form { - Grid { - GenderIdentityPicker(genderIdentity: $genderIdentity) - } - } - .frame(height: 200) + Form { Grid { GenderIdentityPicker(genderIdentity: $genderIdentity) } - .padding(32) } + + Grid { + GenderIdentityPicker(genderIdentity: $genderIdentity) + } + .padding(32) .background(Color(.systemGroupedBackground)) } } diff --git a/Sources/SpeziAccount/Views/SignupForm.swift b/Sources/SpeziAccount/Views/SignupForm.swift index 653dce48..4f62a0df 100644 --- a/Sources/SpeziAccount/Views/SignupForm.swift +++ b/Sources/SpeziAccount/Views/SignupForm.swift @@ -58,7 +58,7 @@ public struct SignupForm: View { @State private var presentingCloseConfirmation = false - private var accountKeyByCategory: OrderedDictionary { + @MainActor private var accountKeyByCategory: OrderedDictionary { var result = account.configuration.allCategorized(filteredBy: [.required, .collected]) // patch the user configured account values with account values additionally required by diff --git a/Tests/UITests/TestApp/AccountTests/AccountTestsView.swift b/Tests/UITests/TestApp/AccountTests/AccountTestsView.swift index b0a8833e..1517ccd5 100644 --- a/Tests/UITests/TestApp/AccountTests/AccountTestsView.swift +++ b/Tests/UITests/TestApp/AccountTests/AccountTestsView.swift @@ -134,10 +134,16 @@ struct AccountTestsView_Previews: PreviewProvider { static var previews: some View { AccountTestsView() - .environment(Account(TestAccountService(TestAlertModel(), .emailAddress))) + .previewWith { + AccountConfiguration { + TestAccountService(TestAlertModel(), .emailAddress) + } + } AccountTestsView() - .environment(Account(building: details, active: TestAccountService(TestAlertModel(), .emailAddress))) + .previewWith { + AccountConfiguration(building: details, active: TestAccountService(TestAlertModel(), .emailAddress)) + } AccountTestsView() .environment(Account()) diff --git a/Tests/UITests/TestApp/AccountTests/TestAlertModifier.swift b/Tests/UITests/TestApp/AccountTests/TestAlertModifier.swift index 30da11db..c5f1096a 100644 --- a/Tests/UITests/TestApp/AccountTests/TestAlertModifier.swift +++ b/Tests/UITests/TestApp/AccountTests/TestAlertModifier.swift @@ -10,7 +10,7 @@ import SwiftUI @Observable -final class TestAlertModel: Sendable { +final class TestAlertModel: @unchecked Sendable { var presentingAlert = false var continuation: CheckedContinuation? } diff --git a/Tests/UITests/UITests.xcodeproj/project.pbxproj b/Tests/UITests/UITests.xcodeproj/project.pbxproj index 39c0204f..67808eb3 100644 --- a/Tests/UITests/UITests.xcodeproj/project.pbxproj +++ b/Tests/UITests/UITests.xcodeproj/project.pbxproj @@ -235,7 +235,7 @@ attributes = { BuildIndependentTargetsInParallel = 1; LastSwiftUpdateCheck = 1410; - LastUpgradeCheck = 1410; + LastUpgradeCheck = 1520; TargetAttributes = { 2F6D139128F5F384007C25D6 = { CreatedOnToolsVersion = 14.1; @@ -361,6 +361,7 @@ isa = XCBuildConfiguration; buildSettings = { ALWAYS_SEARCH_USER_PATHS = NO; + ASSETCATALOG_COMPILER_GENERATE_SWIFT_ASSET_SYMBOL_EXTENSIONS = YES; CLANG_ANALYZER_NONNULL = YES; CLANG_ANALYZER_NUMBER_OBJECT_CONVERSION = YES_AGGRESSIVE; CLANG_CXX_LANGUAGE_STANDARD = "gnu++20"; @@ -421,6 +422,7 @@ isa = XCBuildConfiguration; buildSettings = { ALWAYS_SEARCH_USER_PATHS = NO; + ASSETCATALOG_COMPILER_GENERATE_SWIFT_ASSET_SYMBOL_EXTENSIONS = YES; CLANG_ANALYZER_NONNULL = YES; CLANG_ANALYZER_NUMBER_OBJECT_CONVERSION = YES_AGGRESSIVE; CLANG_CXX_LANGUAGE_STANDARD = "gnu++20"; @@ -582,6 +584,7 @@ isa = XCBuildConfiguration; buildSettings = { ALWAYS_SEARCH_USER_PATHS = NO; + ASSETCATALOG_COMPILER_GENERATE_SWIFT_ASSET_SYMBOL_EXTENSIONS = YES; CLANG_ANALYZER_NONNULL = YES; CLANG_ANALYZER_NUMBER_OBJECT_CONVERSION = YES_AGGRESSIVE; CLANG_CXX_LANGUAGE_STANDARD = "gnu++20"; @@ -733,7 +736,7 @@ repositoryURL = "https://github.com/StanfordSpezi/XCTestExtensions"; requirement = { kind = upToNextMinorVersion; - minimumVersion = 0.4.7; + minimumVersion = 0.4.9; }; }; A969240D2A9A198800E2128B /* XCRemoteSwiftPackageReference "swift-argument-parser" */ = { diff --git a/Tests/UITests/UITests.xcodeproj/xcshareddata/xcschemes/TestApp.xcscheme b/Tests/UITests/UITests.xcodeproj/xcshareddata/xcschemes/TestApp.xcscheme index 7099d095..f9df77c5 100644 --- a/Tests/UITests/UITests.xcodeproj/xcshareddata/xcschemes/TestApp.xcscheme +++ b/Tests/UITests/UITests.xcodeproj/xcshareddata/xcschemes/TestApp.xcscheme @@ -1,5 +1,6 @@