yellow team mailing list archive
-
yellow team
-
Mailing list archive
-
Message #01621
[Merge] lp:~frankban/juju-gui/bug-1076404-growl into lp:juju-gui
Francesco Banconi has proposed merging lp:~frankban/juju-gui/bug-1076404-growl into lp:juju-gui.
Requested reviews:
Juju GUI Hackers (juju-gui)
Related bugs:
Bug #1076404 in juju-ui: "User notifications need better immediate feedback"
https://bugs.launchpad.net/juju-gui/+bug/1076404
For more details, see:
https://code.launchpad.net/~frankban/juju-gui/bug-1076404-growl/+merge/134660
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://codereview.appspot.com/6851058/
--
https://code.launchpad.net/~frankban/juju-gui/bug-1076404-growl/+merge/134660
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~frankban/juju-gui/bug-1076404-growl into lp:juju-gui.
=== 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);
+ });
+
+});
Follow ups
