-
-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
Notifications: passive success notifications fade out #289
Comments
Makes sense. I wasn't sure about what counts as a user action, because for some, I might want to keep the previous notification up. There was also no one online when this thought came to me, so I just rolled with it. That being said, I can change it around. Did you merge the notifications branch into master, or should I keep on doing stuff on notifications? |
I thought it would be good to stay on the notifications branch til everything was sorted and merge it all as one big beautiful finished feature :) Don't feel you have to do all of the notifications issues though, nor that this issue is saying that anything you did before was wrong. It's an iteration / evolution now that we can play with what is there. When using the system I found myself expecting, almost waiting for the big green success notifications to fade away, which I take as a good measure that this is probably what they should do :) Probably ought to get @JohnONolan to confirm this though. |
Oh, with the |
Having had a play with the notifications branch, I think the passive notifications need some tweaks.
Original requirement was that: "Passive should disappear on the next pageload or user action". I think saving again should count as a user action which makes them disappear - else everytime you save you get yet another success notification.
Furthermore, in the case of success notifications, where the message isn't required for further action (an error message might be needed to help fix a problem etc), I think the message should only last a few seconds before fading out.
Can we also please delete the commented out window.alerts?
The text was updated successfully, but these errors were encountered: