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

Make it possible to dismiss popup from viewBoundCallback #57

Merged
merged 2 commits into from
Apr 30, 2019
Merged

Make it possible to dismiss popup from viewBoundCallback #57

merged 2 commits into from
Apr 30, 2019

Conversation

Tunous
Copy link
Contributor

@Tunous Tunous commented Apr 27, 2019

Use case

I want to be able to dismiss popup when user performs action on a custom item's view.

Current solution

Impossible?

Implemented solution

When building custom item it is now possible to set viewBoundCallback to an instance of ViewBoundCallback class instead of a regular closure, which then allows user to access dismissPopup function.

section {
  item {
    viewBoundCallback = ViewBoundCallback {
      val someView = it.findViewById(R.id.some_view)
      someView.setOnClickListener {
        dismissPopup()
      }
    }
  }
}

Sample

Here sample has been modified to dismiss the popup when only when checkbox gets checked.

@zawadz88
Copy link
Owner

Thanks for the changes! I'll try to have a look on Monday and get back to you.

@zawadz88
Copy link
Owner

zawadz88 commented Apr 29, 2019

Hi @Tunous,
I've checked and it is actually possible to achieve the effect you presented with the following lines of code:

        var popupMenu by Delegates.notNull<MaterialPopupMenu>()
        popupMenu = popupMenu {
            dropdownGravity = Gravity.END
            section {
                customItem {
                    layoutResId = R.layout.view_custom_item_checkable
                    viewBoundCallback = { view ->
                        val checkBox: CheckBox = view.findViewById(R.id.customItemCheckbox)
                        checkBox.isChecked = true
                        checkBox.setOnCheckedChangeListener { _, isChecked ->
                            if (isChecked) {
                                popupMenu.dismiss()
                            }
                        }
                    }
                    callback = {
                        Toast.makeText(this@LightActivity, "Disabled!", Toast.LENGTH_SHORT).show()
                    }
                }
            }
        }

        popupMenu.show(this@LightActivity, view)

MaterialPopupMenu has a dismiss() method you can use.

@Tunous
Copy link
Contributor Author

Tunous commented Apr 29, 2019

Hmm, true didn't think of that way. But it feels more like a hack than a real solution because you have to create a variable that is initialized later. With that I can't isolate the code that creates the pop-up from the code which shows it, as I have to somehow send the pop-up to builder code...

@zawadz88
Copy link
Owner

it feels more like a hack than a real solution because you have to create a variable that is initialized later.

Kinda feels this way although using Delegates.notNull sometimes can be useful if all else fails.

With that I can't isolate the code that creates the pop-up from the code which shows it, as I have to somehow send the pop-up to builder code...

That's a very good point.

Copy link
Owner

@zawadz88 zawadz88 left a comment

Choose a reason for hiding this comment

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

Looks OK, just have a look at the comment in the test class please.

I'll try to release a new version this week once I complete some other work I'm doing around multi-level menus.

@@ -274,7 +277,9 @@ class MaterialPopupMenuBuilderTest {
val secondPopupMenuItem = secondItem as MaterialPopupMenu.PopupMenuCustomItem
assertEquals("Invalid item layout ID", CUSTOM_ITEM_LAYOUT, secondPopupMenuItem.layoutResId)
assertEquals("Invalid item callback", customCallback, secondPopupMenuItem.callback)
assertEquals("Invalid item view bound callback", customViewBoundCallback, secondPopupMenuItem.viewBoundCallback)

secondPopupMenuItem.viewBoundCallback.invoke(view)
Copy link
Owner

Choose a reason for hiding this comment

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

This feels like something that deserves a separate test to me since you're making and additional step in the verification section of the test (the one in //then).
A separate test for this with a single item and secondPopupMenuItem.viewBoundCallback.invoke(view) in the //when section would be more appropriate I think as it would indicate more clearly which part is being tested (the code for constructing the popup menu should be in //given section then as it's a part of the test preparation).

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. I've added one.

*/
class ViewBoundCallback(
private val callback: ViewBoundCallback.(View) -> Unit
) : (View) -> Unit {
Copy link
Owner

@zawadz88 zawadz88 Apr 29, 2019

Choose a reason for hiding this comment

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

Ha quite clever to extend a closure here ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that's a nice feature. I found it by accident :)

@zawadz88 zawadz88 merged commit 40ffa53 into zawadz88:master Apr 30, 2019
@zawadz88 zawadz88 mentioned this pull request Apr 30, 2019
@Tunous Tunous deleted the feature/dismiss-viewboundcallback branch May 2, 2019 16:17
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