Skip to content

Commit

Permalink
Fix undomanager, further state handelling improvements and fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
philippzagar committed Jan 26, 2025
1 parent 8ea59c9 commit 5a4ebae
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 35 deletions.
2 changes: 0 additions & 2 deletions Sources/SpeziOnboarding/Consent/ConsentViewState.swift
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,4 @@ public enum ConsentViewState: Equatable {
/// The export representation creation (resulting in the ``ConsentViewState/exported(representation:)`` state) can be triggered
/// via setting the ``ConsentViewState/export`` state of the ``ConsentDocument`` .
case exported(representation: ConsentDocumentExportRepresentation)
/// The `storing` state indicates that the ``ConsentDocument`` is currently being stored to the Standard.
case storing
}
39 changes: 23 additions & 16 deletions Sources/SpeziOnboarding/Consent/Views/ConsentDocument.swift
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,13 @@ public struct ConsentDocument: View {
#endif
}
.disabled(inputFieldsDisabled)
.onChange(of: name) {
.onChange(of: name) { _, name in
if !(name.givenName?.isEmpty ?? true) && !(name.familyName?.isEmpty ?? true) {
viewState = .namesEntered
} else {
viewState = .base(.idle)
withAnimation(.easeIn(duration: 0.2)) {
viewState = .base(.idle)
}
// Reset all strokes if name fields are not complete anymore
#if !os(macOS)
signature.strokes.removeAll()
Expand Down Expand Up @@ -135,7 +137,7 @@ public struct ConsentDocument: View {
}
.padding(.vertical, 4)
.disabled(inputFieldsDisabled)
.onChange(of: signature) {
.onChange(of: signature) { _, signature in
#if !os(macOS)
let isSignatureEmpty = signature.strokes.isEmpty
#else
Expand All @@ -144,7 +146,11 @@ public struct ConsentDocument: View {
if !(isSignatureEmpty || (name.givenName?.isEmpty ?? true) || (name.familyName?.isEmpty ?? true)) {
viewState = .signed
} else {
viewState = .namesEntered
if (name.givenName?.isEmpty ?? true) || (name.familyName?.isEmpty ?? true) {
viewState = .base(.idle) // Hide signature view if names not complete anymore

Check warning on line 150 in Sources/SpeziOnboarding/Consent/Views/ConsentDocument.swift

View check run for this annotation

Codecov / codecov/patch

Sources/SpeziOnboarding/Consent/Views/ConsentDocument.swift#L150

Added line #L150 was not covered by tests
} else {
viewState = .namesEntered
}
}
}
}
Expand All @@ -166,14 +172,12 @@ public struct ConsentDocument: View {
}
.transition(.opacity)
.animation(.easeInOut, value: viewState == .namesEntered)
.onChange(of: viewState) {
.task(id: viewState) {
if case .export = viewState {
Task {
// Captures the current state of the document and transforms it to the `ConsentDocumentExportRepresentation`
viewState = .exported(
representation: await self.exportRepresentation
)
}
// Captures the current state of the document and transforms it to the `ConsentDocumentExportRepresentation`
self.viewState = .exported(
representation: await self.exportRepresentation
)
} else if case .base(let baseViewState) = viewState,
case .idle = baseViewState {
// Reset view state to correct one after handling an error view state via `.viewStateAlert()`
Expand All @@ -182,19 +186,22 @@ public struct ConsentDocument: View {
#else
let isSignatureEmpty = signature.isEmpty
#endif

if !isSignatureEmpty {
viewState = .signed
} else if !(name.givenName?.isEmpty ?? true) || !(name.familyName?.isEmpty ?? true) {
viewState = .namesEntered
self.viewState = .signed
} else if !(name.givenName?.isEmpty ?? true) && !(name.familyName?.isEmpty ?? true) {
self.viewState = .namesEntered

Check warning on line 193 in Sources/SpeziOnboarding/Consent/Views/ConsentDocument.swift

View check run for this annotation

Codecov / codecov/patch

Sources/SpeziOnboarding/Consent/Views/ConsentDocument.swift#L193

Added line #L193 was not covered by tests
}
}
}
.viewStateAlert(state: $viewState.base)
}

private var inputFieldsDisabled: Bool {
viewState == .base(.processing) || viewState == .export || viewState == .storing
switch viewState {
case .base(.processing), .export, .exported: true
default: false
}
}

var formattedConsentSignatureDate: String? {
Expand Down
6 changes: 6 additions & 0 deletions Sources/SpeziOnboarding/Consent/Views/SignatureView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,12 @@ public struct SignatureView: View {
#endif
}
#if !os(macOS)
.task {
// Crucial to reset the `UndoManager` between different `ConsentView`s in the `OnboardingStack`.
// Otherwise, actions are often not picked up
undoManager?.removeAllActions()
canUndo = false
}
.onChange(of: undoManager?.canUndo) { _, canUndo in
self.canUndo = canUndo ?? false
}
Expand Down
52 changes: 35 additions & 17 deletions Sources/SpeziOnboarding/OnboardingConsentView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import AppKit
import Foundation
import PDFKit
import Spezi
import SpeziFoundation
import SpeziViews
import SwiftUI

Expand Down Expand Up @@ -52,7 +51,7 @@ public struct OnboardingConsentView: View {
}

private let markdown: () async -> Data
private let action: (_ document: PDFDocument) async throws -> Void
private let action: @MainActor (_ document: PDFDocument) async throws -> Void
private let title: LocalizedStringResource?
private let currentDateInSignature: Bool
private let exportConfiguration: ConsentDocumentExportRepresentation.Configuration
Expand Down Expand Up @@ -84,35 +83,38 @@ public struct OnboardingConsentView: View {
actionView: {
Button(
action: {
viewState = .export
withAnimation(.easeOut(duration: 0.2)) {
viewState = .export // Triggers the export process
}
},
label: {
Text("CONSENT_ACTION", bundle: .module)
.frame(maxWidth: .infinity, minHeight: 38)
.processingOverlay(isProcessing: viewState == .storing || (viewState == .export && !willShowShareSheet))
.processingOverlay(isProcessing: backButtonHidden)
}
)
.buttonStyle(.borderedProminent)
.disabled(!actionButtonsEnabled)
.animation(.easeInOut, value: actionButtonsEnabled)
.animation(.easeInOut(duration: 0.2), value: actionButtonsEnabled)
.id("ActionButton")
}
)
.scrollDisabled($viewState.signing.wrappedValue)
.navigationBarBackButtonHidden(backButtonHidden)
.onChange(of: viewState) {
.task(id: viewState) {
if case .exported(let consentExport) = viewState {
if !willShowShareSheet {
viewState = .storing
do {
// Pass the rendered consent form to the `action` closure
try await action(consentExport.render())

Task {
do {
// Calls the passed `action` closure with the rendered consent PDF.
try await action(consentExport.render())
viewState = .base(.idle)
} catch {
withAnimation(.easeIn(duration: 0.2)) {
self.viewState = .base(.idle)
}
} catch {
withAnimation(.easeIn(duration: 0.2)) {

Check warning on line 115 in Sources/SpeziOnboarding/OnboardingConsentView.swift

View check run for this annotation

Codecov / codecov/patch

Sources/SpeziOnboarding/OnboardingConsentView.swift#L115

Added line #L115 was not covered by tests
// In case of error, go back to previous state.
viewState = .base(.error(AnyLocalizedError(error: error)))
self.viewState = .base(.error(AnyLocalizedError(error: error)))

Check warning on line 117 in Sources/SpeziOnboarding/OnboardingConsentView.swift

View check run for this annotation

Codecov / codecov/patch

Sources/SpeziOnboarding/OnboardingConsentView.swift#L117

Added line #L117 was not covered by tests
}
}
} else {
Expand Down Expand Up @@ -159,6 +161,11 @@ public struct OnboardingConsentView: View {
.task {
willShowShareSheet = false
}
.onDisappear {
withAnimation(.easeIn(duration: 0.2)) {
self.viewState = .base(.idle)
}
}

Check warning on line 168 in Sources/SpeziOnboarding/OnboardingConsentView.swift

View check run for this annotation

Codecov / codecov/patch

Sources/SpeziOnboarding/OnboardingConsentView.swift#L159-L168

Added lines #L159 - L168 were not covered by tests
#endif
} else {
ProgressView()
Expand All @@ -170,6 +177,12 @@ public struct OnboardingConsentView: View {
} else {
ProgressView()
.padding()
.task {
// This is required as only the "Markup" action from the ShareSheet misses to dismiss the share sheet again
if !willShowShareSheet {
showShareSheet = false
}
}

Check warning on line 185 in Sources/SpeziOnboarding/OnboardingConsentView.swift

View check run for this annotation

Codecov / codecov/patch

Sources/SpeziOnboarding/OnboardingConsentView.swift#L180-L185

Added lines #L180 - L185 were not covered by tests
}
}
#if os(macOS)
Expand Down Expand Up @@ -200,12 +213,17 @@ public struct OnboardingConsentView: View {
}

private var backButtonHidden: Bool {
viewState == .storing || (viewState == .export && !willShowShareSheet)
let exportStates = switch viewState {
case .export, .exported: true
default: false
}

return exportStates && !willShowShareSheet
}

private var actionButtonsEnabled: Bool {
switch viewState {
case .signing, .signed, .exported: true
case .signing, .signed: true
default: false
}
}
Expand All @@ -220,7 +238,7 @@ public struct OnboardingConsentView: View {
/// - exportConfiguration: Defines the properties of the exported consent form via ``ConsentDocumentExportRepresentation/Configuration``.
public init(
markdown: @escaping () async -> Data,
action: @escaping (_ document: PDFDocument) async throws -> Void,
action: @escaping @MainActor (_ document: PDFDocument) async throws -> Void,
title: LocalizedStringResource? = LocalizationDefaults.consentFormTitle,
currentDateInSignature: Bool = true,
exportConfiguration: ConsentDocumentExportRepresentation.Configuration = .init()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ struct OnboardingConsentTestView: View {
case .second: standard.secondConsentDocument = exportedConsent
}

// Simulate storage / upload delay of consent form
try await Task.sleep(until: .now + .seconds(0.5))

// Navigates to the next onboarding step
path.nextStep()
},
Expand Down
9 changes: 9 additions & 0 deletions Tests/UITests/TestAppUITests/XCUIApplication+Onboarding.swift
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,23 @@ extension XCUIApplication {
XCTAssert(staticTexts["Name: Leland Stanford"].waitForExistence(timeout: 2))

#if !os(macOS)
XCTAssert(buttons["Undo"].waitForExistence(timeout: 2.0))
XCTAssertFalse(buttons["Undo"].isEnabled)

staticTexts["Name: Leland Stanford"].swipeRight()

XCTAssert(buttons["Undo"].waitForExistence(timeout: 2.0))
XCTAssertTrue(buttons["Undo"].isEnabled)
buttons["Undo"].tap()

XCTAssert(buttons["Undo"].waitForExistence(timeout: 2.0))
XCTAssertFalse(buttons["Undo"].isEnabled)

XCTAssert(scrollViews["Signature Field"].waitForExistence(timeout: 2))
scrollViews["Signature Field"].swipeRight()

XCTAssert(buttons["Undo"].waitForExistence(timeout: 2.0))
XCTAssertTrue(buttons["Undo"].isEnabled)
#else
XCTAssert(textFields["Signature Field"].waitForExistence(timeout: 2))
try textFields["Signature Field"].enter(value: "Leland Stanford")
Expand Down

0 comments on commit 5a4ebae

Please sign in to comment.