From 5a4ebae07cf6fe4dadc0d6f7b282a58486dfb789 Mon Sep 17 00:00:00 2001 From: Philipp Zagar Date: Sun, 26 Jan 2025 23:40:47 +0100 Subject: [PATCH] Fix undomanager, further state handelling improvements and fixes --- .../Consent/ConsentViewState.swift | 2 - .../Consent/Views/ConsentDocument.swift | 39 ++++++++------ .../Consent/Views/SignatureView.swift | 6 +++ .../OnboardingConsentView.swift | 52 +++++++++++++------ .../Consent/OnboardingConsentTestView.swift | 3 ++ .../XCUIApplication+Onboarding.swift | 9 ++++ 6 files changed, 76 insertions(+), 35 deletions(-) diff --git a/Sources/SpeziOnboarding/Consent/ConsentViewState.swift b/Sources/SpeziOnboarding/Consent/ConsentViewState.swift index 3d47b88..061ca12 100644 --- a/Sources/SpeziOnboarding/Consent/ConsentViewState.swift +++ b/Sources/SpeziOnboarding/Consent/ConsentViewState.swift @@ -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 } diff --git a/Sources/SpeziOnboarding/Consent/Views/ConsentDocument.swift b/Sources/SpeziOnboarding/Consent/Views/ConsentDocument.swift index d42ee64..5d7f366 100644 --- a/Sources/SpeziOnboarding/Consent/Views/ConsentDocument.swift +++ b/Sources/SpeziOnboarding/Consent/Views/ConsentDocument.swift @@ -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() @@ -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 @@ -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 + } else { + viewState = .namesEntered + } } } } @@ -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()` @@ -182,11 +186,11 @@ 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 } } } @@ -194,7 +198,10 @@ public struct ConsentDocument: View { } private var inputFieldsDisabled: Bool { - viewState == .base(.processing) || viewState == .export || viewState == .storing + switch viewState { + case .base(.processing), .export, .exported: true + default: false + } } var formattedConsentSignatureDate: String? { diff --git a/Sources/SpeziOnboarding/Consent/Views/SignatureView.swift b/Sources/SpeziOnboarding/Consent/Views/SignatureView.swift index 337dc01..d7fcb43 100644 --- a/Sources/SpeziOnboarding/Consent/Views/SignatureView.swift +++ b/Sources/SpeziOnboarding/Consent/Views/SignatureView.swift @@ -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 } diff --git a/Sources/SpeziOnboarding/OnboardingConsentView.swift b/Sources/SpeziOnboarding/OnboardingConsentView.swift index 21107c9..1114f37 100644 --- a/Sources/SpeziOnboarding/OnboardingConsentView.swift +++ b/Sources/SpeziOnboarding/OnboardingConsentView.swift @@ -13,7 +13,6 @@ import AppKit import Foundation import PDFKit import Spezi -import SpeziFoundation import SpeziViews import SwiftUI @@ -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 @@ -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)) { // In case of error, go back to previous state. - viewState = .base(.error(AnyLocalizedError(error: error))) + self.viewState = .base(.error(AnyLocalizedError(error: error))) } } } else { @@ -159,6 +161,11 @@ public struct OnboardingConsentView: View { .task { willShowShareSheet = false } + .onDisappear { + withAnimation(.easeIn(duration: 0.2)) { + self.viewState = .base(.idle) + } + } #endif } else { ProgressView() @@ -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 + } + } } } #if os(macOS) @@ -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 } } @@ -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() diff --git a/Tests/UITests/TestApp/Views/Consent/OnboardingConsentTestView.swift b/Tests/UITests/TestApp/Views/Consent/OnboardingConsentTestView.swift index 90f890c..dfb0fa1 100644 --- a/Tests/UITests/TestApp/Views/Consent/OnboardingConsentTestView.swift +++ b/Tests/UITests/TestApp/Views/Consent/OnboardingConsentTestView.swift @@ -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() }, diff --git a/Tests/UITests/TestAppUITests/XCUIApplication+Onboarding.swift b/Tests/UITests/TestAppUITests/XCUIApplication+Onboarding.swift index 4abdf11..70351ed 100644 --- a/Tests/UITests/TestAppUITests/XCUIApplication+Onboarding.swift +++ b/Tests/UITests/TestAppUITests/XCUIApplication+Onboarding.swift @@ -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")