Feature: Dim background#18
Conversation
|
|
||
| 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?
| 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 |
3 participants
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?