From 8ea80e114ae508586947db614b37db6b35972a9c Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Fri, 19 Feb 2016 12:09:46 +0100 Subject: [PATCH] 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 | 6 +- core/css/styles.css | 18 +++ core/js/js.js | 170 ++++++++++++++++++-------- core/js/tests/specs/coreSpec.js | 209 ++++++++++++++++++++++---------- 4 files changed, 288 insertions(+), 115 deletions(-) diff --git a/apps/files/js/file-upload.js b/apps/files/js/file-upload.js index 8ba294e2a7f..bd80afd072c 100644 --- a/apps/files/js/file-upload.js +++ b/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') { diff --git a/core/css/styles.css b/core/css/styles.css index e78db6fde01..2fd350f3c4a 100644 --- a/core/css/styles.css +++ b/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 { diff --git a/core/js/js.js b/core/js/js.js index fac9c45f668..e90ceaf4e18 100644 --- a/core/js/js.js +++ b/core/js/js.js @@ -988,7 +988,11 @@ OC.msg = { OC.Notification={ queuedNotifications: [], getDefaultNotificationFunction: null, - notificationTimer: 0, + + /** + * @type Array. 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 = $('
'); + if (options.type) { + $row.addClass('type-' + options.type); + } + if (options.type === 'error') { + // add a close button + var $closeButton = $(''); + $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($('
').text(text).html()); - } + show: function(text, options) { + return this.showHtml($('
').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; } }; diff --git a/core/js/tests/specs/coreSpec.js b/core/js/tests/specs/coreSpec.js index 32eb8df32d1..774c2fdc72f 100644 --- a/core/js/tests/specs/coreSpec.js +++ b/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('
'); }); - 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('My notification test', { isHTML: true }); + + expect(showSpy.notCalled).toEqual(true); + expect(showHtmlSpy.calledOnce).toEqual(true); + expect(showHtmlSpy.firstCall.args[0]).toEqual('My notification test'); + 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('My notification test'); + 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('My notification test', { 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() { -- 2.39.5