andreamazz / AMScrollingNavbar

Scrollable UINavigationBar that follows the scrolling of a UIScrollView
MIT License
6.05k stars 634 forks source link

Removed recursive subview hiding. #368

Open evil159 opened 5 years ago

evil159 commented 5 years ago

Fix for #367.

Motivation

After investigating this and testing on iOS's 8-12 I could not observe that recursive subview actually does anything(except causing flickering on iOS 11 and 12 :).

Bar button items(including those with custom views) are handled by these:

    navigationItem.leftBarButtonItem?.tintColor = navigationItem.leftBarButtonItem?.tintColor?.withAlphaComponent(alpha)
    navigationItem.rightBarButtonItem?.tintColor = navigationItem.rightBarButtonItem?.tintColor?.withAlphaComponent(alpha)
    navigationItem.leftBarButtonItems?.forEach { $0.tintColor = $0.tintColor?.withAlphaComponent(alpha) }
    navigationItem.rightBarButtonItems?.forEach { $0.tintColor = $0.tintColor?.withAlphaComponent(alpha) }
...
    // Hide the left items
    navigationItem.leftBarButtonItem?.customView?.alpha = alpha
    navigationItem.leftBarButtonItems?.forEach { $0.customView?.alpha = alpha }

    // Hide the right items
    navigationItem.rightBarButtonItem?.customView?.alpha = alpha
    navigationItem.rightBarButtonItems?.forEach { $0.customView?.alpha = alpha }

Title and title view handled here:

    navigationItem.titleView?.alpha = alpha
   ...
    if let titleColor = navigationBar.titleTextAttributes?[NSAttributedString.Key.foregroundColor] as? UIColor {
      navigationBar.titleTextAttributes?[NSAttributedString.Key.foregroundColor] = titleColor.withAlphaComponent(alpha)
    } else {
      let blackAlpha = UIColor.black.withAlphaComponent(alpha)
      if navigationBar.titleTextAttributes == nil {
        navigationBar.titleTextAttributes = [NSAttributedString.Key.foregroundColor: blackAlpha]
      } else {
        navigationBar.titleTextAttributes?[NSAttributedString.Key.foregroundColor] = blackAlpha
      }
    }

Any other "tintable" stuff handled here

    navigationBar.tintColor = navigationBar.tintColor.withAlphaComponent(alpha)

It seems that all cases are covered already without the need to traverse navigation bar view hierarchy updating alpha for each of them(which also has it's problems).

Issue with recursive alpha

As the documentation for UIView.alpha states

transparency imparted by that alpha value affects all of the view's contents, including its subviews

Which results in alpha value of a view being multiplied by alpha of its superview(and superview of superview and so on). In this illustration you can see what I'm trying to describe:

Screenshot 2019-07-31 at 14 44 47

The first square consists of a white view with alpha 0.5 and a black subview with alpha 0.5. Square two and three are just for comparison - the first one is a black view with alpha 0.25, second one - 0.5. This shows that recursively applying alpha produces result different from desired - in an attempt to set everything to 0.5 - the result ended up looking as if it has alpha of value 0.25. And this is in controlled environment - view hierarchy depth is limited to 2 in this example, this should be more pronounced when nesting is deeper, the alpha of the deepest view should be desired_alpha^depth.

Here's playground with this setup: RecursiveAlphaPlayground.playground.zip

andreamazz commented 5 years ago

Hi @evil159 Thanks for pointing that out, I remember this being added in another PR a while ago, as a fix for the issue you mentioned, #257 I'm unable to test it thoroughly now, can you confirm that the flicker is gone and doesn't introduce regressions?