Merge lp:~frankban/juju-gui/bug-1076404-growl into lp:juju-gui/experimental

[Francesco Banconi]

Growl-style notifications.

This branch introduces growl-style notifications
in the top-right corner of the window.
Implemented a notifier widget: it is a reusable
piece of code that just need a title, a message
and a timeout to display a notification. It is used
to render notification outside view containers, so
that notifications are preserved when the user changes
the current view.

A notifier is created when a notification is added,
following these rules:
- the notification is an error;
- the notification is local (it's not related to the
  delta stream).

The last point involves adding a new field "isDelta"
to the notification model, defaulting to false.
Notifications created when the delta stream arrives are
marked as "isDelta: true".

UI: new notifications appear on top, and disappear after
8 seconds. Mouse over prevents a notification to hide,
mouse click destroys a notification.

Also fixed a bug in the unit view: a callable was undefined
when trying to "retry" or "resolve" a unit.

https://codereview.appspot.com/6851058/

[Francesco Banconi (frankban)]

Reviewers: mp+134660_code.launchpad.net,

Message:
Please take a look.

Description:
Growl-style notifications.

This branch introduces growl-style notifications
in the top-right corner of the window.
Implemented a notifier widget: it is a reusable
piece of code that just need a title, a message
and a timeout to display a notification. It is used
to render notification outside view containers, so
that notifications are preserved when the user changes
the current view.

A notifier is created when a notification is added,
following these rules:
- the notification is an error;
- the notification is local (it's not related to the
   delta stream).

The last point involves adding a new field "isDelta"
to the notification model, defaulting to false.
Notifications created when the delta stream arrives are
marked as "isDelta: true".

UI: new notifications appear on top, and disappear after
8 seconds. Mouse over prevents a notification to hide,
mouse click destroys a notification.

Also fixed a bug in the unit view: a callable was undefined
when trying to "retry" or "resolve" a unit.

This branch is intended to be a first implementation proposal
of the growl notifications. It doesn't solve open issues like:
- how to handle/aggregate multiple notifications;
- how to correctly generate and format titles and messages;
- where notifications should appear in the window.

https://code.launchpad.net/~frankban/juju-gui/bug-1076404-growl/+merge/134660

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/6851058/

Affected files:
   A [revision details]
   M app/app.js
   A app/assets/images/notifier-error.png
   M app/index.html
   M app/models/models.js
   M app/modules-debug.js
   M app/store/notifications.js
   A app/templates/notifier.handlebars
   M app/views/notifications.js
   A app/widgets/notifier.js
   M lib/views/stylesheet.less
   M test/index.html
   M test/test_notifications.js
   A test/test_notifier_widget.js

[Thiago Veronezi (tveronezi)]

Thanks, Francesco.
I like it! I have just one really minor comment.

[]s,
Thiago.

https://codereview.appspot.com/6851058/diff/1/app/views/notifications.js
File app/views/notifications.js (right):

https://codereview.appspot.com/6851058/diff/1/app/views/notifications.js#newcode50
app/views/notifications.js:50: if (notifierBox &&
I think will you always have the 'notifier-box' element. It is
hard-coded in index.html. So probably you can remove this check.

https://codereview.appspot.com/6851058/

[Francesco Banconi (frankban)]

Thanks for the review Thiago! A reply to your comment follows.

https://codereview.appspot.com/6851058/diff/1/app/views/notifications.js
File app/views/notifications.js (right):

https://codereview.appspot.com/6851058/diff/1/app/views/notifications.js#newcode50
app/views/notifications.js:50: if (notifierBox &&
On 2012/11/16 14:39:29, thiago wrote:
> I think will you always have the 'notifier-box' element. It is
hard-coded in
> index.html. So probably you can remove this check.

The check is here to make tests work. The element is not present in the
test page, and it's created by tests' setUp and tearDown only when
needed, so that notifications are not created as side effects by
non-relevant test cases.

https://codereview.appspot.com/6851058/

[Madison Scott-Clary (makyo)]

This all looks pretty good to me, Francesco! Thanks for the branch.
Works well, tests pass.

https://codereview.appspot.com/6851058/diff/1/lib/views/stylesheet.less
File lib/views/stylesheet.less (right):

https://codereview.appspot.com/6851058/diff/1/lib/views/stylesheet.less#newcode592
lib/views/stylesheet.less:592: opacity: @opacity - 0.1;
Less is so neat, sometimes :)

https://codereview.appspot.com/6851058/

[Francesco Banconi (frankban)]

*** Submitted:

Growl-style notifications.

This branch introduces growl-style notifications
in the top-right corner of the window.
Implemented a notifier widget: it is a reusable
piece of code that just need a title, a message
and a timeout to display a notification. It is used
to render notification outside view containers, so
that notifications are preserved when the user changes
the current view.

A notifier is created when a notification is added,
following these rules:
- the notification is an error;
- the notification is local (it's not related to the
   delta stream).

The last point involves adding a new field "isDelta"
to the notification model, defaulting to false.
Notifications created when the delta stream arrives are
marked as "isDelta: true".

UI: new notifications appear on top, and disappear after
8 seconds. Mouse over prevents a notification to hide,
mouse click destroys a notification.

Also fixed a bug in the unit view: a callable was undefined
when trying to "retry" or "resolve" a unit.

R=thiago, matthew.scott
CC=
https://codereview.appspot.com/6851058

https://codereview.appspot.com/6851058/

=== modified file 'app/app.js'
--- app/app.js	2012-11-07 12:35:06 +0000
+++ app/app.js	2012-11-16 13:56:21 +0000
@@ -280,7 +280,8 @@
           'unit',
           // The querystring is used to handle highlighting relation rows in
           // links from notifications about errors.
-          { unit: unit, db: this.db, env: this.env,
+          { getModelURL: Y.bind(this.getModelURL, this),
+            unit: unit, db: this.db, env: this.env,
             querystring: req.query });
     },
 

