philippzagar commented 1 year ago

Advanced Onboarding Flow Infrastructure

:recycle: Current situation & Problem

The existing SpeziOnboarding module offers an array of views to facilitate the construction of onboarding interfaces for developers. However, it doesn't possess the requisite infrastructure to enable efficient creation and management of complex onboarding processes typically seen in digital health applications. This lack of infrastructure necessitates developers to incorporate custom logic within their applications to manage specific aspects of the onboarding process, like condition-based skipping of steps. An example illustrating this problem can be found here.

:bulb: Proposed solution

This pull request proposes enhancements to the SpeziOnboarding module with the aim of bridging the identified gaps and facilitating a more streamlined approach towards the creation and management of comprehensive onboarding flows. To achieve this, we introduce additional onboarding flow infrastructure within the SpeziOnboarding package that balances the automation offered by the framework with the customization requirements of developers.

The primary additions to the SpeziOnboarding public API are the OnboardingStack and OnboardingNavigationPath components. These components empower developers to create an intuitive and easy-to-manage onboarding flow in digital health applications built on the Spezi framework.

OnboardingStack: Enables developers to state the onboarding views (and their order) which are then managed by SpeziOnboarding (similar to the SwiftUI NavigationStack). One is able to simply state the onboarding views as well as conditions in there as the underlying technology is a Swift result builder. However, make sure to understand that the evaluation of this result builder is only done as long as the user is still on the first onboarding view. This is necessary as later reevaluations of the result builder make it quite hard to keep track of the order of the individual views.
OnboardingNavigationPath: Represents the main logic of the onboarding flow by managing the current navigation path (similar to the SwiftUI NavigationPath). An instance of this type is injected as an EnvironmentObject into all onboarding views stated within the OnboardingStack which can then utilize it to either automatically move to the next meant onboarding step or manually set the path to a specific view (one that is either stated in the OnboardingStack or an entirely custom view that isn't managed by the OnboardingNavigationPath).

Following is a small code example to illustrate the use of these new APIs:

struct Onboarding: View {
     @AppStorage(StorageKeys.onboardingFlowComplete) var completedOnboardingFlow = false
     @State private var localNotificationAuthorization = false

     var body: some View {
         OnboardingStack(onboardingFlowComplete: $completedOnboardingFlow) {
            Welcome()
            InterestingModules()

            if HKHealthStore.isHealthDataAvailable() {
                 HealthKitPermissions()
             }

             if !localNotificationAuthorization {
                 NotificationPermissions()
             }
         }
         .task {
             localNotificationAuthorization = await ...
         }
     }
 }

 struct Welcome: View {
     @EnvironmentObject private var onboardingNavigationPath: OnboardingNavigationPath

     var body: some View {
         OnboardingView(
             ...,
             action: {
                 // Automatically navigates to the next `OnboardingStep`, as outlined by the order of views within the `OnboardingStack`
                 onboardingNavigationPath.nextStep()

                 // Manually navigates to an onboarding view identified by its static type which is declared within the `OnboardingStack`. After this manual navigation step, the `OnboardingNavigationPath` will continue in the declared onboarding order from the `OnboardingStack`.
                 onboardingNavigationPath.append(InterestingModules.self)

                // Manually navigate to an onboarding view which is not declared within the `OnboardingStack`. The internal state of the `OnboardingNavigationPath` won't be moved and stay at the old position.
                onboardingNavigationPath.append(customView: SomeCustomView())
             }
         )
     }
 }

:gear: Release Notes

Provide infrastructure for a simplified onboarding flow. The main part of the public API revolves around the OnboardingStack and the OnboardingNavigationPath.

:heavy_plus_sign: Additional Information

The current implementation of SpeziOnboarding identifies views within the OnboardingStack via their static types. Although it would have been possible to introduce a new protocol — to which all onboarding views would need to conform — requiring a manual identification mechanism, such an approach was determined to impose too much adaptation overhead to the new onboarding system. However, this method of identification through static types imposes certain limitations, notably the restriction that only a single onboarding view of the same type can be used within the OnboardingStack. Despite this, the limitation is unlikely to affect most users, yet it is crucial to acknowledge this trade-off.

Another essential design consideration relates to the mechanism used to determine the current state of the internal navigation path of the OnboardingNavigationPath, i.e., which onboarding view is currently being displayed. This identification is critical for transitioning to the subsequent onboarding view. While it would have been feasible to manually maintain a record of all views within the navigation path, this would have required extensive bookkeeping, along with potentially complex mechanisms to manage back button interactions (for instance, disabling it during HealthKit authorizations), necessitating a custom back button due to SwiftUI's inability to "listen" for back button interactions. Hence, I chose to leverage the Codable property of SwiftUI's NavigationPath (as detailed here). Using an auxiliary logic function (notwithstanding its computational inefficiency, which should not be a bottleneck in this case), we can extract the top element from the SwiftUI NavigationPath, precisely fulfilling our requirement for the onboarding functionality. One downside of this approach is its perceived "hacky" nature since Apple did not explicitly design the NavigationPath content to be accessed directly by developers. However, the current implementation appears to be the most sensible solution with minimal drawbacks. Please refer to the NavigationPath+Codable.swift file for more information on the implementation specifics.

Related PRs

Application of the functionality implemented within this PR can be seen here: https://github.com/StanfordSpezi/SpeziTemplateApplication/pull/27

Testing

UI tests are included and adjusted from the original ones in order to better reflect the updated onboarding infrastructure.

Reviewer Nudging

Start at the OnboardingStack which is the main entry point into the new onboarding infrastructure, then continue with the OnboardingNavigationPath which represents the main logic behind navigating the onboarding flow.

Code of Conduct & Contributing Guidelines

By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:

[x] I agree to follow the Code of Conduct and Contributing Guidelines.

codecov[bot] commented 1 year ago

Codecov Report

Merging #10 (5bd277b) into main (a84ddd7) will increase coverage by 2.38%. The diff coverage is 80.44%.

Additional details and impacted files

[![Impacted file tree graph](https://app.codecov.io/gh/StanfordSpezi/SpeziOnboarding/pull/10/graphs/tree.svg?width=650&height=150&src=pr&token=tVwIFVPdJG&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi)](https://app.codecov.io/gh/StanfordSpezi/SpeziOnboarding/pull/10?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi) ```diff @@ Coverage Diff @@ ## main #10 +/- ## ========================================== + Coverage 73.98% 76.36% +2.38% ========================================== Files 7 14 +7 Lines 511 740 +229 ========================================== + Hits 378 565 +187 - Misses 133 175 +42 ``` | [Files Changed](https://app.codecov.io/gh/StanfordSpezi/SpeziOnboarding/pull/10?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi) | Coverage Δ | | |---|---|---| | [...ources/SpeziOnboarding/OnboardingActionsView.swift](https://app.codecov.io/gh/StanfordSpezi/SpeziOnboarding/pull/10?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi#diff-U291cmNlcy9TcGV6aU9uYm9hcmRpbmcvT25ib2FyZGluZ0FjdGlvbnNWaWV3LnN3aWZ0) | `45.68% <0.00%> (ø)` | | | [...ing/OnboardingFlow/IllegalOnboardingStepView.swift](https://app.codecov.io/gh/StanfordSpezi/SpeziOnboarding/pull/10?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi#diff-U291cmNlcy9TcGV6aU9uYm9hcmRpbmcvT25ib2FyZGluZ0Zsb3cvSWxsZWdhbE9uYm9hcmRpbmdTdGVwVmlldy5zd2lmdA==) | `0.00% <0.00%> (ø)` | | | [...oarding/OnboardingFlow/OnboardingViewBuilder.swift](https://app.codecov.io/gh/StanfordSpezi/SpeziOnboarding/pull/10?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi#diff-U291cmNlcy9TcGV6aU9uYm9hcmRpbmcvT25ib2FyZGluZ0Zsb3cvT25ib2FyZGluZ1ZpZXdCdWlsZGVyLnN3aWZ0) | `53.85% <53.85%> (ø)` | | | [...eziOnboarding/OnboardingFlow/OnboardingStack.swift](https://app.codecov.io/gh/StanfordSpezi/SpeziOnboarding/pull/10?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi#diff-U291cmNlcy9TcGV6aU9uYm9hcmRpbmcvT25ib2FyZGluZ0Zsb3cvT25ib2FyZGluZ1N0YWNrLnN3aWZ0) | `83.34% <83.34%> (ø)` | | | [...ding/OnboardingFlow/OnboardingNavigationPath.swift](https://app.codecov.io/gh/StanfordSpezi/SpeziOnboarding/pull/10?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi#diff-U291cmNlcy9TcGV6aU9uYm9hcmRpbmcvT25ib2FyZGluZ0Zsb3cvT25ib2FyZGluZ05hdmlnYXRpb25QYXRoLnN3aWZ0) | `83.90% <83.90%> (ø)` | | | [...arding/OnboardingFlow/NavigationPath+Codable.swift](https://app.codecov.io/gh/StanfordSpezi/SpeziOnboarding/pull/10?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi#diff-U291cmNlcy9TcGV6aU9uYm9hcmRpbmcvT25ib2FyZGluZ0Zsb3cvTmF2aWdhdGlvblBhdGgrQ29kYWJsZS5zd2lmdA==) | `94.74% <94.74%> (ø)` | | | [.../OnboardingFlow/OnboardingFlowViewCollection.swift](https://app.codecov.io/gh/StanfordSpezi/SpeziOnboarding/pull/10?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi#diff-U291cmNlcy9TcGV6aU9uYm9hcmRpbmcvT25ib2FyZGluZ0Zsb3cvT25ib2FyZGluZ0Zsb3dWaWV3Q29sbGVjdGlvbi5zd2lmdA==) | `100.00% <100.00%> (ø)` | | | [...ding/OnboardingFlow/OnboardingStepIdentifier.swift](https://app.codecov.io/gh/StanfordSpezi/SpeziOnboarding/pull/10?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi#diff-U291cmNlcy9TcGV6aU9uYm9hcmRpbmcvT25ib2FyZGluZ0Zsb3cvT25ib2FyZGluZ1N0ZXBJZGVudGlmaWVyLnN3aWZ0) | `100.00% <100.00%> (ø)` | | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/StanfordSpezi/SpeziOnboarding/pull/10/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi) ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/StanfordSpezi/SpeziOnboarding/pull/10?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/StanfordSpezi/SpeziOnboarding/pull/10?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi). Last update [a84ddd7...5bd277b](https://app.codecov.io/gh/StanfordSpezi/SpeziOnboarding/pull/10?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=StanfordSpezi).

philippzagar commented 11 months ago

Thanks for the feedback @PSchmiedmayer, it greatly contributed to overall design of the implementation! 🚀

Feel free to take a quick look into the PR again, I think it's in a good state to receive some high level feedback. Of course, there are some minor details to be figured out, more docs and test cases to be written etc., but I guess now would be a good time to receive some more feedback on the overall concept and design of the PR 😄

Feel free to check out the direct application of the functionality in this template application PR

philippzagar commented 11 months ago

Hi @PSchmiedmayer, the PR is now in a reviewable state and proper descriptions, docs, and tests are written. 🚀 Only issue I couldn't solve was regarding CodeCov which apparently isn't properly receiving the collected code coverage stats. I tried to follow your suggestions from https://github.com/StanfordBDHG/XCTHealthKit/pull/13#issuecomment-1622464382 but sadly that didn't seem to work. Any other ideas here? :) -> Reverting my code cov changes suddenly resulted in a working CodeCov 😄

PSchmiedmayer commented 11 months ago

@philippzagar Regarding the flakey UI test coverages: I would recommend that you explicitly add the SpeziOnboarding target to the list of build dependencies in the Scheme editor:

This helped a lot in previous PRs when I ran into similar issues. I would also recommend explicitly adding it as a code coverage target in the TestPlan:

It seems like the test plan is currently not used (see scheme settings). You can explicitly point Xcode to it and set the Build configuration to "Test" to ensure that Previews should not count to the coverage:

CC: @StanfordSpezi/developers if we run into similar issues in other PRs as well 👍

philippzagar commented 11 months ago

Thanks @PSchmiedmayer for the nice review, I'll merge this PR soon! 🚀

philippzagar commented 11 months ago

@PSchmiedmayer I drafted a new release of SpeziOnboarding (0.4.0 as it is quite a big upgrade for the onboarding package), feel free to publish it: https://github.com/StanfordSpezi/SpeziOnboarding/releases

PSchmiedmayer commented 11 months ago

Thank you for the PR, I tagged the release! 🚀

StanfordSpezi / SpeziOnboarding