woocommerce / woocommerce-ios

WooCommerce iOS app
https://www.woocommerce.com/mobile
GNU General Public License v2.0
301 stars 109 forks source link

[Woo POS] Improvement: Extract `updateSharedConfiguration` to separate methods #13679

Closed iamgabrielma closed 1 month ago

iamgabrielma commented 2 months ago

Follow-up from https://github.com/woocommerce/woocommerce-ios/pull/13670#discussion_r1724646148

I think we'd have problems if there was no tabBarController, but strictly there's no reason that this should prevent us from disabling the in app notifications – perhaps this should become an if let, or separated into another function.

When we switch modes between POS and App, we make some changes in the shared app delegate regarding the main tab and in-app notifications. If the initial guard does not pass, it would also make the notifications not work as expected, but technically there's no reason this should happen as both are separate bits of logic, they just happen when we switch modes.

It would be good to add some logs as well, just for support reference.

// MARK: - Helper method for WooCommerce POS
//
extension AppDelegate {
    func updateSharedConfiguration(_ isPointOfSaleActive: Bool) {
        // Show/hide app's tab bars
        guard let tabBarController = AppDelegate.shared.tabBarController else {
            return
        }
        tabBarController.tabBar.isHidden = isPointOfSaleActive
        tabBarController.selectedViewController?.view.layoutIfNeeded()

        // Enable/disable foreground in-app notifications
        if isPointOfSaleActive {
            ServiceLocator.pushNotesManager.disableInAppNotifications()
        } else {
            ServiceLocator.pushNotesManager.enableInAppNotifications()
        }
    }
}

We could simply do something like:

func updateSharedConfiguration(_ isPointOfSaleActive: Bool) {
    updateTabBarVisibility(isPointOfSaleActive)
    updateInAppNotifications(isPointOfSaleActive)
}
dangermattic commented 2 months ago

Thanks for reporting! 👍