-
Notifications
You must be signed in to change notification settings - Fork 6.7k
Conversation
I like this addition. I think a default timeout value would be great. This allows to:
|
+1 |
@khalilravanna Thnx for this PR. I think that several people expressed interest in a similar feature. Having said this I would ideally see this feature as a separate directive where people could decide a condition upon which an alert is dismissed. With the current proposal closing on timeout is kind of hard-coded without much room for extensibility. I think what would work great is to have a separate directive that just removes an alert (or any other directive for that matter (!)) after a certain timeout. Such a directive would be highly reusable. I will try to craft a PR with this idea so we can discuss. |
@khalilravanna @sanjo could you guys please have a look at the alternative proposal in #2854? With this proposal people can write a directive and decide on they own what should be a condition upon which an alert is dismissed. By default it could be a timeout but one could easily imagine listening to route change, some data bound variable to change its value etc. WDYT? |
I like the better reusability and greater flexibility of #2854. |
I agree with you both, #2854 looks like a better, more modular/extensible approach. |
@pkozlowski-opensource I had a need for an auto dismiss directive that could be used on any element that leveraged ng-if, ng-show, or ng-hide. Here is what I came up with: plunk. I wish it was a little more robust in that it didn't need a Boolean value on the scope, but it's a start. |
Sorry for the basic question, but what version will this be included in? I have 0.11.2 and I do not see the dismissOnTimeout directive there yet. I am using the following markup:
Alerts are visible, but never auto close on timeout. I have to manually close them. |
A feature I always like having with alerts is having them optionally be able to disappear after a short time if they're not important.
Lemme know if I'm missing anything in this PR.