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

Add option to dynamically enable bottom sheet drag for a single page #45

Merged
merged 3 commits into from
Sep 2, 2023

Conversation

ulusoyca
Copy link
Collaborator

Don't merge this PR before #44

Summary

This pull request introduces a new feature that enhances the behavior of the WoltModalSheet library by allowing the enabling or disabling of drag-to-dismiss functionality for the bottom sheet on a per-page basis. The primary change is the addition of the enableDragForBottomSheet property within the WoltModalSheetPage class.

Changes

  • Added a new property enableDragForBottomSheet to the WoltModalSheetPage class.
  • Enhanced the WoltBottomSheetDragHandle to be conditionally displayed based on the enableDragForBottomSheet property.
  • Added a new use case to the playground example app to demonstrate how to dynamically enable/disable drag for modal sheet.
  • Updated documentation and examples to demonstrate the usage of the new feature.

Use Case

This feature addition addresses a common use case where dynamic conditions dictate whether a bottom sheet can be dismissed by dragging or not. By providing this per-page option, developers can create more tailored and flexible user experiences, enabling or disabling the drag-to-dismiss behavior based on specific context and requirements.

Example

WoltModalSheetPage(
  enableDragForBottomSheet: shouldEnableDrag, // Enable or disable drag-to-dismiss
  // ... other page properties ...
)
dynamic_property.mp4

@ulusoyca ulusoyca force-pushed the add-theme-extensions branch from f5c2d72 to d85d6f4 Compare August 19, 2023 18:07
@ulusoyca ulusoyca mentioned this pull request Aug 19, 2023
Base automatically changed from add-theme-extensions to main August 30, 2023 10:59
@ulusoyca ulusoyca force-pushed the add-option-to-dynamically-enable-drag branch from 49799e2 to 868dd80 Compare August 31, 2023 09:27
@ulusoyca ulusoyca requested a review from TahaTesser August 31, 2023 09:27
@ulusoyca ulusoyca marked this pull request as ready for review August 31, 2023 09:27
@@ -118,6 +118,10 @@ class WoltModalSheetPage {
/// `true`.
final bool? hasSabGradient;

/// Controls the draggability of the bottom sheet. This setting overrides the value provided
/// via [WoltModalSheet.show] specifically for this page when the modal is displayed as a bottom sheet.
final bool? enableDragForBottomSheet;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unless there is another enableDrag property in the package. I'd suggest just enableDrag.
The current name is too verbose IMO. The documentation should be enough to explain what enable drag is for.

See https://master-api.flutter.dev/flutter/material/BottomSheet/enableDrag.html

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, removed 868dd80 and 219fc21


class SheetPageWithDynamicPageProperties {
SheetPageWithDynamicPageProperties._();

Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this is already being used. Short documentation will be helpful.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added docs here: 219fc21

@ulusoyca ulusoyca merged commit 53746ae into main Sep 2, 2023
@ulusoyca ulusoyca deleted the add-option-to-dynamically-enable-drag branch September 2, 2023 09:16
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