Feature: Dim background #18
Conversation
| @@ -69,6 +75,31 @@ open class CascadePopupMenu @JvmOverloads constructor( | |||
| popup.contentView.background = it | |||
| } | |||
|
|
|||
| styler.overlayColor()?.let { | |||
| check(context is Activity) { "Activity context is required in order to add an overlay view" } | |||
| val container = (context.window.decorView as ViewGroup) | |||
There was a problem hiding this comment.
I think an activity context isn't needed. Does anchor.rootView work here?
There was a problem hiding this comment.
anchor.rootView does work.
| styler.overlayColor()?.let { | ||
| check(context is Activity) { "Activity context is required in order to add an overlay view" } | ||
| val container = (context.window.decorView as ViewGroup) | ||
| val overlay = View(context, null, 0).apply { |
There was a problem hiding this comment.
Let's use the single param constructor?
| container.addView(overlay) | ||
|
|
||
| val alphaInAnimator = ObjectAnimator.ofFloat(overlay, View.ALPHA, 1f).apply { | ||
| duration = 500 |
There was a problem hiding this comment.
It's important to sync these two animations here PopupWindow's transition. See popup.enterTransition and popup.exitTransition and reuse their duration and interpolator?
There was a problem hiding this comment.
enterTransition & exitTransition are only available from Nougat. Have added default fallback values for animation properties.
| alphaInAnimator.start() | ||
|
|
||
| popup.setOnDismissListener { | ||
| val alphaOutAnimator = ObjectAnimator.ofFloat(overlay, View.ALPHA, 0f) |
There was a problem hiding this comment.
Any reason for not using overlay.animate().alpha(0f) instead of an ObjectAnimator?
| @@ -52,7 +57,8 @@ open class CascadePopupMenu @JvmOverloads constructor( | |||
| val background: () -> Drawable? = { null }, | |||
| val menuList: (RecyclerView) -> Unit = {}, | |||
| val menuTitle: (MenuHeaderViewHolder) -> Unit = {}, | |||
| val menuItem: (MenuItemViewHolder) -> Unit = {} | |||
| val menuItem: (MenuItemViewHolder) -> Unit = {}, | |||
| val overlayColor: () -> Int? = { null } | |||
There was a problem hiding this comment.
Should there be a flag to enable this particular feature or just check the nullability of resource value?
Hmmmm a nullable Int looks good to me 👍. I'd suggest two things:
- Rename
overlayColortobackgroundDimColor. - Remove the lambda type. A color integer doesn't need to be lazily evaluated. It was necessary for the
backgroundDrawable to ensure a new instance is created on each lambda call because Drawables contain shared mutable state.
…ity context for `decorView`
… visibility changes
|
Potentially should be fixed in #45 |
Fixes #3
Added additional support for any color instead of just one.
Question: Should there be a flag to enable this particular feature or just check the nullability of resource value?