Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
d4adc5a
Add lightning image
itsmeichigo Sep 16, 2021
5601042
Update IconsTests for lightning image
itsmeichigo Sep 16, 2021
b21d196
Add extension for UIViewController to configure offline banner
itsmeichigo Sep 16, 2021
2ee175f
Update navigation controller toolbar visibility only when view contro…
itsmeichigo Sep 16, 2021
fbf1c12
Merge branch 'issue/4384-connectivity-service' into issue/4384-offlin…
itsmeichigo Sep 16, 2021
686cf54
Update method name updateListener for updating the listener of connec…
itsmeichigo Sep 16, 2021
f55c281
Merge branch 'issue/4384-connectivity-service' into issue/4384-offlin…
itsmeichigo Sep 16, 2021
0e48831
Revert "Update method name updateListener for updating the listener o…
itsmeichigo Sep 16, 2021
895dc4c
Apply callout style for offline banner text
itsmeichigo Sep 16, 2021
decf9e4
Configure offline banner for dashboard screen
itsmeichigo Sep 16, 2021
415bd82
Configure offline banner for order flow
itsmeichigo Sep 16, 2021
0ead06f
Configure offline banner for review flow
itsmeichigo Sep 16, 2021
6607671
Configure offline banner for product flow
itsmeichigo Sep 16, 2021
f791ad5
Merge branch 'issue/4384-connectivity-service' into issue/4384-offlin…
itsmeichigo Sep 16, 2021
0bd6643
Update configuring offline banner in UIViewController extension
itsmeichigo Sep 16, 2021
45dc949
Add offline banner for shipping label form
itsmeichigo Sep 16, 2021
9d52ffe
Merge branch 'issue/4384-connectivity-service' into issue/4384-offlin…
itsmeichigo Sep 16, 2021
fb4de9a
Update release notes
itsmeichigo Sep 16, 2021
221eae9
Add statusPublisher to ConnectivityObserver
itsmeichigo Sep 17, 2021
37ab5b4
Update UIViewController+Connectivity extension to configure offline b…
itsmeichigo Sep 17, 2021
b9b5a85
Configure offline banner in WooNavigationController
itsmeichigo Sep 17, 2021
315001a
Update offline banner configuration in My Store
itsmeichigo Sep 17, 2021
edaf329
Update offline banner configuration in Review flow
itsmeichigo Sep 17, 2021
c54bce8
Rename connectivitySubscription to observeConnectivity
itsmeichigo Sep 17, 2021
d32c80c
Update offline banner configuration for order flow
itsmeichigo Sep 17, 2021
3c62701
Update offline banner configuration for products flow
itsmeichigo Sep 17, 2021
912d117
Flip lightning image for RTL languages
itsmeichigo Sep 17, 2021
c742b27
Merge branch 'develop' into issue/4384-offline-banners
itsmeichigo Sep 20, 2021
2eee06b
Update pathUpdateHandler to networkUpdateHandler to fix build failure
itsmeichigo Sep 20, 2021
263e532
Update function hasConfiguredOfflineBanner to shouldShowOfflineBanner…
itsmeichigo Sep 20, 2021
db4fcc6
Inject connectivity observer to WooNavigationControllerDelegate
itsmeichigo Sep 20, 2021
1ce862e
Add separate view for offline banner
itsmeichigo Sep 20, 2021
0033684
Use new view for offline banner
itsmeichigo Sep 20, 2021
7530df4
Replace isConnectivityAvailable with currentStatus for ConnectivityOb…
itsmeichigo Sep 22, 2021
06994f4
Remove offline banner configuration in view controllers
itsmeichigo Sep 22, 2021
c8ca97e
Handle offline banner configuration on WooNavigationControllerDelegate
itsmeichigo Sep 22, 2021
407f229
Update offline banner configuration by adding subview instead of navi…
itsmeichigo Sep 22, 2021
ebc44ef
Update unit tests for DeafultConnectivityObserver
itsmeichigo Sep 22, 2021
e739597
Update release notes
itsmeichigo Sep 22, 2021
cc626f8
Add missing listener in updateListener method of DefaultConnectivityO…
itsmeichigo Sep 22, 2021
cc2f90c
Merge branch 'develop' into issue/4384-offline-banners
itsmeichigo Sep 22, 2021
67aa0de
Update additionalSafeAreaInsets with extraBottomSpace when offline ba…
itsmeichigo Sep 22, 2021
4fabfb7
Revert "Update additionalSafeAreaInsets with extraBottomSpace when of…
itsmeichigo Sep 22, 2021
2408211
Remove updateListener method from ConnectivityObserver
itsmeichigo Sep 23, 2021
53456ce
Update unit tests for DefaultConnectivityObserver
itsmeichigo Sep 23, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion RELEASE-NOTES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

7.7
-----
- [*] Show banner on screens that use cached data when device is offline. [https://github.com/woocommerce/woocommerce-ios/pull/5000]


7.6
Expand All @@ -10,7 +11,6 @@
- [***] Shipping Labels: Merchants can now add new custom and service packages for shipping labels directly from the app. [https://github.com/woocommerce/woocommerce-ios/pull/4976]
- [*] Fix: when product image upload fails, the image cell stop loading. [https://github.com/woocommerce/woocommerce-ios/pull/4989]


7.5
-----
- [***] Merchants can now purchase shipping labels and declare customs forms for international orders. [https://github.com/woocommerce/woocommerce-ios/pull/4896]
Expand Down
6 changes: 6 additions & 0 deletions WooCommerce/Classes/Extensions/UIImage+Woo.swift
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,12 @@ extension UIImage {
return UIImage.gridicon(.menu)
}

/// Lightning icon on offline banner
///
static var lightningImage: UIImage {
return UIImage.gridicon(.offline).imageFlippedForRightToLeftLayoutDirection()
}

/// Invisible Image
///
static var invisibleImage: UIImage {
Expand Down
12 changes: 12 additions & 0 deletions WooCommerce/Classes/Extensions/UIViewController+Connectivity.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import UIKit
import Combine

extension UIViewController {
/// Defines if the view controller should show a "no connection" banner when offline.
/// This requires the view controller to be contained inside a `WooNavigationController`.
/// Defaults to `false`.
///
@objc var shouldShowOfflineBanner: Bool {
false
}
}
79 changes: 79 additions & 0 deletions WooCommerce/Classes/System/WooNavigationController.swift
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import Combine
import UIKit

/// Subclass to set Woo styling. Removes back button text on managed view controllers.
Expand Down Expand Up @@ -38,13 +39,25 @@ class WooNavigationController: UINavigationController {
///
private class WooNavigationControllerDelegate: NSObject, UINavigationControllerDelegate {

private let connectivityObserver: ConnectivityObserver
private var currentController: UIViewController?
private var subscriptions: Set<AnyCancellable> = []

init(connectivityObserver: ConnectivityObserver = ServiceLocator.connectivityObserver) {
self.connectivityObserver = connectivityObserver
super.init()
observeConnectivity()
}

/// Children delegate, all events will be forwarded to this object
///
weak var forwardDelegate: UINavigationControllerDelegate?

/// Configures the back button for the managed `ViewController` and forwards the event to the children delegate.
///
func navigationController(_ navigationController: UINavigationController, willShow viewController: UIViewController, animated: Bool) {
currentController = viewController
configureOfflineBanner(for: viewController)
configureBackButton(for: viewController)
forwardDelegate?.navigationController?(navigationController, willShow: viewController, animated: animated)
}
Expand Down Expand Up @@ -76,3 +89,69 @@ private extension WooNavigationControllerDelegate {
viewController.removeNavigationBackBarButtonText()
}
}

// MARK: Offline banner configuration
private extension WooNavigationControllerDelegate {

/// Observes changes in status of connectivity and updates the offline banner in current view controller accordingly.
///
func observeConnectivity() {
connectivityObserver.statusPublisher
.sink { [weak self] status in
guard let self = self, let currentController = self.currentController else { return }
self.configureOfflineBanner(for: currentController, status: status)
}
.store(in: &subscriptions)
}

/// Shows or hides offline banner based on the input connectivity status and
/// whether the view controller supports showing the banner.
///
func configureOfflineBanner(for viewController: UIViewController, status: ConnectivityStatus? = nil) {
if viewController.shouldShowOfflineBanner {
setOfflineBannerWhenNoConnection(for: viewController, status: status ?? connectivityObserver.currentStatus)
} else {
removeOfflineBanner(for: viewController)
}
}

/// Adds offline banner at the bottom of the view controller.
///
func setOfflineBannerWhenNoConnection(for viewController: UIViewController, status: ConnectivityStatus) {
// We can only show it when we are sure we can't reach the internet
guard status == .notReachable else {
return removeOfflineBanner(for: viewController)
}

// Only add banner view if it's not already added.
guard let navigationController = viewController.navigationController,
let view = viewController.view,
view.subviews.first(where: { $0 is OfflineBannerView }) == nil else {
return
Comment on lines +127 to +130
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we have the banner as a lazy var we could later do bannerView.superView == nil right?
I'm thinking that we could add the view to the navigation view but still modify the additionalSafeAreaInsets of the VC.

Would that work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of reusing the banner view, so I've tried your suggested solution but found a couple of issues:

  • Calling removeConstraints on all constraints of the banner causes the banner to lose even its constraints to its sub views (the image and title inside). So we may need to keep references to the constraints to parent view to remove them later.
  • Handling the constraints is tedious, so I thought about only adding the banner once and then showing / hiding it later. This introduces another issue about updating bottom constraint for view controller with / without the bottom bar. It's still necessary to keep reference to this bottom constraint.
  • Adding banner on navigation controller can also cause issue when swiping the navigation controller half way through before releasing:
    https://user-images.githubusercontent.com/5533851/134460006-b8b7ed0a-a831-4ed0-84a0-0a84a6adf26d.MP4

So I'll keep the current solution until finding a better one.

}

let offlineBannerView = OfflineBannerView(frame: .zero)
offlineBannerView.backgroundColor = .gray
offlineBannerView.translatesAutoresizingMaskIntoConstraints = false
view.addSubview(offlineBannerView)

let extraBottomSpace = viewController.hidesBottomBarWhenPushed ? navigationController.view.safeAreaInsets.bottom : 0
NSLayoutConstraint.activate([
offlineBannerView.heightAnchor.constraint(equalToConstant: OfflineBannerView.height),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is safe to do, would it all fit correctly if fonts grow bigger?

Probably better to get that height dynamically with systemLayoutFittingSize

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have created a separate issue here #5041 to handle it later.

offlineBannerView.leadingAnchor.constraint(equalTo: view.leadingAnchor),
offlineBannerView.trailingAnchor.constraint(equalTo: view.trailingAnchor),
offlineBannerView.bottomAnchor.constraint(equalTo: view.bottomAnchor, constant: -extraBottomSpace)
])
viewController.additionalSafeAreaInsets = UIEdgeInsets(top: 0, left: 0, bottom: OfflineBannerView.height, right: 0)
}

/// Removes the offline banner from the view controller if it exists.
///
func removeOfflineBanner(for viewController: UIViewController) {
guard let offlineBanner = viewController.view.subviews.first(where: { $0 is OfflineBannerView }) else {
return
}
offlineBanner.removeFromSuperview()
viewController.additionalSafeAreaInsets = .zero
}
}
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
import Foundation
import Combine

/// Interface for the observing connectivity
///
protocol ConnectivityObserver {
/// Getter for current state of the connectivity.
var isConnectivityAvailable: Bool { get }
var currentStatus: ConnectivityStatus { get }

/// Publisher for connectivity availability.
var statusPublisher: AnyPublisher<ConnectivityStatus, Never> { get }

/// Starts the observer.
func startObserving()

/// Updates the listener for the connectivity observer.
func updateListener(_ listener: @escaping (ConnectivityStatus) -> Void)

/// Stops the observer.
func stopObserving()
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import Foundation
import Combine
import Network

final class DefaultConnectivityObserver: ConnectivityObserver {
Expand All @@ -8,32 +8,27 @@ final class DefaultConnectivityObserver: ConnectivityObserver {
private let networkMonitor: NetworkMonitoring
private let observingQueue: DispatchQueue = .global(qos: .background)

var isConnectivityAvailable: Bool {
if case .reachable = connectivityStatus(from: networkMonitor.currentNetwork) {
return true
}
return false
@Published private(set) var currentStatus: ConnectivityStatus = .unknown

var statusPublisher: AnyPublisher<ConnectivityStatus, Never> {
$currentStatus.eraseToAnyPublisher()
}

init(networkMonitor: NetworkMonitoring = NWPathMonitor()) {
self.networkMonitor = networkMonitor
startObserving()
}

func startObserving() {
networkMonitor.start(queue: observingQueue)
}

func updateListener(_ listener: @escaping (ConnectivityStatus) -> Void) {
networkMonitor.networkUpdateHandler = { [weak self] path in
guard let self = self else { return }
let connectivityStatus = self.connectivityStatus(from: path)
DispatchQueue.main.async {
listener(connectivityStatus)
self.currentStatus = self.connectivityStatus(from: path)
}
}
}

func startObserving() {
networkMonitor.start(queue: observingQueue)
}

func stopObserving() {
networkMonitor.cancel()
}
Expand Down Expand Up @@ -65,8 +60,6 @@ final class DefaultConnectivityObserver: ConnectivityObserver {

/// Proxy protocol for mocking `NWPathMonitor`.
protocol NetworkMonitoring: AnyObject {
var currentNetwork: NetworkMonitorable { get }

/// A handler that receives network updates.
var networkUpdateHandler: ((NetworkMonitorable) -> Void)? { get set }

Expand All @@ -88,10 +81,6 @@ protocol NetworkMonitorable {

extension NWPath: NetworkMonitorable {}
extension NWPathMonitor: NetworkMonitoring {
var currentNetwork: NetworkMonitorable {
currentPath
}

var networkUpdateHandler: ((NetworkMonitorable) -> Void)? {
get {
let closure: ((NetworkMonitorable) -> Void)? = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,10 @@ final class DashboardViewController: UIViewController {
super.viewDidLayoutSubviews()
dashboardUI?.view.frame = containerView.bounds
}

override var shouldShowOfflineBanner: Bool {
return true
}
}

// MARK: - Configuration
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@ final class OrderDetailsViewController: UIViewController {
super.viewDidLayoutSubviews()
tableView.updateHeaderHeight()
}

override var shouldShowOfflineBanner: Bool {
return true
}
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,16 @@ final class ReviewOrderViewController: UIViewController {
super.viewDidLayoutSubviews()
tableView.updateFooterHeight()
}

override var shouldShowOfflineBanner: Bool {
return true
}
}

// MARK: - UI Configuration
//
private extension ReviewOrderViewController {

func configureViewModel() {
viewModel.configureResultsControllers { [weak self] in
self?.tableView.reloadData()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ final class ShippingLabelFormViewController: UIViewController {
registerTableViewHeaderFooters()
observeViewModel()
}

override var shouldShowOfflineBanner: Bool {
return true
}
}

// MARK: - View Configuration
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ final class OrdersRootViewController: UIViewController {
func presentDetails(for note: Note) {
ordersViewController.presentDetails(for: note)
}

override var shouldShowOfflineBanner: Bool {
return true
}
}

// MARK: - Configuration
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,10 @@ final class ProductFormViewController<ViewModel: ProductFormViewModelProtocol>:
view.endEditing(true)
}

override var shouldShowOfflineBanner: Bool {
return true
}

// MARK: - Navigation actions handling

override func shouldPopOnBackButton() -> Bool {
Expand Down Expand Up @@ -403,6 +407,7 @@ final class ProductFormViewController<ViewModel: ProductFormViewModelProtocol>:
// MARK: - Configuration
//
private extension ProductFormViewController {

func configureNavigationBar() {
updateNavigationBar()
updateBackButtonTitle()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,10 @@ final class ProductsViewController: UIViewController {

updateTableHeaderViewHeight()
}

override var shouldShowOfflineBanner: Bool {
return true
}
}

// MARK: - Navigation Bar Actions
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import UIKit

/// Gray banner showing message when device is offline.
///
final class OfflineBannerView: UIView {

static let height: CGFloat = 44

override init(frame: CGRect) {
super.init(frame: frame)
setupView()
}

required init?(coder: NSCoder) {
fatalError("init(coder:) has not been implemented")
}

private func setupView() {
let stackView = UIStackView()
stackView.axis = .horizontal
stackView.spacing = 3
stackView.distribution = .fillProportionally
stackView.alignment = .center
stackView.translatesAutoresizingMaskIntoConstraints = false

let imageView = UIImageView(image: .lightningImage)
imageView.tintColor = .white
imageView.contentMode = .scaleAspectFit
imageView.translatesAutoresizingMaskIntoConstraints = false
NSLayoutConstraint.activate([
imageView.widthAnchor.constraint(equalToConstant: 24),
imageView.heightAnchor.constraint(equalToConstant: 24)
])

let messageLabel = UILabel()
messageLabel.text = NSLocalizedString("Offline - using cached data", comment: "Message for offline banner")
messageLabel.applyCalloutStyle()
messageLabel.textColor = .white
messageLabel.translatesAutoresizingMaskIntoConstraints = false

stackView.addArrangedSubview(imageView)
stackView.addArrangedSubview(messageLabel)

addSubview(stackView)
NSLayoutConstraint.activate([
stackView.safeLeadingAnchor.constraint(greaterThanOrEqualTo: safeLeadingAnchor, constant: 0),
stackView.safeBottomAnchor.constraint(greaterThanOrEqualTo: safeBottomAnchor, constant: 0),
stackView.centerXAnchor.constraint(equalTo: centerXAnchor),
stackView.centerYAnchor.constraint(equalTo: centerYAnchor)
])
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import Foundation
import UIKit
import Yosemite
import Gridicons
Expand Down Expand Up @@ -77,6 +76,10 @@ final class ReviewDetailsViewController: UIViewController {
super.viewWillAppear(animated)
markAsReadIfNeeded(notification)
}

override var shouldShowOfflineBanner: Bool {
return true
}
}


Expand Down
Loading