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

UIVibrancyEffect designable #245

Merged
merged 7 commits into from
Jul 17, 2016
Merged

UIVibrancyEffect designable #245

merged 7 commits into from
Jul 17, 2016

Conversation

tbaranes
Copy link
Member

@tbaranes tbaranes commented Jul 14, 2016

I started the implementation of vibrancy designable (close #243), up to now it's working well but I have one issue which deserve more thought.

The vibrancy is applied to its subviews while the blur is applied to everything below it. While using in interface builder, the blur is working well since we don't have to think about which view is parent of who, but for the vibrancy we have to know who will be its subviews. Since we can't use two different views to differentiate the vibrancy and the blur (they are fully related and can't work independently), how to add the subviews which will have the vibrancy effect applied?

The only option I find until now is to consider that all the subviews of the AnimatableView will be removed and then added to the vibrancyView which means, all the subviews will have a vibrancy without exception. That could be a good solution, but I'm expecting incomprehension when using the feature.
That option can be a bit extended by considering adding an applyVibrancy boolean to each subviews, but I'm not really fan of this.

Beside that, it's working well:

screen shot 2016-07-14 at 15 31 00

The left image has been added programatically to the vibrancyView whereas the right one is a classic subview.

@JakeLin @f1nality Any suggestions about this issue?
@f1nality Is that what you were expecting or did you have another idea in mind?

@f1nality
Copy link

f1nality commented Jul 14, 2016

@tbaranes Yes, this is exactly what I was looking for. Adding all subviews to vibrancyView doesn't look good. I don't see anything bad in adding an applyVibrancy boolean, what's wrong with it? Btw will this work for nested views (ex UILabel inside UIView which will be added to vibrancyView)?

@tbaranes
Copy link
Member Author

tbaranes commented Jul 14, 2016

I don't see anything bad in adding an applyVibrancy boolean

That means each subviews which will have a vibrancy applied will have be an VibrancyView which conform to DesignableVibrancy and set the boolean. It's a bit complex to use. I doesn't really like the idea to add all the subviews, but it's just easier to use. Anyway, both have pro and cons, I'd like to find a third options, otherwise it will be up to you since you ask for that feature 👍

Btw will this work for nested views (ex UILabel inside UIView which will be added to vibrancyView)?

Just tried it by applying the vibrancy on all subviews, and it seems to work!

screen shot 2016-07-14 at 16 15 11

private func configInspectableProperties() {
configAnimatableProperties()
configTintedColor()
configBlurEffectStyle()
Copy link
Member

Choose a reason for hiding this comment

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

Do we need that any more? Since we have called it in didSet

@JakeLin
Copy link
Member

JakeLin commented Jul 17, 2016

Both solutions have pros and cons.

I like the apply to all subviews solution more because

  1. vibrancyViews can only be used in a blurView. If we provide applyVibrancy boolean for the subview, the user make set it to true but this view is not in a blurView.
  2. I think in most of cases, we will either apply apply vibrancy to all subviews or none. If the user wants to apply some but not all. They may need to use them in code.
  3. The implementation for DesignableVibrancy is quiet complicated and looks like a little bit over engineering.

@@ -10,7 +10,7 @@ public protocol BlurDesignable {
blur effect style: `ExtraLight`, `Light` or `Dark`
*/
var blurEffectStyle: String? { get set }

var vibrancyEffectStyle: String? { get set }
Copy link
Member

Choose a reason for hiding this comment

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

May add a comment here.

/**
  Vibrancy effect style: `ExtraLight`, `Light` or `Dark`. Once specify the Vibrancy effect style, all subviews will apply this vibrancy effect.
 */

@JakeLin
Copy link
Member

JakeLin commented Jul 17, 2016

@tbaranes nice work 👍, please have a look my comments.

@f1nality Thanks for the request and involving into this PR, please let me know if you have any question or have a better solution. I can't figure out any solution better than @tbaranes 's

@tbaranes
Copy link
Member Author

tbaranes commented Jul 17, 2016

All comments have been treated 👍
I also updated the documentation, it's ready to go on my side. Your call @JakeLin!

BuddyBuild is failing again. Is it really useful?

@JakeLin JakeLin merged commit 43a3131 into master Jul 17, 2016
@JakeLin
Copy link
Member

JakeLin commented Jul 17, 2016

@tbaranes I have contacted BuddyBuild again to fix it. There are some issues with singing.

@JakeLin JakeLin deleted the feature/designable_vibrancy branch July 17, 2016 11:44
@JakeLin JakeLin modified the milestones: 2.6, 2.5 Jul 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UIVibrancyEffect designable?
3 participants