=== added file 'app/assets/images/notifier-error.png'
Binary files app/assets/images/notifier-error.png	1970-01-01 00:00:00 +0000 and app/assets/images/notifier-error.png	2012-11-16 13:56:21 +0000 differ
=== modified file 'app/index.html'
--- app/index.html	2012-11-13 18:18:47 +0000
+++ app/index.html	2012-11-16 13:56:21 +0000
@@ -18,6 +18,7 @@
 
   <body>
 
+      <div id="notifier-box"></div>
       <div id="viewport-wrapper">
         <div id="vp-left-border"></div>
         <div id="viewport">

=== modified file 'app/models/models.js'
--- app/models/models.js	2012-10-19 01:44:45 +0000
+++ app/models/models.js	2012-11-16 13:56:21 +0000
@@ -309,6 +309,8 @@
               [model.name,
                (model instanceof Y.Model) ? model.get('id') : model.id]);
         }},
+      // Whether or not the notification is related to the delta stream.
+      isDelta: {value: false},
       link: {},
       link_title: {
         value: 'View Details'

=== modified file 'app/modules-debug.js'
--- app/modules-debug.js	2012-11-13 16:46:36 +0000
+++ app/modules-debug.js	2012-11-16 13:56:21 +0000
@@ -24,6 +24,10 @@
       modules: {
         // Primitives
 
+        'notifier': {
+          fullpath: '/juju-ui/widgets/notifier.js'
+        },
+
         'svg-layouts': {
           fullpath: '/juju-ui/assets/javascripts/svg-layouts.js'
         },

=== modified file 'app/store/notifications.js'
--- app/store/notifications.js	2012-10-15 10:07:49 +0000
+++ app/store/notifications.js	2012-11-16 13:56:21 +0000
@@ -156,7 +156,7 @@
             var change_type = change[0],
                     change_op = change[1],
                     change_data = change[2],
-                    notify_data = {},
+                    notify_data = {isDelta: true},
                     rule = rules[change_type],
                     model;
 

=== added file 'app/templates/notifier.handlebars'
--- app/templates/notifier.handlebars	1970-01-01 00:00:00 +0000
+++ app/templates/notifier.handlebars	2012-11-16 13:56:21 +0000
@@ -0,0 +1,3 @@
+<i class="sprite notifier-error"></i>
+<h3>{{title}}</h3>
+<div>{{message}}</div>

=== modified file 'app/views/notifications.js'
--- app/views/notifications.js	2012-10-05 13:08:43 +0000
+++ app/views/notifications.js	2012-11-16 13:56:21 +0000
@@ -3,10 +3,11 @@
 YUI.add('juju-notifications', function(Y) {
 
   var views = Y.namespace('juju.views'),
+      widgets = Y.namespace('juju.widgets'),
       Templates = views.Templates;
 
   /*
-   * Abstract Base class used to view a ModelList  of notifications
+   * Abstract Base class used to view a ModelList of notifications
    */
   var NotificationsBaseView = Y.Base.create('NotificationsBaseView',
       Y.View, [views.JujuBaseView], {
@@ -24,10 +25,36 @@
           notifications.after('create', this.slowRender, this);
           notifications.after('remove', this.slowRender, this);
           notifications.after('reset', this.slowRender, this);
+          // Bind new notifications to the notifier widget.
+          notifications.after('add', this.addNotifier, this);
 
           // Env connection state watcher
           env.on('connectedChange', this.slowRender, this);
+        },
 
+        /**
+         * Create and display a notifier widget when a notification is added.
+         * The notifier is created only if:
+         * - the notifier box exists in the DOM;
+         * - the notification is a local one (not related to the delta stream);
+         * - the notification is an error.
+         *
+         * @method addNotifier
+         * @param {Object} ev An event object (with a "model" attribute).
+         * @return {undefined} Mutates only.
+         */
+        addNotifier: function(ev) {
+          var notification = ev.model,
+              notifierBox = Y.one('#notifier-box');
+          // Show error notifications only if the DOM contain the notifier box.
+          if (notifierBox &&
+              !notification.get('isDelta') &&
+              notification.get('level') === 'error') {
+            new widgets.Notifier({
+              title: notification.get('title'),
+              message: notification.get('message')
+            }).render(notifierBox);
+          }
         },
 
         /*
@@ -251,6 +278,7 @@
     'view',
     'juju-view-utils',
     'node',
-    'handlebars'
+    'handlebars',
+    'notifier'
   ]
 });

=== added directory 'app/widgets'
=== added file 'app/widgets/notifier.js'
--- app/widgets/notifier.js	1970-01-01 00:00:00 +0000
+++ app/widgets/notifier.js	2012-11-16 13:56:21 +0000
@@ -0,0 +1,148 @@
+'use strict';
+
+YUI.add('notifier', function(Y) {
+
+  var widgets = Y.namespace('juju.widgets');
+
+  /**
+   * Display a notification.
+   * This is the constructor for the notifier widget.
+   *
+   * @class Notifier
+   * @namespace widgets
+   */
+  function Notifier(config) {
+    Notifier.superclass.constructor.apply(this, arguments);
+  }
+
+  Notifier.NAME = 'Notifier';
+  Notifier.ATTRS = {
+    title: {value: ''},
+    message: {value: ''},
+    timeout: {value: 8000}
+  };
+
+  /**
+   * Define the widget class extending Y.Widget.
+   *
+   * @class Notifier
+   * @namespace widgets
+   */
+  Y.extend(Notifier, Y.Widget, {
+
+    CONTENT_TEMPLATE: null,
+    TEMPLATE: Y.namespace('juju.views').Templates.notifier,
+
+    /**
+     * Attach the widget bounding box to the DOM.
+     * Override to insert new instances before existing ones.
+     * This way new notification are displayed on top of the page.
+     * The resulting rendering process is also very simplified.
+     *
+     * @method _renderBox
+     * @protected
+     * @param {Y.Node object} parentNode The node containing this widget.
+     * @return {undefined} Mutates only.
+     */
+    _renderBox: function(parentNode) {
+      parentNode.prepend(this.get('boundingBox'));
+    },
+
+    /**
+     * Create the nodes required by this widget and attach them to the DOM.
+     *
+     * @method renderUI
+     * @return {undefined} Mutates only.
+     */
+    renderUI: function() {
+      var content = this.TEMPLATE({
+        title: this.get('title'),
+        message: this.get('message')
+      });
+      this.get('contentBox').append(content);
+    },
+
+    /**
+     * Attach event listeners which bind the UI to the widget state.
+     * The mouse enter event on a notification node pauses the timer.
+     * The mouse click event on a notification destroys the widget.
+     *
+     * @method bindUI
+     * @return {undefined} Mutates only.
+     */
+    bindUI: function() {
+      var contentBox = this.get('contentBox');
+      contentBox.on(
+          'hover',
+          function() {
+            if (this.timer) {
+              this.timer.pause();
+            }
+          },
+          function() {
+            if (this.timer) {
+              this.timer.resume();
+            }
+          },
+          this
+      );
+      contentBox.on('click', function(ev) {
+        this.hideAndDestroy();
+        ev.halt();
+      }, this);
+    },
+
+    /**
+     * Create and start the timer that will destroy the widget after N seconds.
+     *
+     * @method syncUI
+     * @return {undefined} Mutates only.
+     */
+    syncUI: function() {
+      this.timer = new Y.Timer({
+        length: this.get('timeout'),
+        repeatCount: 1,
+        callback: Y.bind(this.hideAndDestroy, this)
+      });
+      this.timer.start();
+    },
+
+    /**
+     * Hide this widget using an animation and destroy the widget at the end.
+     *
+     * @method hideAndDestroy
+     * @return {undefined} Mutates only.
+     */
+    hideAndDestroy: function() {
+      this.timer.stop();
+      this.timer = null;
+      if (this.get('boundingBox').getDOMNode()) {
+        // Animate and destroy the notification if it still exists in the DOM.
+        var anim = new Y.Anim({
+          node: this.get('boundingBox'),
+          to: {opacity: 0},
+          easing: 'easeIn',
+          duration: 0.2
+        });
+        anim.on('end', this.destroy, this);
+        anim.run();
+      } else {
+        // Otherwise, just destroy the notification.
+        this.destroy();
+      }
+    }
+
+  });
+
+  widgets.Notifier = Notifier;
+
+}, '0.1.0', {requires: [
+  'anim',
+  'event',
+  'event-hover',
+  'handlebars',
+  'gallery-timer',
+  'juju-templates',
+  'node',
+  'widget'
+]});

=== modified file 'lib/views/stylesheet.less'
--- lib/views/stylesheet.less	2012-11-15 14:33:54 +0000
+++ lib/views/stylesheet.less	2012-11-16 13:56:21 +0000
@@ -565,6 +565,56 @@
 }
 
 /*
+ * Notifier widget.
+ */
+#notifier-box {
+    position: absolute;
+    right: 0px;
+    top: 0px;
+    z-index: 9999;
+    .yui3-notifier-content {
+        @background-color: black;
+        @margin-left: 41px;
+        @margin-top: 13px;
+        @opacity: 0.8;
+        .create-border-radius(8px);
+        // Using a variable here because LESS strips commas in mixin args.
+        @box-shadow: 0 2px 4px lighten(@background-color, 10%),
+            0 1px 2px lighten(@background-color, 20%) inset;
+        .create-box-shadow(@box-shadow);
+        background-color: @background-color;
+        margin: 6px;
+        opacity: @opacity;
+        overflow: hidden;
+        width: 277px;
+        &:hover {
+            cursor: pointer;
+            opacity: @opacity - 0.1;
+        }
+        h3 {
+            color: #FDF6E3;
+            font-size: 16px;
+            font-weight: normal;
+            margin-top: @margin-top;
+            margin-left: @margin-left;
+        }
+        div {
+            color: #DDD7C6;
+            font-size: 12px;
+            line-height: 16px;
+            margin-left: @margin-left;
+            margin-bottom: 12px;
+        }
+        .sprite {
+            float: left;
+            margin-top: @margin-top + 5px;
+            margin-left: 12px;
+            opacity: 1;
+        }
+    }
+}
+
+/*
  * Overview Module
  */
 #overview {

=== modified file 'test/index.html'
--- test/index.html	2012-11-13 13:55:16 +0000
+++ test/index.html	2012-11-16 13:56:21 +0000
@@ -33,6 +33,7 @@
   <script src="test_endpoints.js"></script>
   <script src="test_application_notifications.js"></script>
   <script src="test_charm_store.js"></script>
+  <script src="test_notifier_widget.js"></script>
 
   <script>
   YUI().use('node', 'event', function(Y) {

=== modified file 'test/test_notifications.js'
--- test/test_notifications.js	2012-10-18 15:12:38 +0000
+++ test/test_notifications.js	2012-11-16 13:56:21 +0000
@@ -414,3 +414,64 @@
         'Relation with endpoint1 (relation type "relation1") was created');
   });
 });
+
+describe('notification visual feedback', function() {
+  var env, models, notifications, notificationsView, notifierBox, views, Y;
+
+  before(function(done) {
+    Y = YUI(GlobalConfig).use('juju-env', 'juju-models', 'juju-views',
+        function(Y) {
+          var juju = Y.namespace('juju');
+          env = new juju.Environment();
+          models = Y.namespace('juju.models');
+          views = Y.namespace('juju.views');
+          done();
+        });
+  });
+
+  // Instantiate the notifications model list and view.
+  // Also create the notifier box and attach it as first element of the body.
+  beforeEach(function() {
+    notifications = new models.NotificationList();
+    notificationsView = new views.NotificationsView({
+      env: env,
+      notifications: notifications
+    });
+    notifierBox = Y.Node.create('<div id="notifier-box"></div>');
+    notifierBox.setStyle('display', 'none');
+    Y.one('body').prepend(notifierBox);
+  });
+
+  // Destroy the notifier box created in beforeEach.
+  afterEach(function() {
+    notifierBox.remove();
+    notifierBox.destroy(true);
+  });
+
+  // Assert the notifier box contains the expectedNumber of notifiers.
+  var assertNumNotifiers = function(expectedNumber) {
+    assert.equal(expectedNumber, notifierBox.get('children').size());
+  };
+
+  it('should appear when a new error is notified', function() {
+    notifications.add({title: 'mytitle', level: 'error'});
+    assertNumNotifiers(1);
+  });
+
+  it('should only appear when the DOM contains the notifier box', function() {
+    notifierBox.remove();
+    notifications.add({title: 'mytitle', level: 'error'});
+    assertNumNotifiers(0);
+  });
+
+  it('should not appear when the notification is not an error', function() {
+    notifications.add({title: 'mytitle', level: 'info'});
+    assertNumNotifiers(0);
+  });
+
+  it('should not appear when the notification comes form delta', function() {
+    notifications.add({title: 'mytitle', level: 'error', isDelta: true});
+    assertNumNotifiers(0);
+  });
+
+});

=== added file 'test/test_notifier_widget.js'
--- test/test_notifier_widget.js	1970-01-01 00:00:00 +0000
+++ test/test_notifier_widget.js	2012-11-16 13:56:21 +0000
@@ -0,0 +1,103 @@
+'use strict';
+
+describe('notifier widget', function() {
+  var Notifier, notifierBox, Y;
+
+  before(function(done) {
+    Y = YUI(GlobalConfig).use('notifier', 'node-event-simulate',
+        function(Y) {
+          Notifier = Y.namespace('juju.widgets').Notifier;
+          done();
+        });
+  });
+
+  // Create the notifier box and attach it as first element of the body.
+  beforeEach(function() {
+    notifierBox = Y.Node.create('<div id="notifier-box"></div>');
+    notifierBox.setStyle('display', 'none');
+    Y.one('body').prepend(notifierBox);
+  });
+
+  // Destroy the notifier box created in beforeEach.
+  afterEach(function() {
+    notifierBox.remove();
+    notifierBox.destroy(true);
+  });
+
+  // Factory rendering and returning a notifier instance.
+  var makeNotifier = function(title, message, timeout) {
+    var notifier = new Notifier({
+      title: title || 'mytitle',
+      message: message || 'mymessage',
+      timeout: timeout || 10000
+    });
+    notifier.render(notifierBox);
+    return notifier;
+  };
+
+  // Assert the notifier box contains the expectedNumber of notifiers.
+  var assertNumNotifiers = function(expectedNumber) {
+    assert.equal(expectedNumber, notifierBox.get('children').size());
+  };
+
+  it('should be able to display a notification', function() {
+    makeNotifier();
+    assertNumNotifiers(1);
+  });
+
+  it('should display the given title and message', function() {
+    makeNotifier('mytitle', 'mymessage');
+    var notifierNode = notifierBox.one('*');
+    assert.equal('mytitle', notifierNode.one('h3').getContent());
+    assert.equal('mymessage', notifierNode.one('div').getContent());
+  });
+
+  it('should be able to display multiple notifications', function() {
+    var number = 10;
+    for (var i = 0; i < number; i += 1) {
+      makeNotifier();
+    }
+    assertNumNotifiers(number);
+  });
+
+  it('should display new notifications on top', function() {
+    makeNotifier('mytitle1', 'mymessage1');
+    makeNotifier('mytitle2', 'mymessage2');
+    var notifierNode = notifierBox.one('*');
+    assert.equal('mytitle2', notifierNode.one('h3').getContent());
+    assert.equal('mymessage2', notifierNode.one('div').getContent());
+  });
+
+  it('should destroy notifications after N milliseconds', function(done) {
+    makeNotifier('mytitle', 'mymessage', 1);
+    // A timeout of 250 milliseconds is used so that we ensure the destroying
+    // animation can be completed.
+    setTimeout(function() {
+      assertNumNotifiers(0);
+      done();
+    }, 250);
+  });
+
+  it('should destroy notifications on click', function(done) {
+    makeNotifier();
+    notifierBox.one('*').simulate('click');
+    // A timeout of 250 milliseconds is used so that we ensure the destroying
+    // animation can be completed.
+    setTimeout(function() {
+      assertNumNotifiers(0);
+      done();
+    }, 250);
+  });
+
+  it('should prevent notification removal on mouse enter', function(done) {
+    makeNotifier('mytitle', 'mymessage', 1);
+    notifierBox.one('*').simulate('mouseover');
+    // A timeout of 250 milliseconds is used so that we ensure the node is not
+    // preserved by the destroying animation.
+    setTimeout(function() {
+      assertNumNotifiers(1);
+      done();
+    }, 250);
+  });
+
+});