Skip to content

Commit

Permalink
Bottom sheet dialogs should dismiss instead of hiding when a button i…
Browse files Browse the repository at this point in the history
…s pressed (#3951)

<!--
Note: This checklist is a reminder of our shared engineering
expectations.
The items in Bold are required
If your PR involves UI changes:
1. Upload screenshots or screencasts that illustrate the changes before
/ after
2. Add them under the UI changes section (feel free to add more columns
if needed)
If your PR does not involve UI changes, you can remove the **UI
changes** section

At a minimum, make sure your changes are tested in API 23 and one of the
more recent API levels available.
-->

Task/Issue URL:
https://app.asana.com/0/488551667048375/1206077448728301/f

### Description
When a button is pressed in the bottom sheet dialogs, ensure we
`dismiss()` instead of `hide()` to prevent unintended side-effects
depending on what the caller does not (as detailed in the linked task).

Sets the dismiss listener to `null` before calling `dismiss()` to ensure
we get only one of:
- button listener invoked, OR
- dismiss listener invoked

### Steps to test this PR

- [x] Go to ADS internal settings->Dialogs
- [x] ensure the `Action Botton Sheet` works as expected
- [x] ensure the `Promo Bottom Sheet` works as expected
  • Loading branch information
CDRussell authored Dec 1, 2023
1 parent c9977fc commit 62c85bd
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,13 @@ class ActionBottomSheetDialog(builder: Builder) : BottomSheetDialog(builder.cont
setOnShowListener { builder.listener.onBottomSheetShown() }
binding.actionBottomSheetDialogPrimaryItem.setOnClickListener {
builder.listener.onPrimaryItemClicked()
hide()
setOnDismissListener(null)
dismiss()
}
binding.actionBottomSheetDialogSecondaryItem.setOnClickListener {
builder.listener.onSecondaryItemClicked()
hide()
setOnDismissListener(null)
dismiss()
}

builder.titleText?.let {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,13 @@ class PromoBottomSheetDialog(builder: Builder) : BottomSheetDialog(builder.conte
setOnShowListener { builder.listener.onBottomSheetShown() }
binding.bottomSheetPromoPrimaryButton.setOnClickListener {
builder.listener.onPrimaryButtonClicked()
hide()
setOnDismissListener(null)
dismiss()
}
binding.bottomSheetPromoSecondaryButton.setOnClickListener {
builder.listener.onSecondaryButtonClicked()
hide()
setOnDismissListener(null)
dismiss()
}

builder.icon?.let {
Expand Down

0 comments on commit 62c85bd

Please sign in to comment.