Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed recursive subview hiding. #368

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Removed recursive subview hiding. #368

wants to merge 1 commit into from

Conversation

evil159
Copy link

@evil159 evil159 commented Jul 31, 2019

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
Copy link
Owner

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants