Improve the implementation of Notifications
Summary:
Currently, you can't change a notification that's already shown. There's no reason for this.
(I'm planning to put file upload progress/errors in notifications.)
  - Make `setContent()` and `setDuration()` immediately affect the notification.
  - When there are more than 5 notifications, queue them up instead of dropping them.
  - Allow arbitrarily many classes to be added/removed.
  - Make the examples in the UIExamples tests more rich.
Test Plan:
  - Verified normal notifications continue to function as expected.
  - Played with the UIExamples notifications:
    - Verified the "update every second" notification udpated every second.
    - Verified the permanent alert notification was yellow and requires a click to dismiss.
    - Verified the interactive notification responds correctly to "OK" / "Cancel".
    - Verified the "click every 2 seconds" notification doesn't vanish until not clicked for 2 seconds.
Reviewers: btrahan, vrana
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D3653
			
			
This commit is contained in:
		| @@ -888,7 +888,7 @@ celerity_register_resource_map(array( | ||||
|   ), | ||||
|   'javelin-behavior-aphlict-listen' => | ||||
|   array( | ||||
|     'uri' => '/res/0743d3f3/rsrc/js/application/aphlict/behavior-aphlict-listen.js', | ||||
|     'uri' => '/res/6dde3f43/rsrc/js/application/aphlict/behavior-aphlict-listen.js', | ||||
|     'type' => 'js', | ||||
|     'requires' => | ||||
|     array( | ||||
| @@ -1522,7 +1522,7 @@ celerity_register_resource_map(array( | ||||
|   ), | ||||
|   'javelin-behavior-phabricator-notification-example' => | ||||
|   array( | ||||
|     'uri' => '/res/df97e4b3/rsrc/js/application/uiexample/notification-example.js', | ||||
|     'uri' => '/res/a6d51998/rsrc/js/application/uiexample/notification-example.js', | ||||
|     'type' => 'js', | ||||
|     'requires' => | ||||
|     array( | ||||
| @@ -2499,7 +2499,7 @@ celerity_register_resource_map(array( | ||||
|   ), | ||||
|   'phabricator-notification' => | ||||
|   array( | ||||
|     'uri' => '/res/cacd79f1/rsrc/js/application/core/Notification.js', | ||||
|     'uri' => '/res/c604fbbe/rsrc/js/application/core/Notification.js', | ||||
|     'type' => 'js', | ||||
|     'requires' => | ||||
|     array( | ||||
|   | ||||
| @@ -35,7 +35,7 @@ JX.behavior('aphlict-listen', function(config) { | ||||
|  | ||||
|         new JX.Notification() | ||||
|           .setContent('(Aphlict) [' + type + '] ' + details) | ||||
|           .setClassName('jx-notification-debug') | ||||
|           .alterClassName('jx-notification-debug', true) | ||||
|           .setDuration(0) | ||||
|           .show(); | ||||
|       } | ||||
| @@ -63,7 +63,7 @@ JX.behavior('aphlict-listen', function(config) { | ||||
|         !showing_reload) { | ||||
|       var reload = new JX.Notification() | ||||
|         .setContent('Page updated, click to reload.') | ||||
|         .setClassName('jx-notification-alert') | ||||
|         .alterClassName('jx-notification-alert', true) | ||||
|         .setDuration(0); | ||||
|       reload.listen('activate', function(e) { JX.$U().go(); }) | ||||
|       reload.show(); | ||||
|   | ||||
| @@ -8,7 +8,7 @@ | ||||
|  */ | ||||
|  | ||||
| /** | ||||
|  * Show a notification. Usage: | ||||
|  * Show a notification popup on screen. Usage: | ||||
|  * | ||||
|  *   var n = new JX.Notification() | ||||
|  *     .setContent('click me!'); | ||||
| @@ -21,49 +21,78 @@ JX.install('Notification', { | ||||
|   events : ['activate', 'close'], | ||||
|  | ||||
|   members : { | ||||
|     _container : null, | ||||
|     _visible : false, | ||||
|     _hideTimer : null, | ||||
|     _duration : 12000, | ||||
|  | ||||
|     show : function() { | ||||
|       var self = JX.Notification; | ||||
|       self._show(this); | ||||
|       if (!this._visible) { | ||||
|         this._visible = true; | ||||
|  | ||||
|       if (this.getDuration()) { | ||||
|         setTimeout(JX.bind(self, self._hide, this), this.getDuration()); | ||||
|         var self = JX.Notification; | ||||
|         self._show(this); | ||||
|         this._updateTimer(); | ||||
|       } | ||||
|       return this; | ||||
|     }, | ||||
|     _render : function() { | ||||
|       return JX.$N( | ||||
|         'div', | ||||
|         { | ||||
|           className: 'jx-notification ' + this.getClassName(), | ||||
|           sigil: 'jx-notification' | ||||
|         }, | ||||
|         this.getContent()); | ||||
|     } | ||||
|   }, | ||||
|  | ||||
|   properties : { | ||||
|     hide : function() { | ||||
|       if (this._visible) { | ||||
|         this._visible = false; | ||||
|  | ||||
|         var self = JX.Notification; | ||||
|         self._hide(this); | ||||
|         this._updateTimer(); | ||||
|       } | ||||
|       return this; | ||||
|     }, | ||||
|  | ||||
|     alterClassName : function(name, enable) { | ||||
|       JX.DOM.alterClass(this._getContainer(), name, enable); | ||||
|       return this; | ||||
|     }, | ||||
|  | ||||
|     setContent : function(content) { | ||||
|       JX.DOM.setContent(this._getContainer(), content); | ||||
|       return this; | ||||
|     }, | ||||
|  | ||||
|     /** | ||||
|      * Optional class name(s) to add to the rendered notification. | ||||
|      * | ||||
|      * @param string Class name(s). | ||||
|      */ | ||||
|     className : null, | ||||
|  | ||||
|     /** | ||||
|      * Notification content. | ||||
|      * | ||||
|      * @param mixed Content. | ||||
|      */ | ||||
|     content : null, | ||||
|  | ||||
|     /** | ||||
|      * Duration before the notification fades away, in milliseconds. If set to | ||||
|      * 0, the notification persists until dismissed. | ||||
|      * Set duration before the notification fades away, in milliseconds. If set | ||||
|      * to 0, the notification persists until dismissed. | ||||
|      * | ||||
|      * @param int Notification duration, in milliseconds. | ||||
|      * @return this | ||||
|      */ | ||||
|     duration : 12000 | ||||
|     setDuration : function(milliseconds) { | ||||
|       this._duration = milliseconds; | ||||
|       this._updateTimer(false); | ||||
|       return this; | ||||
|     }, | ||||
|  | ||||
|     _updateTimer : function() { | ||||
|       if (this._hideTimer) { | ||||
|         clearTimeout(this._hideTimer); | ||||
|         this._hideTimer = null; | ||||
|       } | ||||
|  | ||||
|       if (this._visible && this._duration) { | ||||
|         this._hideTimer = setTimeout(JX.bind(this, this.hide), this._duration); | ||||
|       } | ||||
|     }, | ||||
|  | ||||
|     _getContainer : function() { | ||||
|       if (!this._container) { | ||||
|         this._container = JX.$N( | ||||
|           'div', | ||||
|           { | ||||
|             className: 'jx-notification', | ||||
|             sigil: 'jx-notification' | ||||
|           }); | ||||
|       } | ||||
|       return this._container; | ||||
|     } | ||||
|   }, | ||||
|  | ||||
|   statics : { | ||||
| @@ -74,23 +103,14 @@ JX.install('Notification', { | ||||
|       var self = JX.Notification; | ||||
|  | ||||
|       self._installListener(); | ||||
|       self._active.push({ | ||||
|         object: notification, | ||||
|         render: notification._render() | ||||
|       }); | ||||
|  | ||||
|       // Don't show more than a few notifications at once because it's silly. | ||||
|       while (self._active.length > 5) { | ||||
|         self._hide(self._active[0].object); | ||||
|       } | ||||
|  | ||||
|       self._active.push(notification); | ||||
|       self._redraw(); | ||||
|     }, | ||||
|     _hide : function(notification) { | ||||
|       var self = JX.Notification; | ||||
|  | ||||
|       for (var ii = 0; ii < self._active.length; ii++) { | ||||
|         if (self._active[ii].object === notification) { | ||||
|         if (self._active[ii] === notification) { | ||||
|           notification.invoke('close'); | ||||
|           self._active.splice(ii, 1); | ||||
|           break; | ||||
| @@ -113,17 +133,17 @@ JX.install('Notification', { | ||||
|         'jx-notification', | ||||
|         function(e) { | ||||
|           // NOTE: Don't kill the event since the user might have clicked a | ||||
|           // link, and we want to follow the link if they did. Istead, invoke | ||||
|           // link, and we want to follow the link if they did. Instead, invoke | ||||
|           // the activate event for the active notification and dismiss it if it | ||||
|           // isn't handled. | ||||
|  | ||||
|           var target = e.getNode('jx-notification'); | ||||
|           for (var ii = 0; ii < self._active.length; ii++) { | ||||
|             var n = self._active[ii]; | ||||
|             if (n.render === target) { | ||||
|               var activation = n.object.invoke('activate'); | ||||
|             if (n._getContainer() === target) { | ||||
|               var activation = n.invoke('activate'); | ||||
|               if (!activation.getPrevented()) { | ||||
|                 self._hide(n.object); | ||||
|                 n.hide(); | ||||
|               } | ||||
|               return; | ||||
|             } | ||||
| @@ -151,9 +171,15 @@ JX.install('Notification', { | ||||
|         document.body.appendChild(self._container); | ||||
|       } | ||||
|  | ||||
|       // Show only a limited number of notifications at once. | ||||
|       var limit = 5; | ||||
|  | ||||
|       var notifications = []; | ||||
|       for (var ii = 0; ii < self._active.length; ii++) { | ||||
|         notifications.push(self._active[ii].render); | ||||
|         notifications.push(self._active[ii]._getContainer()); | ||||
|         if (!(--limit)) { | ||||
|           break; | ||||
|         } | ||||
|       } | ||||
|  | ||||
|       JX.DOM.setContent(self._container, notifications); | ||||
|   | ||||
| @@ -7,6 +7,9 @@ | ||||
|  */ | ||||
|  | ||||
| JX.behavior('phabricator-notification-example', function(config) { | ||||
|  | ||||
|   var sequence = 0; | ||||
|  | ||||
|   JX.Stratcom.listen( | ||||
|     'click', | ||||
|     'notification-example', | ||||
| @@ -14,29 +17,51 @@ JX.behavior('phabricator-notification-example', function(config) { | ||||
|       e.kill(); | ||||
|  | ||||
|       var notification = new JX.Notification(); | ||||
|       if (Math.random() > 0.1) { | ||||
|         notification.setContent('It is ' + new Date().toString()); | ||||
|       switch (sequence % 4) { | ||||
|         case 0: | ||||
|           var update = function() { | ||||
|             notification.setContent('It is ' + new Date().toString()); | ||||
|           }; | ||||
|  | ||||
|         notification.listen( | ||||
|           'activate', | ||||
|           function(e) { | ||||
|             if (!confirm("Close notification?")) { | ||||
|               e.kill(); | ||||
|             } | ||||
|           }); | ||||
|       } else { | ||||
|         notification | ||||
|           .setContent('Alert! Click to reload!') | ||||
|           .setDuration(0) | ||||
|           .setClassName('jx-notification-alert'); | ||||
|           update(); | ||||
|           setInterval(update, 1000); | ||||
|  | ||||
|         notification.listen( | ||||
|           'activate', | ||||
|           function(e) { | ||||
|             new JX.$U().go(); | ||||
|           }); | ||||
|           break; | ||||
|         case 1: | ||||
|           notification | ||||
|             .setContent('Permanent alert notification (until clicked).') | ||||
|             .setDuration(0) | ||||
|             .alterClassName('jx-notification-alert', true); | ||||
|           break; | ||||
|         case 2: | ||||
|           notification | ||||
|             .setContent('This notification reacts when you click it.'); | ||||
|  | ||||
|           notification.listen( | ||||
|             'activate', | ||||
|             function() { | ||||
|               if (!confirm("Close notification?")) { | ||||
|                 JX.Stratcom.context().kill(); | ||||
|               } | ||||
|             }); | ||||
|           break; | ||||
|         case 3: | ||||
|           notification | ||||
|             .setDuration(2000) | ||||
|             .setContent('This notification will close after 2 seconds ' + | ||||
|                         'unless you keep clicking it!'); | ||||
|  | ||||
|           notification.listen( | ||||
|             'activate', | ||||
|             function() { | ||||
|               notification.setDuration(2000); | ||||
|               JX.Stratcom.context().kill(); | ||||
|             }); | ||||
|           break; | ||||
|       } | ||||
|       notification.show() | ||||
|  | ||||
|       notification.show(); | ||||
|       sequence++; | ||||
|     }); | ||||
|  | ||||
| }); | ||||
|   | ||||
		Reference in New Issue
	
	Block a user
	 epriestley
					epriestley