Improve toast notification performance

dist/origin-web-common-services.js

@@ -1538,7 +1538,7 @@ angular.module('openshiftCommonServices')
             }
             // Use `$rootScope.$emit` instead of NotificationsService directly
             // so that DataService doesn't add a dependency on `openshiftCommonUI`
-            $rootScope.$emit('addNotification', {
+            $rootScope.$emit('NotificationsService.addNotification', {
               type: 'error',
               message: msg
             });
@@ -2083,7 +2083,7 @@ DataService.prototype.createStream = function(resource, name, context, opts, isR
           }
           // Use `$rootScope.$emit` instead of NotificationsService directly
           // so that DataService doesn't add a dependency on `openshiftCommonUI`
-          $rootScope.$emit('addNotification', {
+          $rootScope.$emit('NotificationsService.addNotification', {
             type: 'error',
             message: msg
           });
@@ -2114,7 +2114,7 @@ DataService.prototype.createStream = function(resource, name, context, opts, isR
         }
         // Use `$rootScope.$emit` instead of NotificationsService directly
         // so that DataService doesn't add a dependency on `openshiftCommonUI`
-        $rootScope.$emit('addNotification', {
+        $rootScope.$emit('NotificationsService.addNotification', {
           type: 'error',
           message: msg
         });
@@ -2304,7 +2304,7 @@ DataService.prototype.createStream = function(resource, name, context, opts, isR
       if (_.get(opts, 'errorNotification', true)) {
         // Use `$rootScope.$emit` instead of NotificationsService directly
         // so that DataService doesn't add a dependency on `openshiftCommonUI`
-        $rootScope.$emit('addNotification', {
+        $rootScope.$emit('NotificationsService.addNotification', {
           id: 'websocket_retry_halted',
           type: 'error',
           message: 'Server connection interrupted.',

dist/origin-web-common-ui.js

@@ -390,7 +390,7 @@ hawtioPluginLoader.addModule('openshiftCommonUI');
 
   $templateCache.put('src/components/toast-notifications/toast-notifications.html',
     "<div class=\"toast-notifications-list-pf\">\n" +
-    "  <div ng-repeat=\"(notificationID, notification) in notifications track by (notificationID + (notification.message || notification.details))\" ng-if=\"!notification.hidden\"\n" +
+    "  <div ng-repeat=\"(notificationID, notification) in notifications track by (notificationID + (notification.message || notification.details))\" ng-if=\"!notification.hidden || notification.isHover\"\n" +
     "       ng-mouseenter=\"setHover(notification, true)\" ng-mouseleave=\"setHover(notification, false)\">\n" +
     "    <div class=\"toast-pf alert {{notification.type | alertStatus}}\" ng-class=\"{'alert-dismissable': !hideCloseButton}\">\n" +
     "      <button ng-if=\"!hideCloseButton\" type=\"button\" class=\"close\" ng-click=\"close(notification)\">\n" +
@@ -1016,49 +1016,79 @@ angular.module('openshiftCommonUI')
 ;'use strict';
 
 angular.module('openshiftCommonUI')
-  .directive('toastNotifications', function(NotificationsService, $timeout) {
+  .directive('toastNotifications', function(NotificationsService, $rootScope, $timeout) {
     return {
       restrict: 'E',
       scope: {},
       templateUrl: 'src/components/toast-notifications/toast-notifications.html',
       link: function($scope) {
-        $scope.notifications = NotificationsService.getNotifications();
+        $scope.notifications = [];
 
-        $scope.close = function(notification) {
+        // A notification is removed if it has hidden set and the user isn't
+        // currently hovering over it.
+        var isRemoved = function(notification) {
+          return notification.hidden && !notification.isHover;
+        };
+
+        var removeNotification = function(notification) {
+          notification.isHover = false;
           notification.hidden = true;
+        };
+
+        // Remove items that are now hidden to keep the array from growing
+        // indefinitely. We loop over the entire array each digest loop, even
+        // if everything is hidden, and any watch update triggers a new digest
+        // loop. If the array grows large, it can hurt performance.
+        var pruneRemovedNotifications = function() {
+          $scope.notifications = _.reject($scope.notifications, isRemoved);
+        };
+
+        $scope.close = function(notification) {
+          removeNotification(notification);
           if (_.isFunction(notification.onClose)) {
             notification.onClose();
           }
         };
+
         $scope.onClick = function(notification, link) {
           if (_.isFunction(link.onClick)) {
             // If onClick() returns true, also hide the alert.
             var close = link.onClick();
             if (close) {
-              notification.hidden = true;
+              removeNotification(notification);
             }
           }
         };
+
         $scope.setHover = function(notification, isHover) {
-          notification.isHover = isHover;
+          // Don't change anything if the notification was already removed.
+          // Avoids a potential issue where the flag is reset during the slide
+          // out animation.
+          if (!isRemoved(notification)) {
+            notification.isHover = isHover;
+          }
         };
 
-        $scope.$watch('notifications', function() {
-          _.each($scope.notifications, function(notification) {
-            if (NotificationsService.isAutoDismiss(notification) && !notification.hidden) {
-              if (!notification.timerId) {
-                notification.timerId = $timeout(function () {
-                  notification.timerId = -1;
-                  if (!notification.isHover) {
-                    notification.hidden = true;
-                  }
-                }, NotificationsService.dismissDelay);
-              } else if (notification.timerId === -1 && !notification.isHover) {
-                notification.hidden = true;
-              }
-            }
-          });
-        }, true);
+        // Listen for updates from NotificationsService to show a notification.
+        var deregisterNotificationListener = $rootScope.$on('NotificationsService.onNotificationAdded', function(event, notification) {
+          $scope.notifications.push(notification);
+          if (NotificationsService.isAutoDismiss(notification)) {
+            $timeout(function () {
+              notification.hidden = true;
+            }, NotificationsService.dismissDelay);
+          }
+
+          // Whenever we add a new notification, also remove any hidden toasts
+          // so that the array doesn't grow indefinitely.
+          pruneRemovedNotifications();
+        });
+
+        $scope.$on('$destroy', function() {
+          if (deregisterNotificationListener) {
+            deregisterNotificationListener();
+            deregisterNotificationListener = null;
+          }
+        });
       }
     };
   });
@@ -1998,6 +2028,7 @@ angular.module('openshiftCommonUI').provider('NotificationsService', function()
       }
 
       notifications.push(notification);
+      $rootScope.$emit('NotificationsService.onNotificationAdded', notification);
     };
 
     var hideNotification = function (notificationID) {
@@ -2050,7 +2081,7 @@ angular.module('openshiftCommonUI').provider('NotificationsService', function()
     };
 
     // Also handle `addNotification` events on $rootScope, which is used by DataService.
-    $rootScope.$on('addNotification', function(event, data) {
+    $rootScope.$on('NotificationsService.addNotification', function(event, data) {
       addNotification(data);
     });
 

dist/origin-web-common.js

@@ -561,7 +561,7 @@ hawtioPluginLoader.addModule('openshiftCommonUI');
 
   $templateCache.put('src/components/toast-notifications/toast-notifications.html',
     "<div class=\"toast-notifications-list-pf\">\n" +
-    "  <div ng-repeat=\"(notificationID, notification) in notifications track by (notificationID + (notification.message || notification.details))\" ng-if=\"!notification.hidden\"\n" +
+    "  <div ng-repeat=\"(notificationID, notification) in notifications track by (notificationID + (notification.message || notification.details))\" ng-if=\"!notification.hidden || notification.isHover\"\n" +
     "       ng-mouseenter=\"setHover(notification, true)\" ng-mouseleave=\"setHover(notification, false)\">\n" +
     "    <div class=\"toast-pf alert {{notification.type | alertStatus}}\" ng-class=\"{'alert-dismissable': !hideCloseButton}\">\n" +
     "      <button ng-if=\"!hideCloseButton\" type=\"button\" class=\"close\" ng-click=\"close(notification)\">\n" +
@@ -1187,49 +1187,79 @@ angular.module('openshiftCommonUI')
 ;'use strict';
 
 angular.module('openshiftCommonUI')
-  .directive('toastNotifications', ["NotificationsService", "$timeout", function(NotificationsService, $timeout) {
+  .directive('toastNotifications', ["NotificationsService", "$rootScope", "$timeout", function(NotificationsService, $rootScope, $timeout) {
     return {
       restrict: 'E',
       scope: {},
       templateUrl: 'src/components/toast-notifications/toast-notifications.html',
       link: function($scope) {
-        $scope.notifications = NotificationsService.getNotifications();
+        $scope.notifications = [];
 
-        $scope.close = function(notification) {
+        // A notification is removed if it has hidden set and the user isn't
+        // currently hovering over it.
+        var isRemoved = function(notification) {
+          return notification.hidden && !notification.isHover;
+        };
+
+        var removeNotification = function(notification) {
+          notification.isHover = false;
           notification.hidden = true;
+        };
+
+        // Remove items that are now hidden to keep the array from growing
+        // indefinitely. We loop over the entire array each digest loop, even
+        // if everything is hidden, and any watch update triggers a new digest
+        // loop. If the array grows large, it can hurt performance.
+        var pruneRemovedNotifications = function() {
+          $scope.notifications = _.reject($scope.notifications, isRemoved);
+        };
+
+        $scope.close = function(notification) {
+          removeNotification(notification);
           if (_.isFunction(notification.onClose)) {
             notification.onClose();
           }
         };
+
         $scope.onClick = function(notification, link) {
           if (_.isFunction(link.onClick)) {
             // If onClick() returns true, also hide the alert.
             var close = link.onClick();
             if (close) {
-              notification.hidden = true;
+              removeNotification(notification);
             }
           }
         };
+
         $scope.setHover = function(notification, isHover) {
-          notification.isHover = isHover;
+          // Don't change anything if the notification was already removed.
+          // Avoids a potential issue where the flag is reset during the slide
+          // out animation.
+          if (!isRemoved(notification)) {
+            notification.isHover = isHover;
+          }
         };
 
-        $scope.$watch('notifications', function() {
-          _.each($scope.notifications, function(notification) {
-            if (NotificationsService.isAutoDismiss(notification) && !notification.hidden) {
-              if (!notification.timerId) {
-                notification.timerId = $timeout(function () {
-                  notification.timerId = -1;
-                  if (!notification.isHover) {
-                    notification.hidden = true;
-                  }
-                }, NotificationsService.dismissDelay);
-              } else if (notification.timerId === -1 && !notification.isHover) {
-                notification.hidden = true;
-              }
-            }
-          });
-        }, true);
+        // Listen for updates from NotificationsService to show a notification.
+        var deregisterNotificationListener = $rootScope.$on('NotificationsService.onNotificationAdded', function(event, notification) {
+          $scope.notifications.push(notification);
+          if (NotificationsService.isAutoDismiss(notification)) {
+            $timeout(function () {
+              notification.hidden = true;
+            }, NotificationsService.dismissDelay);
+          }
+
+          // Whenever we add a new notification, also remove any hidden toasts
+          // so that the array doesn't grow indefinitely.
+          pruneRemovedNotifications();
+        });
+
+        $scope.$on('$destroy', function() {
+          if (deregisterNotificationListener) {
+            deregisterNotificationListener();
+            deregisterNotificationListener = null;
+          }
+        });
       }
     };
   }]);
@@ -3262,7 +3292,7 @@ angular.module('openshiftCommonServices')
             }
             // Use `$rootScope.$emit` instead of NotificationsService directly
             // so that DataService doesn't add a dependency on `openshiftCommonUI`
-            $rootScope.$emit('addNotification', {
+            $rootScope.$emit('NotificationsService.addNotification', {
               type: 'error',
               message: msg
             });
@@ -3807,7 +3837,7 @@ DataService.prototype.createStream = function(resource, name, context, opts, isR
           }
           // Use `$rootScope.$emit` instead of NotificationsService directly
           // so that DataService doesn't add a dependency on `openshiftCommonUI`
-          $rootScope.$emit('addNotification', {
+          $rootScope.$emit('NotificationsService.addNotification', {
             type: 'error',
             message: msg
           });
@@ -3838,7 +3868,7 @@ DataService.prototype.createStream = function(resource, name, context, opts, isR
         }
         // Use `$rootScope.$emit` instead of NotificationsService directly
         // so that DataService doesn't add a dependency on `openshiftCommonUI`
-        $rootScope.$emit('addNotification', {
+        $rootScope.$emit('NotificationsService.addNotification', {
           type: 'error',
           message: msg
         });
@@ -4028,7 +4058,7 @@ DataService.prototype.createStream = function(resource, name, context, opts, isR
       if (_.get(opts, 'errorNotification', true)) {
         // Use `$rootScope.$emit` instead of NotificationsService directly
         // so that DataService doesn't add a dependency on `openshiftCommonUI`
-        $rootScope.$emit('addNotification', {
+        $rootScope.$emit('NotificationsService.addNotification', {
           id: 'websocket_retry_halted',
           type: 'error',
           message: 'Server connection interrupted.',
@@ -5178,6 +5208,7 @@ angular.module('openshiftCommonUI').provider('NotificationsService', function()
       }
 
       notifications.push(notification);
+      $rootScope.$emit('NotificationsService.onNotificationAdded', notification);
     };
 
     var hideNotification = function (notificationID) {
@@ -5230,7 +5261,7 @@ angular.module('openshiftCommonUI').provider('NotificationsService', function()
     };
 
     // Also handle `addNotification` events on $rootScope, which is used by DataService.
-    $rootScope.$on('addNotification', function(event, data) {
+    $rootScope.$on('NotificationsService.addNotification', function(event, data) {
       addNotification(data);
     });
 

dist/scripts/templates.js

@@ -361,7 +361,7 @@ angular.module('openshiftCommonUI').run(['$templateCache', function($templateCac
 
   $templateCache.put('src/components/toast-notifications/toast-notifications.html',
     "<div class=\"toast-notifications-list-pf\">\n" +
-    "  <div ng-repeat=\"(notificationID, notification) in notifications track by (notificationID + (notification.message || notification.details))\" ng-if=\"!notification.hidden\"\n" +
+    "  <div ng-repeat=\"(notificationID, notification) in notifications track by (notificationID + (notification.message || notification.details))\" ng-if=\"!notification.hidden || notification.isHover\"\n" +
     "       ng-mouseenter=\"setHover(notification, true)\" ng-mouseleave=\"setHover(notification, false)\">\n" +
     "    <div class=\"toast-pf alert {{notification.type | alertStatus}}\" ng-class=\"{'alert-dismissable': !hideCloseButton}\">\n" +
     "      <button ng-if=\"!hideCloseButton\" type=\"button\" class=\"close\" ng-click=\"close(notification)\">\n" +

src/components/toast-notifications/toast-notifications.html

 <div class="toast-notifications-list-pf">
-  <div ng-repeat="(notificationID, notification) in notifications track by (notificationID + (notification.message || notification.details))" ng-if="!notification.hidden"
+  <div ng-repeat="(notificationID, notification) in notifications track by (notificationID + (notification.message || notification.details))" ng-if="!notification.hidden || notification.isHover"
        ng-mouseenter="setHover(notification, true)" ng-mouseleave="setHover(notification, false)">
     <div class="toast-pf alert {{notification.type | alertStatus}}" ng-class="{'alert-dismissable': !hideCloseButton}">
       <button ng-if="!hideCloseButton" type="button" class="close" ng-click="close(notification)">

src/components/toast-notifications/toastNotifications.js

 'use strict';
 
 angular.module('openshiftCommonUI')
-  .directive('toastNotifications', function(NotificationsService, $timeout) {
+  .directive('toastNotifications', function(NotificationsService, $rootScope, $timeout) {
     return {
       restrict: 'E',
       scope: {},
       templateUrl: 'src/components/toast-notifications/toast-notifications.html',
       link: function($scope) {
-        $scope.notifications = NotificationsService.getNotifications();
+        $scope.notifications = [];
 
-        $scope.close = function(notification) {
+        // A notification is removed if it has hidden set and the user isn't
+        // currently hovering over it.
+        var isRemoved = function(notification) {
+          return notification.hidden && !notification.isHover;
+        };
+
+        var removeNotification = function(notification) {
+          notification.isHover = false;
           notification.hidden = true;
+        };
+
+        // Remove items that are now hidden to keep the array from growing
+        // indefinitely. We loop over the entire array each digest loop, even
+        // if everything is hidden, and any watch update triggers a new digest
+        // loop. If the array grows large, it can hurt performance.
+        var pruneRemovedNotifications = function() {
+          $scope.notifications = _.reject($scope.notifications, isRemoved);
+        };
+
+        $scope.close = function(notification) {
+          removeNotification(notification);
           if (_.isFunction(notification.onClose)) {
             notification.onClose();
           }
         };
+
         $scope.onClick = function(notification, link) {
           if (_.isFunction(link.onClick)) {
             // If onClick() returns true, also hide the alert.
             var close = link.onClick();
             if (close) {
-              notification.hidden = true;
+              removeNotification(notification);
             }
           }
         };
+
         $scope.setHover = function(notification, isHover) {
-          notification.isHover = isHover;
+          // Don't change anything if the notification was already removed.
+          // Avoids a potential issue where the flag is reset during the slide
+          // out animation.
+          if (!isRemoved(notification)) {
+            notification.isHover = isHover;
+          }
         };
 
-        $scope.$watch('notifications', function() {
-          _.each($scope.notifications, function(notification) {
-            if (NotificationsService.isAutoDismiss(notification) && !notification.hidden) {
-              if (!notification.timerId) {
-                notification.timerId = $timeout(function () {
-                  notification.timerId = -1;
-                  if (!notification.isHover) {
-                    notification.hidden = true;
-                  }
-                }, NotificationsService.dismissDelay);
-              } else if (notification.timerId === -1 && !notification.isHover) {
-                notification.hidden = true;
-              }
-            }
-          });
-        }, true);
+        // Listen for updates from NotificationsService to show a notification.
+        var deregisterNotificationListener = $rootScope.$on('NotificationsService.onNotificationAdded', function(event, notification) {
+          $scope.notifications.push(notification);
+          if (NotificationsService.isAutoDismiss(notification)) {
+            $timeout(function () {
+              notification.hidden = true;
+            }, NotificationsService.dismissDelay);
+          }
+
+          // Whenever we add a new notification, also remove any hidden toasts
+          // so that the array doesn't grow indefinitely.
+          pruneRemovedNotifications();
+        });
+
+        $scope.$on('$destroy', function() {
+          if (deregisterNotificationListener) {
+            deregisterNotificationListener();
+            deregisterNotificationListener = null;
+          }
+        });
       }
     };
   });

src/services/dataService.js

@@ -418,7 +418,7 @@ angular.module('openshiftCommonServices')
             }
             // Use `$rootScope.$emit` instead of NotificationsService directly
             // so that DataService doesn't add a dependency on `openshiftCommonUI`
-            $rootScope.$emit('addNotification', {
+            $rootScope.$emit('NotificationsService.addNotification', {
               type: 'error',
               message: msg
             });
@@ -963,7 +963,7 @@ DataService.prototype.createStream = function(resource, name, context, opts, isR
           }
           // Use `$rootScope.$emit` instead of NotificationsService directly
           // so that DataService doesn't add a dependency on `openshiftCommonUI`
-          $rootScope.$emit('addNotification', {
+          $rootScope.$emit('NotificationsService.addNotification', {
             type: 'error',
             message: msg
           });
@@ -994,7 +994,7 @@ DataService.prototype.createStream = function(resource, name, context, opts, isR
         }
         // Use `$rootScope.$emit` instead of NotificationsService directly
         // so that DataService doesn't add a dependency on `openshiftCommonUI`
-        $rootScope.$emit('addNotification', {
+        $rootScope.$emit('NotificationsService.addNotification', {
           type: 'error',
           message: msg
         });
@@ -1184,7 +1184,7 @@ DataService.prototype.createStream = function(resource, name, context, opts, isR
       if (_.get(opts, 'errorNotification', true)) {
         // Use `$rootScope.$emit` instead of NotificationsService directly
         // so that DataService doesn't add a dependency on `openshiftCommonUI`
-        $rootScope.$emit('addNotification', {
+        $rootScope.$emit('NotificationsService.addNotification', {
           id: 'websocket_retry_halted',
           type: 'error',
           message: 'Server connection interrupted.',

src/ui-services/notificationsService.js

@@ -23,6 +23,7 @@ angular.module('openshiftCommonUI').provider('NotificationsService', function()
       }
 
       notifications.push(notification);
+      $rootScope.$emit('NotificationsService.onNotificationAdded', notification);
     };
 
     var hideNotification = function (notificationID) {
@@ -75,7 +76,7 @@ angular.module('openshiftCommonUI').provider('NotificationsService', function()
     };
 
     // Also handle `addNotification` events on $rootScope, which is used by DataService.
-    $rootScope.$on('addNotification', function(event, data) {
+    $rootScope.$on('NotificationsService.addNotification', function(event, data) {
       addNotification(data);
     });