From 28a51ef724518c337ecc882275afa6031a86f932 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Thu, 25 Jun 2015 16:20:06 +0200 Subject: [PATCH 01/10] Add loading spinner to download icon * vanishes after 7 seconds --- apps/files/js/fileactions.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/apps/files/js/fileactions.js b/apps/files/js/fileactions.js index 1956fda007..9f6a77a378 100644 --- a/apps/files/js/fileactions.js +++ b/apps/files/js/fileactions.js @@ -478,6 +478,14 @@ }, function (filename, context) { var dir = context.dir || context.fileList.getCurrentDirectory(); var url = context.fileList.getDownloadUrl(filename, dir); + + var icon = $(context.$file).find('.fileactions .action-download img'); + var sourceImage = icon.attr('src'); + icon.attr('src', sourceImage.replace('actions/download.svg', 'loading-small.gif')); + setTimeout(function(){ + icon.attr('src', sourceImage); + }, 7000); + if (url) { OC.redirect(url); } From 0cdb46b5c6c9e83d67f67ccd9a10fbf4577eed13 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Fri, 26 Jun 2015 10:26:01 +0200 Subject: [PATCH 02/10] Fix layout of disabled links in the file list * also disable download button after click --- apps/files/css/files.css | 17 +++++++++++++++-- apps/files/js/fileactions.js | 13 ++++++++++++- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/apps/files/css/files.css b/apps/files/css/files.css index e4bf791761..a5f72199b1 100644 --- a/apps/files/css/files.css +++ b/apps/files/css/files.css @@ -583,8 +583,14 @@ a.action>img { #fileList tr:focus a.action, #fileList a.action.permanent, #fileList tr:hover a.action.no-permission:hover, -#fileList tr:focus a.action.no-permission:focus -/*#fileList .name:focus .action*/ { +#fileList tr:focus a.action.no-permission:focus, +/*#fileList .name:focus .action,*/ +/* also enforce the low opacity for disabled links that are hovered/focused */ +.ie8 #fileList a.action.disabled:hover img, +#fileList tr:hover a.action.disabled:hover, +#fileList tr:focus a.action.disabled:focus, +#fileList .name:focus a.action.disabled:focus, +#fileList a.action.disabled img { -ms-filter: "progid:DXImageTransform.Microsoft.Alpha(Opacity=50)"; filter: alpha(opacity=50); opacity: .5; @@ -599,6 +605,13 @@ a.action>img { opacity: 1; display:inline; } +#fileList tr a.action.disabled { + background: none; +} + +#fileList tr:hover a.action.disabled:hover * { + cursor: default; +} .summary { -ms-filter: "progid:DXImageTransform.Microsoft.Alpha(Opacity=30)"; diff --git a/apps/files/js/fileactions.js b/apps/files/js/fileactions.js index 9f6a77a378..ec47534de2 100644 --- a/apps/files/js/fileactions.js +++ b/apps/files/js/fileactions.js @@ -479,11 +479,22 @@ var dir = context.dir || context.fileList.getCurrentDirectory(); var url = context.fileList.getDownloadUrl(filename, dir); - var icon = $(context.$file).find('.fileactions .action-download img'); + var downloadFileaction = $(context.$file).find('.fileactions .action-download'); + + // don't allow a second click on the download action + if(downloadFileaction.hasClass('disabled')) { + return; + } + + downloadFileaction.addClass('disabled'); + var icon = downloadFileaction.find('img'); var sourceImage = icon.attr('src'); icon.attr('src', sourceImage.replace('actions/download.svg', 'loading-small.gif')); + + // TODO proper detection of "download has started" setTimeout(function(){ icon.attr('src', sourceImage); + downloadFileaction.removeClass('disabled'); }, 7000); if (url) { From 16c29021e6e4c2d879cd9d5d5d9b8f3feec61861 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Mon, 29 Jun 2015 08:52:37 +0200 Subject: [PATCH 03/10] Add loading spinner to multi select download button --- apps/files/js/filelist.js | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index 417c4b9fe9..807d1a1689 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -417,6 +417,26 @@ else { files = _.pluck(this.getSelectedFiles(), 'name'); } + + var downloadFileaction = $('#selectedActionsList').find('.download'); + + // don't allow a second click on the download action + if(downloadFileaction.hasClass('disabled')) { + event.preventDefault(); + return; + } + + downloadFileaction.addClass('disabled'); + var icon = downloadFileaction.find('img'); + var sourceImage = icon.attr('src'); + icon.attr('src', sourceImage.replace('actions/download.svg', 'loading-small.gif')); + + // TODO proper detection of "download has started" + setTimeout(function(){ + icon.attr('src', sourceImage); + downloadFileaction.removeClass('disabled'); + }, 7000); + OC.redirect(this.getDownloadUrl(files, dir)); return false; }, From 28a869799594b36f1b12ef67e08fae085f9bc0d7 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Mon, 29 Jun 2015 09:32:11 +0200 Subject: [PATCH 04/10] Reduce timeout for download spinner to 2 seconds --- apps/files/js/fileactions.js | 2 +- apps/files/js/filelist.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/files/js/fileactions.js b/apps/files/js/fileactions.js index ec47534de2..bf529aeb3e 100644 --- a/apps/files/js/fileactions.js +++ b/apps/files/js/fileactions.js @@ -495,7 +495,7 @@ setTimeout(function(){ icon.attr('src', sourceImage); downloadFileaction.removeClass('disabled'); - }, 7000); + }, 2000); if (url) { OC.redirect(url); diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index 807d1a1689..60fcd86588 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -435,7 +435,7 @@ setTimeout(function(){ icon.attr('src', sourceImage); downloadFileaction.removeClass('disabled'); - }, 7000); + }, 2000); OC.redirect(this.getDownloadUrl(files, dir)); return false; From 5a528214b8c6326590c8ff6784187c4191d64f56 Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Mon, 29 Jun 2015 11:24:18 +0200 Subject: [PATCH 05/10] Make download status fully visible --- apps/files/css/files.css | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/apps/files/css/files.css b/apps/files/css/files.css index a5f72199b1..f2f2c5ac3b 100644 --- a/apps/files/css/files.css +++ b/apps/files/css/files.css @@ -597,6 +597,9 @@ a.action>img { display:inline; } .ie8 #fileList a.action:hover img, +#fileList tr a.action.disabled.action-download, +#fileList tr:hover a.action.disabled.action-download:hover, +#fileList tr:focus a.action.disabled.action-download:focus, #fileList tr:hover a.action:hover, #fileList tr:focus a.action:focus, #fileList .name:focus a.action:focus { @@ -609,6 +612,11 @@ a.action>img { background: none; } +#selectedActionsList a.download.disabled, +#fileList tr a.action.action-download.disabled { + color: #000000; +} + #fileList tr:hover a.action.disabled:hover * { cursor: default; } From e557fe0aabe9e1e97249aa5446eef30c804da79f Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Mon, 29 Jun 2015 15:20:47 +0200 Subject: [PATCH 06/10] Add proper download started feedback * this code adds a cookie when a special get parameter is set * the content of this get parameter is used as value for the cookie * the cookie expires after 20 seconds * the JS code checks every 500 milliseconds for the cookie -> if the cookie is set the request returned and the download is started --- apps/files/ajax/download.php | 11 +++++++++++ apps/files/js/fileactions.js | 29 +++++++++++++++++++++++------ apps/files/js/filelist.js | 31 +++++++++++++++++++++++++------ 3 files changed, 59 insertions(+), 12 deletions(-) diff --git a/apps/files/ajax/download.php b/apps/files/ajax/download.php index e67635ab85..26bab8837b 100644 --- a/apps/files/ajax/download.php +++ b/apps/files/ajax/download.php @@ -39,4 +39,15 @@ if (!is_array($files_list)) { $files_list = array($files); } +/** + * this sets a cookie to be able to recognize the start of the download + * the content must not be longer than 32 characters and must only contain + * alphanumeric characters + */ +if(isset($_GET['downloadStartSecret']) + && !isset($_GET['downloadStartSecret'][32]) + && preg_match('!^[a-zA-Z0-9]+$!', $_GET['downloadStartSecret']) === 1) { + setcookie('ocDownloadStarted', $_GET['downloadStartSecret'], time() + 20, '/'); +} + OC_Files::get($dir, $files_list, $_SERVER['REQUEST_METHOD'] == 'HEAD'); diff --git a/apps/files/js/fileactions.js b/apps/files/js/fileactions.js index bf529aeb3e..6678c2d5b3 100644 --- a/apps/files/js/fileactions.js +++ b/apps/files/js/fileactions.js @@ -491,14 +491,31 @@ var sourceImage = icon.attr('src'); icon.attr('src', sourceImage.replace('actions/download.svg', 'loading-small.gif')); - // TODO proper detection of "download has started" - setTimeout(function(){ - icon.attr('src', sourceImage); - downloadFileaction.removeClass('disabled'); - }, 2000); + var randomString = Math.random().toString(36).substring(2); + + var isCookieSet = function(name, value) { + var cookies = document.cookie.split(';'); + for (var i=0; i < cookies.length; i++) { + var cookie = cookies[i].split('='); + if (cookie[0].trim() === name && cookie[1].trim() === value) { + return true; + } + } + return false; + }; + + var checkForDownloadCookie = function() { + if (!isCookieSet('ocDownloadStarted', randomString)){ + setTimeout(checkForDownloadCookie, 500); + } else { + icon.attr('src', sourceImage); + downloadFileaction.removeClass('disabled'); + } + }; if (url) { - OC.redirect(url); + OC.redirect(url + '&downloadStartSecret=' + randomString); + checkForDownloadCookie(); } }, t('files', 'Download')); } diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index 60fcd86588..86c6e22511 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -431,13 +431,32 @@ var sourceImage = icon.attr('src'); icon.attr('src', sourceImage.replace('actions/download.svg', 'loading-small.gif')); - // TODO proper detection of "download has started" - setTimeout(function(){ - icon.attr('src', sourceImage); - downloadFileaction.removeClass('disabled'); - }, 2000); + var randomString = Math.random().toString(36).substring(2); - OC.redirect(this.getDownloadUrl(files, dir)); + var isCookieSet = function(name, value) { + var cookies = document.cookie.split(';'); + for (var i=0; i < cookies.length; i++) { + var cookie = cookies[i].split('='); + if (cookie[0].trim() === name && cookie[1].trim() === value) { + return true; + } + } + return false; + }; + + var checkForDownloadCookie = function() { + console.log('check'); + if (!isCookieSet('ocDownloadStarted', randomString)){ + setTimeout(checkForDownloadCookie, 500); + } else { + console.log('boom'); + icon.attr('src', sourceImage); + downloadFileaction.removeClass('disabled'); + } + }; + + OC.redirect(this.getDownloadUrl(files, dir) + '&downloadStartSecret=' + randomString); + checkForDownloadCookie(); return false; }, From 3d8297c25473c5496fa7e27f600f1e23053df2fc Mon Sep 17 00:00:00 2001 From: Morris Jobke Date: Mon, 29 Jun 2015 15:55:46 +0200 Subject: [PATCH 07/10] Fix unit tests and introduce better mocks of the HTML --- apps/files/tests/js/fileactionsSpec.js | 4 ++-- apps/files/tests/js/filelistSpec.js | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/apps/files/tests/js/fileactionsSpec.js b/apps/files/tests/js/fileactionsSpec.js index 53fa870767..e420ab828a 100644 --- a/apps/files/tests/js/fileactionsSpec.js +++ b/apps/files/tests/js/fileactionsSpec.js @@ -105,7 +105,7 @@ describe('OCA.Files.FileActions tests', function() { $tr.find('.action-download').click(); expect(redirectStub.calledOnce).toEqual(true); - expect(redirectStub.getCall(0).args[0]).toEqual( + expect(redirectStub.getCall(0).args[0]).toContain( OC.webroot + '/index.php/apps/files/ajax/download.php' + '?dir=%2Fsubdir&files=testName.txt'); @@ -129,7 +129,7 @@ describe('OCA.Files.FileActions tests', function() { $tr.find('.action-download').click(); expect(redirectStub.calledOnce).toEqual(true); - expect(redirectStub.getCall(0).args[0]).toEqual( + expect(redirectStub.getCall(0).args[0]).toContain( OC.webroot + '/index.php/apps/files/ajax/download.php' + '?dir=%2Fanotherpath%2Fthere&files=testName.txt' ); diff --git a/apps/files/tests/js/filelistSpec.js b/apps/files/tests/js/filelistSpec.js index b12ac2f251..dbd70682af 100644 --- a/apps/files/tests/js/filelistSpec.js +++ b/apps/files/tests/js/filelistSpec.js @@ -77,8 +77,8 @@ describe('OCA.Files.FileList tests', function() { '' + '' + 'Name' + - '