Accumulate notifications instead of blinking

This makes it possible to display multiple notifications.
If the options.type is set to "error", it will also add a close button.

apps/files/js/file-upload.js

@@ -472,7 +472,11 @@ OC.Upload = {
 							OC.Upload.showUploadCancelMessage();
 						} else {
 							// HTTP connection problem
-							OC.Notification.showTemporary(data.errorThrown, {timeout: 10});
+							var message = t('files', 'Error uploading file "{fileName}": {message}', {
+								fileName: data.files[0].name,
+								message: data.errorThrown
+							});
+							OC.Notification.show(message, {timeout: 0, type: 'error'});
 							if (data.result) {
 								var result = JSON.parse(data.result);
 								if (result && result[0] && result[0].data && result[0].data.code === 'targetnotfound') {

core/css/styles.css

@@ -669,6 +669,24 @@ td.avatar {
 	cursor: pointer;
 	margin-left: 1em;
 }
+#notification {
+	overflow-x: hidden;
+	overflow-y: auto;
+	max-height: 100px;
+}
+#notification .row {
+	position: relative;
+}
+#notification .row .close {
+	display: inline-block;
+	vertical-align: middle;
+	position: absolute;
+	right: 0;
+	top: 0;
+}
+#notification .row.closeable {
+	padding-right: 20px;
+}
 
 tr .action:not(.permanent),
 .selectedActions a {

core/js/js.js

@@ -988,7 +988,11 @@ OC.msg = {
 OC.Notification={
 	queuedNotifications: [],
 	getDefaultNotificationFunction: null,
-	notificationTimer: 0,
+
+	/**
+	 * @type Array.<int> array of notification timers
+	 */
+	notificationTimers: [],
 
 	/**
 	 * @param callback
@@ -999,25 +1003,64 @@ OC.Notification={
 	},
 
 	/**
-	 * Hides a notification
-	 * @param callback
-	 * @todo Write documentation
+	 * Hides a notification.
+	 *
+	 * If a row is given, only hide that one.
+	 * If no row is given, hide all notifications.
+	 *
+	 * @param {jQuery} [$row] notification row
+	 * @param {Function} [callback] callback
 	 */
-	hide: function(callback) {
-		$('#notification').fadeOut('400', function(){
-			if (OC.Notification.isHidden()) {
-				if (OC.Notification.getDefaultNotificationFunction) {
-					OC.Notification.getDefaultNotificationFunction.call();
-				}
-			}
+	hide: function($row, callback) {
+		var self = this;
+		var $notification = $('#notification');
+
+		if (_.isFunction($row)) {
+			// first arg is the callback
+			callback = $row;
+			$row = undefined;
+		}
+
+		if (!$row) {
+			console.warn('Missing argument $row in OC.Notification.hide() call, caller needs to be adjusted to only dismiss its own notification');
+			// assume that the row to be hidden is the first one
+			$row = $notification.find('.row:first');
+		}
+
+		if ($row && $notification.find('.row').length > 1) {
+			// remove the row directly
+			$row.remove();
 			if (callback) {
 				callback.call();
 			}
-			$('#notification').empty();
-			if(OC.Notification.queuedNotifications.length > 0){
-				OC.Notification.showHtml(OC.Notification.queuedNotifications[0]);
-				OC.Notification.queuedNotifications.shift();
+			return;
+		}
+
+		_.defer(function() {
+			// fade out is supposed to only fade when there is a single row
+			// however, some code might call hide() and show() directly after,
+			// which results in more than one element
+			// in this case, simply delete that one element that was supposed to
+			// fade out
+			//
+			// FIXME: remove once all callers are adjusted to only hide their own notifications
+			if ($notification.find('.row').length > 1) {
+				$row.remove();
+				return;
 			}
+
+			// else, fade out whatever was present
+			$notification.fadeOut('400', function(){
+				if (self.isHidden()) {
+					if (self.getDefaultNotificationFunction) {
+						self.getDefaultNotificationFunction.call();
+					}
+				}
+				if (callback) {
+					callback.call();
+				}
+				$notification.empty();
+			});
 		});
 	},
 
@@ -1025,66 +1068,93 @@ OC.Notification={
 	 * Shows a notification as HTML without being sanitized before.
 	 * If you pass unsanitized user input this may lead to a XSS vulnerability.
 	 * Consider using show() instead of showHTML()
+	 *
 	 * @param {string} html Message to display
-	 */
-	showHtml: function(html) {
-		var notification = $('#notification');
-		if((notification.filter('span.undo').length == 1) || OC.Notification.isHidden()){
-			notification.html(html);
-			notification.fadeIn().css('display','inline-block');
-		}else{
-			OC.Notification.queuedNotifications.push(html);
+	 * @param {Object} [options] options
+	 * @param {string] [options.type] notification type
+	 * @param {int} [options.timeout=0] timeout value, defaults to 0 (permanent)
+	 * @return {jQuery} jQuery element for notification row
+	 */
+	showHtml: function(html, options) {
+		options = options || {};
+		_.defaults(options, {
+			timeout: 0
+		});
+
+		var self = this;
+		var $notification = $('#notification');
+		if (this.isHidden()) {
+			$notification.fadeIn().css('display','inline-block');
+		}
+		var $row = $('<div class="row"></div>');
+		if (options.type) {
+			$row.addClass('type-' + options.type);
+		}
+		if (options.type === 'error') {
+			// add a close button
+			var $closeButton = $('<a class="action close icon-close" href="#"></a>');
+			$closeButton.attr('alt', t('core', 'Dismiss'));
+			$row.append($closeButton);
+			$closeButton.one('click', function() {
+				self.hide($row);
+				return false;
+			});
+			$row.addClass('closeable');
 		}
+
+		$row.prepend(html);
+		$notification.append($row);
+
+		if(options.timeout > 0) {
+			// register timeout to vanish notification
+			this.notificationTimers.push(setTimeout(function() {
+				self.hide($row);
+			}, (options.timeout * 1000)));
+		}
+
+		return $row;
 	},
 
 	/**
 	 * Shows a sanitized notification
+	 *
 	 * @param {string} text Message to display
+	 * @param {Object} [options] options
+	 * @param {string] [options.type] notification type
+	 * @param {int} [options.timeout=0] timeout value, defaults to 0 (permanent)
+	 * @return {jQuery} jQuery element for notification row
 	 */
-	show: function(text) {
-		var notification = $('#notification');
-		if((notification.filter('span.undo').length == 1) || OC.Notification.isHidden()){
-			notification.text(text);
-			notification.fadeIn().css('display','inline-block');
-		}else{
-			OC.Notification.queuedNotifications.push($('<div/>').text(text).html());
-		}
+	show: function(text, options) {
+		return this.showHtml($('<div/>').text(text).html(), options);
 	},
 
-
 	/**
 	 * Shows a notification that disappears after x seconds, default is
 	 * 7 seconds
+	 *
 	 * @param {string} text Message to show
 	 * @param {array} [options] options array
 	 * @param {int} [options.timeout=7] timeout in seconds, if this is 0 it will show the message permanently
 	 * @param {boolean} [options.isHTML=false] an indicator for HTML notifications (true) or text (false)
+	 * @param {string] [options.type] notification type
 	 */
 	showTemporary: function(text, options) {
+		var self = this;
 		var defaults = {
-				isHTML: false,
-				timeout: 7
-			},
-			options = options || {};
+			isHTML: false,
+			timeout: 7
+		};
+		options = options || {};
 		// merge defaults with passed in options
 		_.defaults(options, defaults);
 
-		// clear previous notifications
-		OC.Notification.hide();
-		if(OC.Notification.notificationTimer) {
-			clearTimeout(OC.Notification.notificationTimer);
-		}
-
+		var $row;
 		if(options.isHTML) {
-			OC.Notification.showHtml(text);
+			$row = this.showHtml(text, options);
 		} else {
-			OC.Notification.show(text);
-		}
-
-		if(options.timeout > 0) {
-			// register timeout to vanish notification
-			OC.Notification.notificationTimer = setTimeout(OC.Notification.hide, (options.timeout * 1000));
+			$row = this.show(text, options);
 		}
+		return $row;
 	},
 
 	/**
@@ -1092,7 +1162,7 @@ OC.Notification={
 	 * @return {boolean}
 	 */
 	isHidden: function() {
-		return ($("#notification").text() === '');
+		return !$("#notification").find('.row').length;
 	}
 };
 

core/js/tests/specs/coreSpec.js

@@ -747,100 +747,181 @@ describe('Core base tests', function() {
 		});
 	});
 	describe('Notifications', function() {
-		beforeEach(function(){
-			notificationMock = sinon.mock(OC.Notification);
+		var showSpy;
+		var showHtmlSpy;
+		var hideSpy;
+		var clock;
+
+		beforeEach(function() {
+			clock = sinon.useFakeTimers();
+			showSpy = sinon.spy(OC.Notification, 'show');
+			showHtmlSpy = sinon.spy(OC.Notification, 'showHtml');
+			hideSpy = sinon.spy(OC.Notification, 'hide');
+
+			$('#testArea').append('<div id="notification"></div>');
 		});
-		afterEach(function(){
-			// verify that all expectations are met
-			notificationMock.verify();
-			// restore mocked methods
-			notificationMock.restore();
-			// clean up the global variable
-			delete notificationMock;
+		afterEach(function() {
+			showSpy.restore();
+			showHtmlSpy.restore();
+			hideSpy.restore();
+			// jump past animations
+			clock.tick(10000);
+			clock.restore();
 		});
-		it('Should show a plain text notification' , function() {
-			// one is shown ...
-			notificationMock.expects('show').once().withExactArgs('My notification test');
-			// ... but not the HTML one
-			notificationMock.expects('showHtml').never();
+		describe('showTemporary', function() {
+			it('shows a plain text notification with default timeout', function() {
+				var $row = OC.Notification.showTemporary('My notification test');
 
-			OC.Notification.showTemporary('My notification test');
+				expect(showSpy.calledOnce).toEqual(true);
+				expect(showSpy.firstCall.args[0]).toEqual('My notification test');
+				expect(showSpy.firstCall.args[1]).toEqual({isHTML: false, timeout: 7});
 
-			// verification is done in afterEach
+				expect($row).toBeDefined();
+				expect($row.text()).toEqual('My notification test');
+			});
+			it('shows a HTML notification with default timeout', function() {
+				var $row = OC.Notification.showTemporary('<a>My notification test</a>', { isHTML: true });
+
+				expect(showSpy.notCalled).toEqual(true);
+				expect(showHtmlSpy.calledOnce).toEqual(true);
+				expect(showHtmlSpy.firstCall.args[0]).toEqual('<a>My notification test</a>');
+				expect(showHtmlSpy.firstCall.args[1]).toEqual({isHTML: true, timeout: 7});
+
+				expect($row).toBeDefined();
+				expect($row.text()).toEqual('My notification test');
+			});
+			it('hides itself after 7 seconds', function() {
+				var $row = OC.Notification.showTemporary('');
+
+				// travel in time +7000 milliseconds
+				clock.tick(7000);
+
+				expect(hideSpy.calledOnce).toEqual(true);
+				expect(hideSpy.firstCall.args[0]).toEqual($row);
+			});
 		});
-		it('Should show a HTML notification' , function() {
-			// no plain is shown ...
-			notificationMock.expects('show').never();
-			// ... but one HTML notification
-			notificationMock.expects('showHtml').once().withExactArgs('<a>My notification test</a>');
+		describe('show', function() {
+			it('hides itself after a given time', function() {
+				OC.Notification.show('', { timeout: 10 });
+
+				// travel in time +9 seconds
+				clock.tick(9000);
 
-			OC.Notification.showTemporary('<a>My notification test</a>', { isHTML: true });
+				expect(hideSpy.notCalled).toEqual(true);
 
-			// verification is done in afterEach
+				// travel in time +1 seconds
+				clock.tick(1000);
+
+				expect(hideSpy.calledOnce).toEqual(true);
+			});
+			it('does not hide itself after a given time if a timeout of 0 is defined', function() {
+				OC.Notification.show('', { timeout: 0 });
+
+				// travel in time +1000 seconds
+				clock.tick(1000000);
+
+				expect(hideSpy.notCalled).toEqual(true);
+			});
+			it('does not hide itself if no timeout given to show', function() {
+				OC.Notification.show('');
+
+				// travel in time +1000 seconds
+				clock.tick(1000000);
+
+				expect(hideSpy.notCalled).toEqual(true);
+			});
 		});
-		it('Should hide previous notification and hide itself after 7 seconds' , function() {
-			var clock = sinon.useFakeTimers();
+		it('cumulates several notifications', function() {
+			var $row1 = OC.Notification.showTemporary('One');
+			var $row2 = OC.Notification.showTemporary('Two', {timeout: 2});
+			var $row3 = OC.Notification.showTemporary('Three');
+
+			var $el = $('#notification');
+			var $rows = $el.find('.row');
+			expect($rows.length).toEqual(3);
 
-			// previous notifications get hidden
-			notificationMock.expects('hide').once();
+			expect($rows.eq(0).is($row1)).toEqual(true);
+			expect($rows.eq(1).is($row2)).toEqual(true);
+			expect($rows.eq(2).is($row3)).toEqual(true);
 
-			OC.Notification.showTemporary('');
+			clock.tick(3000);
 
-			// verify the first call
-			notificationMock.verify();
+			$rows = $el.find('.row');
+			expect($rows.length).toEqual(2);
 
-			// expect it a second time
-			notificationMock.expects('hide').once();
+			expect($rows.eq(0).is($row1)).toEqual(true);
+			expect($rows.eq(1).is($row3)).toEqual(true);
+		});
+		it('shows close button for error types', function() {
+			var $row = OC.Notification.showTemporary('One');
+			var $rowError = OC.Notification.showTemporary('Two', {type: 'error'});
+			expect($row.find('.close').length).toEqual(0);
+			expect($rowError.find('.close').length).toEqual(1);
 
-			// travel in time +7000 milliseconds
-			clock.tick(7000);
+			// after clicking, row is gone
+			$rowError.find('.close').click();
 
-			// verification is done in afterEach
+			var $rows = $('#notification').find('.row');
+			expect($rows.length).toEqual(1);
+			expect($rows.eq(0).is($row)).toEqual(true);
 		});
-		it('Should hide itself after a given time' , function() {
-			var clock = sinon.useFakeTimers();
+		it('fades out the last notification but not the other ones', function() {
+			var fadeOutStub = sinon.stub($.fn, 'fadeOut');
+			var $row1 = OC.Notification.show('One', {type: 'error'});
+			var $row2 = OC.Notification.show('Two', {type: 'error'});
+			OC.Notification.showTemporary('Three', {timeout: 2});
 
-			// previous notifications get hidden
-			notificationMock.expects('hide').once();
+			var $el = $('#notification');
+			var $rows = $el.find('.row');
+			expect($rows.length).toEqual(3);
 
-			OC.Notification.showTemporary('', { timeout: 10 });
+			clock.tick(3000);
 
-			// verify the first call
-			notificationMock.verify();
+			$rows = $el.find('.row');
+			expect($rows.length).toEqual(2);
 
-			// expect to not be called after 9 seconds
-			notificationMock.expects('hide').never();
+			$row1.find('.close').click();
+			clock.tick(1000);
 
-			// travel in time +9 seconds
-			clock.tick(9000);
-			// verify this
-			notificationMock.verify();
+			expect(fadeOutStub.notCalled).toEqual(true);
 
-			// expect the second call one second later
-			notificationMock.expects('hide').once();
-			// travel in time +1 seconds
+			$row2.find('.close').click();
 			clock.tick(1000);
+			expect(fadeOutStub.calledOnce).toEqual(true);
 
-			// verification is done in afterEach
+			expect($el.is(':empty')).toEqual(false);
+			fadeOutStub.yield();
+			expect($el.is(':empty')).toEqual(true);
+
+			fadeOutStub.restore();
 		});
-		it('Should not hide itself after a given time if a timeout of 0 is defined' , function() {
-			var clock = sinon.useFakeTimers();
+		it('hides the first notification when calling hide without arguments', function() {
+			var $row1 = OC.Notification.show('One');
+			var $row2 = OC.Notification.show('Two');
 
-			// previous notifications get hidden
-			notificationMock.expects('hide').once();
+			var $el = $('#notification');
+			var $rows = $el.find('.row');
+			expect($rows.length).toEqual(2);
 
-			OC.Notification.showTemporary('', { timeout: 0 });
+			OC.Notification.hide();
 
-			// verify the first call
-			notificationMock.verify();
+			$rows = $el.find('.row');
+			expect($rows.length).toEqual(1);
+			expect($rows.eq(0).is($row2)).toEqual(true);
+		});
+		it('hides the given notification when calling hide with argument', function() {
+			var $row1 = OC.Notification.show('One');
+			var $row2 = OC.Notification.show('Two');
 
-			// expect to not be called after 1000 seconds
-			notificationMock.expects('hide').never();
+			var $el = $('#notification');
+			var $rows = $el.find('.row');
+			expect($rows.length).toEqual(2);
 
-			// travel in time +1000 seconds
-			clock.tick(1000000);
+			OC.Notification.hide($row2);
 
-			// verification is done in afterEach
+			$rows = $el.find('.row');
+			expect($rows.length).toEqual(1);
+			expect($rows.eq(0).is($row1)).toEqual(true);
 		});
 	});
 	describe('global ajax errors', function() {