feat(ErrorBanner): offline banner #478
Conversation
KaylaBrady
force-pushed
the
kb-offline
branch
from
835415e to
8723045
Compare
| @@ -46,6 +46,12 @@ struct ErrorBanner: View { | |||
| } | |||
| .frame(minHeight: minHeight) | |||
| } | |||
| case .networkError: | |||
| ErrorCard { HStack { | |||
There was a problem hiding this comment.
Is the default alignment in this HStack not center? It looks kinda like the image is below the centerline of the text.
There was a problem hiding this comment.
Added some borders to each view - I think looks it is centered when accounting for the full line height of the text.
I swapped in HStack(alignment: .center) and didn't notice a difference
There was a problem hiding this comment.
Okay yeah, I think it's just a matter of the icon being slightly bottom heavy. Thanks for checking!
| @@ -14,6 +14,6 @@ internal fun createDataStore(context: Context): DataStore<Preferences> = | |||
| fun initKoin(appVariant: AppVariant, nativeModule: Module, context: Context) { | |||
| startKoin { | |||
| androidContext(context) | |||
| modules(appModule(appVariant) + platformModule() + nativeModule) | |||
| modules(platformModule() + appModule(appVariant) + nativeModule) | |||
There was a problem hiding this comment.
Just curious, but why were these swapped?
There was a problem hiding this comment.
Oh this isn't actually necessary. At first I tried having ErrorBannerStateRepo call subscribeToNetworkStatusChanges on init, but that was causing dependency injection issues with BeanNotFound errors. I thought maybe the platformModule needed to be called first here since appModule depends on it, but that didn't end up fixing the issue, and I just opted to call subscribeToNetworkStatusChanges from the ViewModel instead.
| case .networkError: | ||
| ErrorCard { HStack { | ||
| Image(systemName: "wifi.slash") | ||
| Text("Unable to connect") |
There was a problem hiding this comment.
Because this has no reload button like the other two banners, it also has no Spacer(), so we're getting the awkward squished banner. If you toss a Spacer() at the end here, I think that'll get it to take up the full width.
| Image(systemName: "wifi.slash") | ||
| Text("Unable to connect") | ||
| } | ||
| } |
There was a problem hiding this comment.
nitpick: I'd put these braces on the same line 😛
| import shared | ||
| import XCTest | ||
|
|
||
| final class ErrorBannerViewModelTests: XCTestCase { |
This reverts commit 10d2bad.
Summary
Ticket: Offline error banner
What is this PR for?
This PR adds an error banner that appears when a user is offline.
I used the following resources as a basis for this implementation:
I drifted from the Blog post's approach in a few ways:
Testing
What testing have you done?
I included the android implementation from the blog post but did not test it.
