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

Provide modal parameter to showSimpleNotification #5

Closed
MsXam opened this issue May 15, 2019 · 5 comments · Fixed by #6
Closed

Provide modal parameter to showSimpleNotification #5

MsXam opened this issue May 15, 2019 · 5 comments · Fixed by #6
Assignees
Labels
enhancement New feature or request

Comments

@MsXam
Copy link

MsXam commented May 15, 2019

Currently, in its simplest form - the quickest way to show a notification at the top of the screen is via showSimpleNotification. Without resorting to writing custom notification overlay, can we extend the showSimpleNotification to include an optional parameter

bool modal=true

This way - if Notifications are tied to (say) button taps - then repeated Notifications will not be shown because the first Notification at the top of the screen being MODAL will prevent this.

@boyan01
Copy link
Owner

boyan01 commented May 16, 2019

thanks for your suggestion. I have considered it for a long time, there could be two way to reach that goal.

  1. In my mind,a modal notification could be a Dialog. So you may be able to use the dialog instead of showNotification

  2. hold the NotificationEntry which return by showSimpleNotification.
    just to check the NotificationEntry#isDismissed to decide to show notification.

@MsXam
Copy link
Author

MsXam commented May 16, 2019

Yes - you are right on both accounts. FYI, I have created my own popups (modal/non modal) using the first approach - i.e via showDialog. If the overlay_support package is extended to use showDialog when modal=true is supplied to showSimpleNotification then the functionality & implementation of a modal notification will be transparent. Developers using this package will not need to do anything special.

Additionally, add the modal=true as an optional parameter which defaults to false - this way the API will not be broken and any existing users will get the same functionality they are currently getting.

Attached gif shows an example of a popup dialog that displays when a user is logging in to app. Its modal thus preventing further taps and centered - when the log in process completes , the main app will load.

modalDialog

@boyan01
Copy link
Owner

boyan01 commented May 16, 2019

yes, add modal as an optional parameter is convenient in this use case.

but I think that is not good to prevent other notification.

  1. toast() is show a 'notification' too. that would be a great influence for other API
  2. showSimpleNotification() is a top level method, so it is hard to known currently if some modal notification is showing. (keep a static reference is wired.

I have considered add a optional parameter name with key, then we will reject the notification with duplicate key which send by showSimpleNotification()

@boyan01
Copy link
Owner

boyan01 commented May 16, 2019

in addition, i have wrote a method to achieved a similar effect in the past.
maybe this can help you.

https://github.com/boyan01/flutter-netease-music/blob/c9c6da295f8bef7ac6e66366470e1c01ae01bc5b/lib/material/dialogs.dart#L7

@MsXam
Copy link
Author

MsXam commented May 16, 2019

toast() is show a 'notification' too. that would be a great influence for other API
showSimpleNotification() is a top level method, so it is hard to known currently if some modal notification is showing. (keep a static reference is wired.
I have considered add a optional parameter name with key, then we will reject the notification with duplicate key which send by showSimpleNotification()

The KEY approach would work and thus the implementation of showSimpleNotification should reject all future invocations that have the same invocation key while it is still RUNNING, i.e it should not be reentrant when the same key is supplied during the showSimpleNotification duration period.

Once the duration period is over, showSimpleNotification can be invoked again with the same key

Yes this works and is good :

https://github.com/boyan01/flutter-netease-music/blob/c9c6da295f8bef7ac6e66366470e1c01ae01bc5b/lib/material/dialogs.dart#L7

my suggestion for showSimpleNotification was just to make the package much better as I think in its current form it is Great but could be Greater

@boyan01 boyan01 self-assigned this May 16, 2019
@boyan01 boyan01 added the enhancement New feature or request label May 16, 2019
@boyan01 boyan01 mentioned this issue May 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants