Bug: UIKit-applied dimming made permanent by call to showNavbar()

[tifroz]

When the keyboard pops up (or a UIAlert), the temporary UIKit-applied dimming is made permanent by windowDidBecomeVisible() > showNavbar() > updateNavbarAlpha()

At ScrollingNavigationControllerDelegateline 653 updateNavbarAlpha() navigationBar.tintColor = navigationBar.tintColor.withAlphaComponent(alpha)

....the proposed fix skips the call to showNavbar() when the right conditions are detected

[andreamazz]

Hi @tifroz Thanks for the PR. I'm not a fan of hardcoding the UIRemoteKeyboardWindow string, let me check if there is an alternative way first.

[tifroz]

In addition the hardcoding of the string UIRemoteKeyboardWindow, the fix falls short in some respect. For example, during the sequence below the dimming will be persisted:

1. Pop up keyboard: windowDidBecomeVisible() called but the fix saves us & dimming is not persisted)
2. Rotate device while the keyboard is still up: windowDidBecomeVisible() called again, showNavbar(), updateNavbarAlpha () are called ...and the dimming caused by the Keyboard is persisted.

A better candidate might be to move the logic out of windowDidBecomeVisible() and into viewDidAppear() (there may still be corner cases that prove problematic?)

Another candidate would be to track events that cause UIKit to dim the navigation bar so we can maintain a uikitDimmingApplied boolean variable that we can use to be smarter about persisting the tintColor in updateNavbarAlpha(). UIKeyboardWillShowNotification / UIKeyboardDidHideNotification can be used for the keyboard. I am not sure UIAlerts events can be tracked via a global event, but we can figure out the state using a code like below (shamelessly copied from SO)

-(BOOL) doesAlertViewExist {
    if ([[UIApplication sharedApplication].keyWindow isMemberOfClass:[UIWindow class]])
    {
        return NO;//AlertView does not exist on current window
    }
    return YES;//AlertView exist on current window
}

...happy to further investigate & offer a PR for either candidate after you express a preference (if any)

[tifroz]

I am not sure about the rationale for calling showNavbar() from windowDidBecomeVisible() instead of viewDidAppear() - but I know that moving the logic into viewDidAppear() won't entirely fix the issue, because other events can call updateNavbarAlpha() while the UIKit dimming is applied. For example, a device rotate will call viewWillTransition() > showNavbar() > updateNavbarAlpha () when a keyboard or UIAlert are up, so the dimming will be made permanent under this scenario no matter what we do with windowDidBecomeVisible()

Tracking the state via a uikitDimmingApplied seems like the only thorough way to address the problem (see my previous message for details).

Are you seeing any obvious issues with this approach? If not I am happy to work on a revised PR

[andreamazz]

Hi @tifroz sorry for the delay, weird times.

I'm a bit foggy on the rationale behind the windowDidBecomeVisible, after some digging I found that it was a fix for #321 It's an edge case, but I worry that removing it might introduce a regression.

[tifroz]

The last few days I have been testing an approach that has been working pretty well for me:

1. In followScrollView() we save the bar tint color in a new member variable savedNavBarTintColor
2. In updateNavbarAlpha() we use the saved color as the reference color, e.g. navigationBar.tintColor = savedNavBarTintColor?.withAlphaComponent(alpha)

This way we never inadvertently capture the dimmed tint occasionally applied by UIKit

3. Ideally we should probably provide a method API to change the value of savedNavBarTintColor: it should be infrequent, but apps that make tint color changes as a normal course of business will need to notify AMScrollingNavBar of the new tint color.

Happy to provide the PR if you think this works? Just let me know

[andreamazz]

Hey @tifroz That sounds good to me, if you can prepare a PR I'll be happy to merge.

[andreamazz]

The other PR is merged, closing this one.

[tifroz]

I found other corner cases where the didBecomeVisibleNotification event is fired and triggers showNavBar at times when it's not useful or appropriate (in a recent situation I have seen the event fire when performing an unrelated animation on a sub-view controller - I have no idea what iOS would trigger the vent in that case, but it does apparently).

It's becoming increasingly clear to me that trying to figure out what should be done in response to didBecomeVisibleNotification is likely a fools errand because so many things can cause the event to fire. I think it would be best to move the logic in windowDidBecomeVisible() into viewWillAppear() and let apps directly handle edge cases themselves by calling showNavBar() directly.

We have had so many iterations on this, I would understand if you'd rather call it a day. Otherwise I am happy to submit a new PR after I have done sufficient testing.