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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Issue-13] Add option to disable dismiss UI. #14

Merged
merged 4 commits into from Mar 22, 2023

Conversation

devyml
Copy link
Collaborator

@devyml devyml commented Mar 22, 2023

Introduction

We have no way of disabling the swipe down, tap on dimmer, or accessibility escape gesture which we use to dismiss the sheet. Some apps might require that the user not be able to dismiss the sheet without choosing a CTA or taking a specific action.

Purpose

Scope

We have added a new property allowDismiss in appearance. If it is true then we can dismiss the bottom sheet else we are suspending the dismiss action

Discussion

BottomSheetViewController.swift had grown too large, so as part of this PR I moved the animator functionality out to a separate code file.

馃搱 Coverage

Code

Screenshot 2023-03-22 at 1 27 47 PM

Documentation

Screenshot 2023-03-22 at 1 30 10 PM

@devyml devyml self-assigned this Mar 22, 2023
@devyml devyml added the enhancement New feature or request label Mar 22, 2023
@devyml devyml changed the title [CM-1261] Add option to disable dismiss UI. [Issue-13] Add option to disable dismiss UI. Mar 22, 2023
@mpospese
Copy link
Contributor

You kind of swapped Introduction and Purpose. Introduction is the more casual introduction of what the problem is or why we are bothering to make a change at all. Purpose is the stated goal of the PR. So I would do:

Introduction

We have no way of disabling the swipe down, tap on dimmer, or accessibility escape gesture which we use to dismiss the sheet. Some apps might require that the user not be able to dismiss the sheet without choosing a CTA or taking a specific action.

Purpose

@mpospese
Copy link
Contributor

You also probably should use the Discussion section to talk about why you introduced a new file.

Discussion

BottomSheetViewController.swift had grown too large, so as part of this PR I moved the animator functionality out to a separate code file.

@devyml devyml requested a review from mpospese March 22, 2023 11:23
@devyml devyml requested a review from mpospese March 22, 2023 11:56
Copy link
Contributor

@mpospese mpospese left a comment

Choose a reason for hiding this comment

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

Looks great. Let's get this merged and release a new version.

@devyml devyml merged commit 320cf01 into main Mar 22, 2023
1 check passed
@devyml devyml deleted the feature/CM-1261-configureDismissToOptional branch March 22, 2023 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

Add option to disable dismiss UI
2 participants