-
Notifications
You must be signed in to change notification settings - Fork 283
Task/14277 Add new consent screen and os confirmation #4879
Task/14277 Add new consent screen and os confirmation #4879
Conversation
src/xcode/ENA/ENA/Source/Scenes/ExposureSubmission/ExposureSubmissionCoordinator.swift
Outdated
Show resolved
Hide resolved
...A/ENA/Resources/Assets/Assets.xcassets/Common/SRS-No-Certificate-icon.imageset/Contents.json
Outdated
Show resolved
Hide resolved
...ode/ENA/ENA/Resources/Assets/Assets.xcassets/Common/SRS-Positive-icon.imageset/Contents.json
Outdated
Show resolved
Hide resolved
.../ENA/ENA/Resources/Assets/Assets.xcassets/Common/SRS-Warn-Others-icon.imageset/Contents.json
Outdated
Show resolved
Hide resolved
src/xcode/ENA/ENA/Resources/Localization/de.lproj/Localizable.legal.strings
Outdated
Show resolved
Hide resolved
src/xcode/ENA/ENA/Resources/Localization/de.lproj/Localizable.strings
Outdated
Show resolved
Hide resolved
src/xcode/ENA/ENA/Source/Scenes/ExposureSubmission/ExposureSubmissionCoordinator.swift
Outdated
Show resolved
Hide resolved
...A/Source/Scenes/ExposureSubmission/View/Controller/SRSConsent/SRSConsentViewController.swift
Show resolved
Hide resolved
...A/Source/Scenes/ExposureSubmission/View/Controller/SRSConsent/SRSConsentViewController.swift
Show resolved
Hide resolved
...A/Source/Scenes/ExposureSubmission/View/Controller/SRSConsent/SRSConsentViewController.swift
Outdated
Show resolved
Hide resolved
...A/Source/Scenes/ExposureSubmission/View/Controller/SRSConsent/SRSConsentViewController.swift
Show resolved
Hide resolved
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.
Great, just a few minor hints or questions.
|
||
|
||
private func makeSRSConsentScreen(srsFlowType: SRSFlowType) -> SRSConsentViewController { | ||
return SRSConsentViewController { [weak self] isLoading in |
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.
return
not needed
|
||
private func makeSRSConsentScreen(srsFlowType: SRSFlowType) -> SRSConsentViewController { | ||
return SRSConsentViewController { [weak self] isLoading in | ||
guard let self = self else { return } |
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.
Can we use guard let self else { return }
from Swift 5.7
in the meanwhile? (Pipeline)
|
||
// MARK: - Private | ||
|
||
private let viewModel = SRSConsentViewModel() |
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.
Your solution is completely fine.
I would use that as the default argument from the constructor, to make it overwritable.
init(viewModel: SRSConsentViewModel = .init()) { ... }
...NA/ENA/Source/Scenes/ExposureSubmission/View/Controller/SRSConsent/SRSConsentViewModel.swift
Show resolved
Hide resolved
|
||
return points | ||
} | ||
} |
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.
Very clean code, awesome!!
…n' of github.com:corona-warn-app/cwa-app-ios into feature/14277-Add-new-consent-screen-and-OS-confirmation
Description
Link to Jira
https://jira-ibs.wbs.net.sap/browse/EXPOSUREAPP-14277
Screenshots