-
-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Component communication #72
Conversation
cb89973
to
1f5a263
Compare
…old default implementations work correctly.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #72 +/- ##
==========================================
- Coverage 89.45% 88.83% -0.61%
==========================================
Files 30 44 +14
Lines 635 743 +108
==========================================
+ Hits 568 660 +92
- Misses 67 83 +16
Continue to review full report in Codecov by Sentry.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for all the changes there @Supereg, a huge step forward 🚀
I had a few comments here and there. Overall the PR looks great and feel free to merge it after addressing some of the comments and iterating on some of the features. Thank for all the work and time that goes into this project.
Very cool to see some of the @Apodini aspects making a reappearance here; more and more of the learnings from that project are applied to @StanfordSpezi, very nice!
Sources/Spezi/Module/Capabilities/Communication/CollectPropertyWrapper.swift
Outdated
Show resolved
Hide resolved
@PSchmiedmayer I addresses all the feedback. Wanted to re-request your review just to make sure 🚀 |
Thank you! I will have some time in the late afternoon to re-review the PR. If it is fine with you I can fix smaller issues that I might find myself and then merge the PR after that? Maybe I don't even have some feedback and could also directly merge it 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing job with the PR @Supereg, very nice additions and a superb code quality!
# Unified Account View with rethought Account Service Model ## ♻️ Current situation & Problem Currently, `SepziAccount` exposes two distinct views: `Login` and `SignUp`. While visually identical (expect for customizations), the AccountService buttons navigate to the respective Login or SignUp views for data entry. This introduces several navigation steps for the user even for simple workflows (e.g. just signing in a returning user). Further, it is not clear where to place Identity Provider buttons (e.g., Sign in with Apple), as these create a new account or just sign you in to your existing one with the same, single button. ## 💡 Proposed solution This PR makes the following changes: * Unifying `Login` and `SignUp` views to be represented by a single view, placing identity providers on the same page as regular Account Services, the `AccountSetup` view. * Restructure the Account Service Model: * Make the necessary preparations to support third-party Identity providers * Introduce the concept of an `EmbeddableAccountService` and a new `KeyPasswordBasedAccountService` as new abstraction layers. This replaces the current `UsernamePasswordAccountService` and `EmailPasswordAccountService` implementations. Those we really only Mock implementations which provided extensibility through class inheritance. This was replaced by providing the same functionality with a hierarchy of protocols being more powerful and providing much more flexibility. Further, we introduce new Mock Account services (e.g., used by PreviewProviders) which more clearly communicate their use through their updated name. * Move the UI parts of an AccountService out into a distinct abstraction layer (`AccountSetupViewStyle`s). * Introduce the `Account Value` (through `AccountKey` implementations) abstraction to support arbitrary account values * They provide a name, a category (for optimized placement in the UI), and a initial value * The provide the UI components do display (`DataDisplayView`) and setup or edit (`DataEntryView`) account values * The provide easy to use accessors using extensions on `AccountKeys` and `AccountValues` * Optimized API surface for end-users: * Exposes the `signedIn` published property (same as previously) * Exposees the `details` shared repository to access the `AccountDetails` of the currently logged in use. This also provides access to the `AccountService` and its configuration that was used to setup the user account. * Provide a new `AccountOverview` view: * Allows to view arbitrary account values * Allows to edit arbitrary account values * Provide Logout and account Removal functionality * Enabled through StanfordSpezi/Spezi#72, one can easily configure `SpeziAccount` centrally while other configured components can provide their AccountServices via [Component Communication](https://swiftpackageindex.com/stanfordspezi/spezi/documentation/spezi/component#Communication). This PR tries to provide extensive documentation for all areas. To provide some insights into the new user-facing API surface. This is how you would configure Spezi Account now: ```swift class YourAppDelegate: SpeziAppDelegate { override var configuration: Configuration { AccountConfiguration(configuration: [ // the user defines what account values are used and if they are `required`, `collected` or just `supported` .requires(\.userId), .requires(\.password), .requires(\.name), .collects(\.dateOfBirth), .collects(\.genderIdentity) ]) } } ``` Below is an example on how to use the account setup view: ```swift struct MyOnboardingView: View { var body: some View { AccountSetup { NavigationLink { // ... next view if already signed in } label: { Text("Continue") } } } } ``` Lastly, the account overview is equally as easy to use: ```swift struct MyView: View { var body: some View { AccountOverview() } } ``` ## ⚙️ Release Notes * New, unified `AccountSetup` view * New `AccountOverview` view * Introduce new `AccountService` abstraction * Support arbitrary account values through `AccountKey`-based shared repository implementation ## ➕ Additional Information ### Related PRs and Issues * This PR provides the groundwork necessary for StanfordSpezi/SpeziFirebase#7 * The updated Firebase implementation: StanfordSpezi/SpeziFirebase#8 ### Testing Substantial UI tests were updated and added. ### Reviewer Nudging As this is quite a large PR it would make sense to start reviewing by reading through the documentation. First of all reviewing the documentation itself (structure and readability). Code example on the DocC article already explain a lot of the concepts. The areas that sparked your attention, where you think that something is off or hard to understand, I would recommend to deep dive into the respective code area. ### 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). --------- Co-authored-by: Paul Schmiedmayer <PSchmiedmayer@users.noreply.github.com>
…ezi#7) # Unified Account View with rethought Account Service Model ## ♻️ Current situation & Problem Currently, `SepziAccount` exposes two distinct views: `Login` and `SignUp`. While visually identical (expect for customizations), the AccountService buttons navigate to the respective Login or SignUp views for data entry. This introduces several navigation steps for the user even for simple workflows (e.g. just signing in a returning user). Further, it is not clear where to place Identity Provider buttons (e.g., Sign in with Apple), as these create a new account or just sign you in to your existing one with the same, single button. ## 💡 Proposed solution This PR makes the following changes: * Unifying `Login` and `SignUp` views to be represented by a single view, placing identity providers on the same page as regular Account Services, the `AccountSetup` view. * Restructure the Account Service Model: * Make the necessary preparations to support third-party Identity providers * Introduce the concept of an `EmbeddableAccountService` and a new `KeyPasswordBasedAccountService` as new abstraction layers. This replaces the current `UsernamePasswordAccountService` and `EmailPasswordAccountService` implementations. Those we really only Mock implementations which provided extensibility through class inheritance. This was replaced by providing the same functionality with a hierarchy of protocols being more powerful and providing much more flexibility. Further, we introduce new Mock Account services (e.g., used by PreviewProviders) which more clearly communicate their use through their updated name. * Move the UI parts of an AccountService out into a distinct abstraction layer (`AccountSetupViewStyle`s). * Introduce the `Account Value` (through `AccountKey` implementations) abstraction to support arbitrary account values * They provide a name, a category (for optimized placement in the UI), and a initial value * The provide the UI components do display (`DataDisplayView`) and setup or edit (`DataEntryView`) account values * The provide easy to use accessors using extensions on `AccountKeys` and `AccountValues` * Optimized API surface for end-users: * Exposes the `signedIn` published property (same as previously) * Exposees the `details` shared repository to access the `AccountDetails` of the currently logged in use. This also provides access to the `AccountService` and its configuration that was used to setup the user account. * Provide a new `AccountOverview` view: * Allows to view arbitrary account values * Allows to edit arbitrary account values * Provide Logout and account Removal functionality * Enabled through StanfordSpezi/Spezi#72, one can easily configure `SpeziAccount` centrally while other configured components can provide their AccountServices via [Component Communication](https://swiftpackageindex.com/stanfordspezi/spezi/documentation/spezi/component#Communication). This PR tries to provide extensive documentation for all areas. To provide some insights into the new user-facing API surface. This is how you would configure Spezi Account now: ```swift class YourAppDelegate: SpeziAppDelegate { override var configuration: Configuration { AccountConfiguration(configuration: [ // the user defines what account values are used and if they are `required`, `collected` or just `supported` .requires(\.userId), .requires(\.password), .requires(\.name), .collects(\.dateOfBirth), .collects(\.genderIdentity) ]) } } ``` Below is an example on how to use the account setup view: ```swift struct MyOnboardingView: View { var body: some View { AccountSetup { NavigationLink { // ... next view if already signed in } label: { Text("Continue") } } } } ``` Lastly, the account overview is equally as easy to use: ```swift struct MyView: View { var body: some View { AccountOverview() } } ``` ## ⚙️ Release Notes * New, unified `AccountSetup` view * New `AccountOverview` view * Introduce new `AccountService` abstraction * Support arbitrary account values through `AccountKey`-based shared repository implementation ## ➕ Additional Information ### Related PRs and Issues * This PR provides the groundwork necessary for StanfordSpezi/SpeziFirebase#7 * The updated Firebase implementation: StanfordSpezi/SpeziFirebase#8 ### Testing Substantial UI tests were updated and added. ### Reviewer Nudging As this is quite a large PR it would make sense to start reviewing by reading through the documentation. First of all reviewing the documentation itself (structure and readability). Code example on the DocC article already explain a lot of the concepts. The areas that sparked your attention, where you think that something is off or hard to understand, I would recommend to deep dive into the respective code area. ### 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). --------- Co-authored-by: Paul Schmiedmayer <PSchmiedmayer@users.noreply.github.com>
Component communication
♻️ Current situation & Problem
Currently, there is no easy way for a
Component
to access features or information provided by anotherComponent
.I illustrate the usefulness using a real-world example we are currently facing with
SpeziAccount
. AAccountConfiguration
is used by the user to configure theSpeziAccount
subsystem which in turn does everything necessary to set up the requestAccountServices
to be accessible by the App's views. While one may just provide off-the-shelveAccountServices
using the initializer of the configuration object, there might be instances where an third-partyAccountService
is controlled and configured by aComponent
. In such cases it should be possible for the user to place theComponent
in theConfiguration
closure as usual with theAccountConfiguration
queryingAccountService
instances from all configuredComponents
.💡 Proposed solution
In this PR we introduce the
@Provide
and@Collect
property wrappers that can be used to pass around information or functionality betweenComponents
. The@Provide
property wrapper is used to supply a singleValue
(orValue?
if it e.g. only supplies it conditionally) or multiple values ([Value]
) to be collected by otherComponents
.Another
Component
can then use@Collect
with a type of[Value]
to collect all the provided items.Here is a short code example:
While the example shows the usage of a reference type, this feature can also easily be used with value types (though then, no modifications post the collection phase are possible).
⚙️ Release Notes
@Provide
and@Collect
property wrappers for easy to use inter-Component
communicationSharedRepository
pattern to easily implement typed collections.➕ Additional Information
Shared Repository Pattern
This PR additionally introduces the concept of the
SharedRepository
software pattern based on Buschmann et al. (Pattern-Oriented Software Architecture: A Pattern Language for Distributed Computing). We provide a variety of different types ofKnowledgeSource
s which are stored inSharedRepository
implementations. Each shared repository defines aSharedRepositoryAnchor
it expects from an implementingKnowledgeSource
.This instantiation of the shared repository pattern replaces the previous
TypedCollection
implementation. Further, it will be of equal use forSpeziAccount
(storing arbitrary account details). The current draft PR introduces a similar concept. This PR helps to generalize this software pattern and increase code reuse across the framework ecosystem.Alternative Considered
As with the previous
TypeCollection
implementation, aSharedRepository
allows for acollect(allOf:)
query which is, e.g., internally used to query allComponents
that conform toLifecycleHandler
. Instead of the proposed system, we could have created a property wrapper that just collects all components that conform to a given protocol. I decided against this approach considering the following points:@Dependency
system (or we would need to consider '@Collect` declarations in the execution order).Component
by default allowing for arbitrary information (value types, reference types, information, functionality).Related PRs
Testing
Test cases were adapted but still need to be extended for newly added code.
Reviewer Nudging
There are two separate parts to the PR:
SharedRepository
data structureCollect
andProvide
property wrappers. You may look at the tests to easily glance what their use case is.Code of Conduct & Contributing Guidelines
By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines: