Skip to content

Commit

Permalink
Support dynamic type sizes and longer timeout for required account mo…
Browse files Browse the repository at this point in the history
…difier (#51)

# Support dynamic type sizes and longer timeout for required account
modifier

## ♻️ 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.

## ⚙️ Release Notes 
* Fixed an issue where the account required sheet was popping up to
early.
* Optimize UI for larger dynamic type sizes.


## 📚 Documentation
--


## ✅ Testing
--


## 📝 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).
  • Loading branch information
Supereg authored Feb 20, 2024
1 parent 714f01a commit 1ced521
Show file tree
Hide file tree
Showing 17 changed files with 77 additions and 115 deletions.
1 change: 1 addition & 0 deletions .github/workflows/build-and-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ jobs:
runsonlabels: '["macOS", "self-hosted"]'
path: 'Tests/UITests'
scheme: TestApp
setupSimulators: true
uploadcoveragereport:
name: Upload Coverage Report
needs: [buildandtest, buildandtestuitests]
Expand Down
4 changes: 2 additions & 2 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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")
],
Expand Down
3 changes: 2 additions & 1 deletion Sources/SpeziAccount/AccountValue/Keys/DateOfBirthKey.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
// SPDX-License-Identifier: MIT
//

import SpeziViews
import SwiftUI


Expand Down Expand Up @@ -58,7 +59,7 @@ extension DateOfBirthKey {
}

public var body: some View {
SimpleTextRow(name: Key.name) {
ListRow(Key.name) {
Text(value.formatted(formatStyle))
}
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/SpeziAccount/AccountValue/Keys/PersonNameKey.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
}
}
Expand Down
3 changes: 2 additions & 1 deletion Sources/SpeziAccount/AccountValue/Keys/UserIdKey.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import SpeziFoundation
import SpeziValidation
import SpeziViews
import SwiftUI


Expand Down Expand Up @@ -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)
}
}
Expand Down
23 changes: 0 additions & 23 deletions Sources/SpeziAccount/Resources/Localizable.xcstrings
Original file line number Diff line number Diff line change
Expand Up @@ -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" : {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ struct AccountRequiredModifier<SetupSheet: View>: ViewModifier {
}
}
.task {
try? await Task.sleep(for: .milliseconds(500))
try? await Task.sleep(for: .seconds(2))
if !account.signedIn {
presentingSheet = true
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
//

import SpeziFoundation
import SpeziViews
import SwiftUI


Expand All @@ -17,7 +18,7 @@ public struct LocalizableStringBasedDisplayView<Key: AccountKey>: DataDisplayVie
private let value: Key.Value

public var body: some View {
SimpleTextRow(name: Key.name) {
ListRow(Key.name) {
Text(value.localizedStringResource)
}
}
Expand All @@ -37,8 +38,8 @@ extension Bool: CustomLocalizedStringResourceConvertible {


#if DEBUG
struct LocalizableStringBasedDisplayView_Previews: PreviewProvider {
static var previews: some View {
#Preview {
Form {
LocalizableStringBasedDisplayView<GenderIdentityKey>(.preferNotToState)
}
}
Expand Down
43 changes: 0 additions & 43 deletions Sources/SpeziAccount/Views/DataDisplay/SimpleTextRow.swift

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
//

import SpeziFoundation
import SpeziViews
import SwiftUI


Expand All @@ -15,7 +16,7 @@ public struct StringBasedDisplayView<Key: AccountKey>: DataDisplayView where Key
private let value: Key.Value

public var body: some View {
SimpleTextRow(name: Key.name) {
ListRow(Key.name) {
Text(value)
}
}
Expand All @@ -27,8 +28,8 @@ public struct StringBasedDisplayView<Key: AccountKey>: DataDisplayView where Key


#if DEBUG
struct StringDataDisplayView_Previews: PreviewProvider {
static var previews: some View {
#Preview {
List {
StringBasedDisplayView<UserIdKey>("andreas.bauer")
}
}
Expand Down
61 changes: 38 additions & 23 deletions Sources/SpeziAccount/Views/Fields/DateOfBirthPicker.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
// SPDX-License-Identifier: MIT
//

import SpeziViews
import SwiftUI


Expand All @@ -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<Date> {
let calendar = Calendar.current
let startDateComponents = DateComponents(year: 1800, month: 1, day: 1)
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -86,6 +99,9 @@ public struct DateOfBirthPicker: DataEntryView {
}
}
}
.onPreferenceChange(DynamicLayout.self) { value in
layout = value
}
}


Expand Down Expand Up @@ -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)
}
Expand Down
14 changes: 6 additions & 8 deletions Sources/SpeziAccount/Views/Fields/GenderIdentityPicker.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/SpeziAccount/Views/SignupForm.swift
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public struct SignupForm<Header: View>: View {

@State private var presentingCloseConfirmation = false

private var accountKeyByCategory: OrderedDictionary<AccountKeyCategory, [any AccountKey.Type]> {
@MainActor private var accountKeyByCategory: OrderedDictionary<AccountKeyCategory, [any AccountKey.Type]> {
var result = account.configuration.allCategorized(filteredBy: [.required, .collected])

// patch the user configured account values with account values additionally required by
Expand Down
10 changes: 8 additions & 2 deletions Tests/UITests/TestApp/AccountTests/AccountTestsView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
2 changes: 1 addition & 1 deletion Tests/UITests/TestApp/AccountTests/TestAlertModifier.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import SwiftUI


@Observable
final class TestAlertModel: Sendable {
final class TestAlertModel: @unchecked Sendable {
var presentingAlert = false
var continuation: CheckedContinuation<Void, Never>?
}
Expand Down
Loading

0 comments on commit 1ced521

Please sign in to comment.