zawadz88 / MaterialPopupMenu

Shows Material popup menus grouped in sections & more
Apache License 2.0
646 stars 57 forks source link

Dismiss is not working when tap outside of popup window #66

Closed cvezina closed 4 years ago

cvezina commented 4 years ago

The popup window is not dismissed when you tap outside. You can reproduce the problem using the sample provided.

minSdkVersion 21 targetSdkVersion 28

DeweyReed commented 4 years ago

I encounter the same problem when I test the app on a Lollipop emulator. Removing this line can help although it's not a good solution: https://github.com/zawadz88/MaterialPopupMenu/blob/master/material-popup-menu/src/main/java/androidx/appcompat/widget/MaterialRecyclerViewPopupWindow.kt#L239

zawadz88 commented 4 years ago

@cvezina @DeweyReed thanks for reporting this. I can reproduce this on Lollipop 5.0 emulator as well. I seems to be working on newer OS version so I'll need to investigate further.

zawadz88 commented 4 years ago

It seems to be an issue on Lollipop only (5.0 and 5.1) - I've checked also API 19, 23, 25 & 28 and it's correct there on the emulator at least.

zawadz88 commented 4 years ago

This seems to be an issue with clipping rounded backgrounds #47 . I can see 2 solutions at the moment:

  1. Use clipToOutline on Marshmallow+ only i.e. change:

        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
            dropDownList.clipToOutline = true
            // Move the background from popup to RecyclerView for clipToOutline to take effect.
            dropDownList.background = background
            popup.setBackgroundDrawable(null)
        }

    to:

        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
            dropDownList.clipToOutline = true
            // Move the background from popup to RecyclerView for clipToOutline to take effect.
            dropDownList.background = background
            popup.setBackgroundDrawable(null)
        }
  2. Set popup.setBackgroundDrawable(null) on Marshmallow only i.e.

        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
            dropDownList.clipToOutline = true
            // Move the background from popup to RecyclerView for clipToOutline to take effect.
            dropDownList.background = background
            if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
                popup.setBackgroundDrawable(null)
            }
        }
  3. Means rounded corners will work only on M+ while 2. means we'll add overdraw on Lollipop devices. Before: Screenshot_1575648176 After: Screenshot_1575648288

@Tunous do you have any thoughts on that?

Tunous commented 4 years ago

Option 2 appears to be a better solution in this case. I’m not sure whether removing it is entirely safe but I also don’t remember if there were any other issues which this line was intended to resolve. Most likely these were on other Android version (if ever).

zawadz88 commented 4 years ago

Fixed in 4.0.1. Thanks all for contributing!