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

Add flag to dim background behind popup #43

Merged
merged 1 commit into from
Apr 15, 2019
Merged

Add flag to dim background behind popup #43

merged 1 commit into from
Apr 15, 2019

Conversation

Tunous
Copy link
Contributor

@Tunous Tunous commented Apr 13, 2019

Can be enabled via style attribute android:backgroundDimEnabled and customized with android:backgroundDimAmount where amount can be a float number betwen 0.0 and 1.0.

Example

@@ -89,6 +89,12 @@ class MaterialRecyclerViewPopupWindow(

private val contextThemeWrapper: Context

private val windowManager: WindowManager = context.getSystemService(Context.WINDOW_SERVICE) as WindowManager
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe initialize this lazily via by lazy { } since this is only used if dimming is enabled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. Done.

@@ -105,6 +111,8 @@ class MaterialRecyclerViewPopupWindow(

dropDownHorizontalOffset = a.getDimensionPixelOffset(androidx.appcompat.R.styleable.ListPopupWindow_android_dropDownHorizontalOffset, 0)
Copy link
Owner

Choose a reason for hiding this comment

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

Since we add a new styleable I think it would be better not to mix androidx.appcompat.R.styleable.ListPopupWindow_XXX and R.styleable.ListPopupWindow_ which might be a bit confusing.
Therefore could you change:

val a = context.obtainStyledAttributes(null, androidx.appcompat.R.styleable.ListPopupWindow, 0, defStyleRes)

dropDownHorizontalOffset = a.getDimensionPixelOffset(androidx.appcompat.R.styleable.ListPopupWindow_android_dropDownHorizontalOffset, 0)
dropDownVerticalOffset = a.getDimensionPixelOffset(androidx.appcompat.R.styleable.ListPopupWindow_android_dropDownVerticalOffset, 0)

to:

val a = context.obtainStyledAttributes(null, R.styleable.ListPopupWindow, 0, defStyleRes)

dropDownHorizontalOffset = a.getDimensionPixelOffset(R.styleable.ListPopupWindow_android_dropDownHorizontalOffset, 0)
dropDownVerticalOffset = a.getDimensionPixelOffset(R.styleable.ListPopupWindow_android_dropDownVerticalOffset, 0)
       

please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds reasonable. I've went a bit further and also renamed the styleable to MaterialRecyclerViewPopupWindow so it'll match the name of class.

@@ -105,6 +111,8 @@ class MaterialRecyclerViewPopupWindow(

dropDownHorizontalOffset = a.getDimensionPixelOffset(androidx.appcompat.R.styleable.ListPopupWindow_android_dropDownHorizontalOffset, 0)
dropDownVerticalOffset = a.getDimensionPixelOffset(androidx.appcompat.R.styleable.ListPopupWindow_android_dropDownVerticalOffset, 0)
backgroundDimEnabled = a.getBoolean(R.styleable.ListPopupWindow_android_backgroundDimEnabled, false)
backgroundDimAmount = a.getFloat(R.styleable.ListPopupWindow_android_backgroundDimAmount, 0.3f)
Copy link
Owner

Choose a reason for hiding this comment

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

Could you move 0.3f to a constant inside the companion object e.g. private const val DEFAULT_BACKGROUND_DIM_AMOUNT = 0.3f?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@@ -359,4 +371,12 @@ class MaterialRecyclerViewPopupWindow(

return menuWidth
}

private fun addBackgroundDimming() {
Copy link
Owner

Choose a reason for hiding this comment

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

Cool, didn't know that adding a dim can be done with few lines of code like this!

@zawadz88 zawadz88 added this to the 3.3.0 milestone Apr 15, 2019
Can be enabled via style attribute `android:backgroundDimEnabled` and customized with `android:backgroundDimAmount` where amount can be a float number betwen 0.0 and 1.0.
@Tunous
Copy link
Contributor Author

Tunous commented Apr 15, 2019

I've made also an additional change and moved the new styles to sample module since they aren't used in library.

@zawadz88 zawadz88 merged commit 0e60529 into zawadz88:master Apr 15, 2019
@Tunous Tunous deleted the feature/background-dim branch April 15, 2019 15:36
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.

2 participants