maxkeppeler / sheets

⭐ ‎‎‎‏‏‎ ‎Offers a range of beautiful sheets (dialogs & bottom sheets) for quick use in your project. Includes many ways to customize sheets.
https://maxkeppeler.github.io/sheets/
Apache License 2.0
921 stars 77 forks source link

OptionsSheet automatically runs action #28

Closed jackdevey closed 3 years ago

jackdevey commented 3 years ago

Describe the bug The first time it is opened you can proceed with the action perfectly fine, however every other subsequent you open it, it will run the same action again instantly.

Library Version: 1.1.2

To Reproduce Steps to reproduce the behavior:

  1. Create an OptionsSheet with the following code
// Builds the options bottom sheet
        val moreOptions = OptionsSheet().build(requireContext()) {
            displayMode(DisplayMode.LIST)
            title("More")
            closeButtonDrawable(R.drawable.ic_down_arrow)
            with(
                Option(R.drawable.ic_share, "Share"),
                Option(R.drawable.ic_add_circle, "Add to list")
            )
            onPositive { index: Int, option: Option ->
                binding.root.performHapticFeedback(HapticFeedbackConstants.VIRTUAL_KEY)
                if (index == 0) {
                    val sendIntent: Intent = Intent().apply {
                        action = Intent.ACTION_SEND
                        val text = binding.quote.text.toString() + "\n\n~Gautama Buddha"
                        putExtra(Intent.EXTRA_TEXT, text)
                        type = "text/plain"
                    }
                    val shareIntent = Intent.createChooser(sendIntent, null)
                    startActivity(shareIntent)
                } else {
                    dismiss()
                    //This is the indented action. We haven't coded the rest yet
                }
            }
        }

Expected behavior Subsequent times you open the bottom sheet, it should act like the first time rather than running the previous action

Screenshots https://user-images.githubusercontent.com/38474124/103697114-5c2c5f80-4f97-11eb-95c0-e98cf1d56ebb.mp4

Affected Device(s):

Additional context We use other variations of the option sheet in our app and haven't come across this issue. This happens on either option the user chooses.

maxkeppeler commented 3 years ago

Thank you for reporting.

I just copied your setup and replaced the closeButtonDrawable. But I can't replicate this behavior.

I noticed you used the build(...)method. Where do you currently call moreOptions.show()?

Does it behave the same for you, if you use the show(...)method and immediately display it?

jackdevey commented 3 years ago

Thanks for your help, I have looked again at my code and it the Bottom Sheet is then shown on a button click. Changing my code so that the sheet is recreated and shown each time the button is pressed has fixed this now. The issue seems to be calling show() on a bottom sheet instance that has already been shown once before.

russellbanks commented 3 years ago

Hey! Sorry for all the issues recently but I'm the other developer that is working on this app.

We were calling moreOptions.show() just below that chunk of code like this:

// Shows the options bottom sheet
binding.more.setOnClickListener {
   moreOptions.show()
}

It seems doing OptionSheet.show() rather than building it first fixes the issue

     // Shows the options bottom sheet
     binding.more.setOnClickListener {
         OptionsSheet().show(requireContext()) {
             displayMode(DisplayMode.LIST)
             title("More")
             closeButtonDrawable(R.drawable.ic_down_arrow)
             with(
                 Option(R.drawable.ic_share, "Share"),
                 Option(R.drawable.ic_add_circle, "Add to list")
             )
             onPositive { index: Int, option: Option ->
                 binding.root.performHapticFeedback(HapticFeedbackConstants.VIRTUAL_KEY)
                 if (index == 0) {
                     val sendIntent: Intent = Intent().apply {
                         action = Intent.ACTION_SEND
                         val text = binding.quote.text.toString() + "\n\n~Gautama Buddha"
                         putExtra(Intent.EXTRA_TEXT, text)
                         type = "text/plain"
                     }
                     val shareIntent = Intent.createChooser(sendIntent, null)
                     startActivity(shareIntent)
                 } else {
                     dismiss()
                 }
             }
         }
     }

Therefore, the issue seems to arise when the bottom sheet is built and then shown rather than just using .show()

maxkeppeler commented 3 years ago

You should just create a new instance of the OptionsSheet and show it everytime you need it.

As in the example from @Fennec-exe. You can just extract it into a method showMoreSheet() and execute it there.

Using build(...) + show(...) should be used in cases where you don't want to display a bottom sheet immediately.

I don't think it's a good idea to automatically reset the instance when using show() multiple times.

It's easier and cleaner to just create a new instance.

I will close this issue since it works as intended.