Fix default timeouts in OC.Notification

When no timeout was given "show()" used the default timeout of
"OCP.Toast", which is 7 seconds instead of indefinitely as stated in the
documentation of "show()". "showHtml()" should also indefinitely show
the notification if no timeout is given, but due to the strict
comparison the notification was indefinitely shown only when a timeout
of 0 was explicitly given. Now both methods show the notification
indefinitely (or until it is explicitly hidden) when no timeout is
given.

The unit tests did not catch this error because "showHtml()" had no
tests (as before the move to Toastify it was called from "show()" and
thus implicitly tested), and because "show()" verified that "hide()" was
not called after some time; "hide()" is no longer called from "show()"
since "OCP.Toast" is used internally, so the test always passed even if
the notification was indeed hidden. Now the test is based on whether the
element is found or not, and explicit tests were added too for
"showHtml()".

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
This commit is contained in:
Daniel Calviño Sánchez 2019-07-26 18:00:07 +02:00
parent 14006b548e
commit 03f2e8a10e
8 changed files with 33 additions and 5 deletions

BIN
core/js/dist/login.js vendored

Binary file not shown.

Binary file not shown.

BIN
core/js/dist/main.js vendored

Binary file not shown.

Binary file not shown.

Binary file not shown.

Binary file not shown.

View file

@ -897,7 +897,6 @@ describe('Core base tests', function() {
describe('Notifications', function() { describe('Notifications', function() {
var showSpy; var showSpy;
var showHtmlSpy; var showHtmlSpy;
var hideSpy;
var clock; var clock;
/** /**
@ -914,13 +913,11 @@ describe('Core base tests', function() {
beforeEach(function() { beforeEach(function() {
clock = sinon.useFakeTimers(); clock = sinon.useFakeTimers();
showSpy = sinon.spy(OCP.Toast, 'message'); showSpy = sinon.spy(OCP.Toast, 'message');
hideSpy = sinon.spy(OC.Notification, 'hide');
$('#testArea').append('<div id="content"></div>'); $('#testArea').append('<div id="content"></div>');
}); });
afterEach(function() { afterEach(function() {
showSpy.restore(); showSpy.restore();
hideSpy.restore();
// jump past animations // jump past animations
clock.tick(10000); clock.tick(10000);
clock.restore(); clock.restore();
@ -995,10 +992,39 @@ describe('Core base tests', function() {
it('does not hide itself if no timeout given to show', function() { it('does not hide itself if no timeout given to show', function() {
OC.Notification.show(''); OC.Notification.show('');
var $row = $('#testArea .toastify');
expect($row.length).toEqual(1);
// travel in time +1000 seconds // travel in time +1000 seconds
clock.tick(1000000); clock.tick(1000000);
expect(hideSpy.notCalled).toEqual(true); $row = $('#testArea .toastify');
expect($row.length).toEqual(1);
});
});
describe('showHtml', function() {
it('hides itself after a given time', function() {
OC.Notification.showHtml('<p></p>', {timeout: 10});
var $row = $('#testArea .toastify');
expect($row.length).toEqual(1);
clock.tick(11500);
$row = $('#testArea .toastify');
expect($row.length).toEqual(0);
});
it('does not hide itself if no timeout given to show', function() {
OC.Notification.showHtml('<p></p>');
var $row = $('#testArea .toastify');
expect($row.length).toEqual(1);
// travel in time +1000 seconds
clock.tick(1000000);
$row = $('#testArea .toastify');
expect($row.length).toEqual(1);
}); });
}); });
it('cumulates several notifications', function() { it('cumulates several notifications', function() {

View file

@ -97,7 +97,7 @@ export default {
showHtml: function (html, options) { showHtml: function (html, options) {
options = options || {} options = options || {}
options.isHTML = true options.isHTML = true
options.timeout = (options.timeout === 0) ? -1 : options.timeout options.timeout = (!options.timeout) ? -1 : options.timeout
const toast = window.OCP.Toast.message(html, options) const toast = window.OCP.Toast.message(html, options)
return $(toast.toastElement) return $(toast.toastElement)
}, },
@ -113,6 +113,8 @@ export default {
* @deprecated 17.0.0 use OCP.Toast * @deprecated 17.0.0 use OCP.Toast
*/ */
show: function (text, options) { show: function (text, options) {
options = options || {};
options.timeout = (!options.timeout) ? -1 : options.timeout;
const toast = window.OCP.Toast.message(text, options); const toast = window.OCP.Toast.message(text, options);
return $(toast.toastElement); return $(toast.toastElement);
}, },