-
Notifications
You must be signed in to change notification settings - Fork 1
feat/replace-images-with-abm #1068
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
Conversation
… one ABCircleAndLoader everywhere
…): add common ProgressFlowAB View with correct margins
…handle "mobile&fixed" and "only fixed" as two separate processes to prevent simultaneous handling of fixed and mobile
mturnau
left a comment
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.
generally I approve ;)
| } | ||
|
|
||
| var syncImage: some View { | ||
| Image(airbeamImage) |
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.
not very important but is there sth like alt text for ios? like accessibility desc or sth
| self.abImage = image(airbeamImageAsset) | ||
| self.continueButtonOnClickDestination = nil | ||
| self.continueButtonOnClick = continueButtonOnClick | ||
| } |
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.
Another minor remark: Is there a way to dry it out? not sure if this is the way to go in Swift (having multiple inits, even if they are v. similar). Maybe you can make structs or whatever like
enum ABImageSource {
case view(AnyView)
case asset(String)
}
enum ContinueAction {
case navigation(AnyView)
case action(@MainActor () -> Void)
case none
}
and then have one init like
init(progress: Float, abImageSource: ABImageSource, title: String, message: String, continueAction: ContinueAction = .none
) {
self.progress = progress
self.abImageSource = abImageSource
self.title = title
self.message = message
self.continueAction = continueAction
}
but possibly I'm thinking in over languages ;)
| let title = if viewModel.isDownloadingFinished { | ||
| Strings.SyncingABView.finishingSyncTitle | ||
| } else if let progressTitle { | ||
| "Syncing \(progressTitle.lowercased()) \(progressCount ?? "")" |
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.
are't we keeping strings somewhere separate
Replace AB3 images in the app with ABM
Correct margins on UI screens with ABM images
Refactor and unify UI screens with ABM images under common ProgressFlowAB view
Fix issue with SD sync failing sometimes when having both Fixed and Mobile SD card data