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 onTap and closeOnTap methods #105

Merged
merged 1 commit into from
Feb 21, 2024
Merged

Conversation

ferraridamiano
Copy link
Contributor

Hi there!
This PR adds a couple of useful properties to this package:

  • onTap let the user click on the whole notification. A developer cannot set both an action and onTap
  • closeOnTap is a property that if set to true closes the notification on user interaction (onTap or action pressed)

Thanks for this package, it is very useful. Now I only miss the swipe to remove gesture on the notification. Thanks for moving the packege to MIT license

Copy link
Owner

@koukibadr koukibadr left a comment

Choose a reason for hiding this comment

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

According to the changes when closeOnTap == true but onTap is null the closeOnTap won't work properly (it won't close the popup)
I propose you update the InkWell widget as below

InkWell(
        onTap:() {
                widget.onTap?.call();
                if (widget.closeOnTap) {
                  closeNotification();
                }
              },

@ferraridamiano
Copy link
Contributor Author

closeOnTap is meant to be only on user iteraction, i.e. only when onTap or an action are pressed. If there is no action or tap event associated with it, it should not close. What do you think about it?

@koukibadr
Copy link
Owner

closeOnTap is meant to be only on user iteraction, i.e. only when onTap or an action are pressed. If there is no action or tap event associated with it, it should not close. What do you think about it?

Let me test it first this evening and then we merge it and publish a new version

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.

None yet

2 participants