From 3671f09b1d168a6e7c1a4d72428cd9d5db4b7f8c Mon Sep 17 00:00:00 2001 From: Richard Date: Thu, 28 Nov 2013 12:26:30 +1100 Subject: [PATCH 1/3] checking file upload response error and giving an error alert if there is one. --- .../app/views/publisher/uploader_view.js | 26 ++++++++++++++----- config/locales/javascript/javascript.en.yml | 1 + 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/app/views/publisher/uploader_view.js b/app/assets/javascripts/app/views/publisher/uploader_view.js index b09d8dd21..f58807d2e 100644 --- a/app/assets/javascripts/app/views/publisher/uploader_view.js +++ b/app/assets/javascripts/app/views/publisher/uploader_view.js @@ -60,13 +60,19 @@ app.views.PublisherUploader = Backbone.View.extend({ }, uploadCompleteHandler: function(id, fileName, response) { - this.el_info.text(Diaspora.I18n.t('photo_uploader.completed', {file: fileName})).fadeTo(2000, 0); + if (response.success){ + this.el_info.text(Diaspora.I18n.t('photo_uploader.completed', {file: fileName})).fadeTo(2000, 0); - var id = response.data.photo.id, - url = response.data.photo.unprocessed_image.url; + var id = response.data.photo.id, + url = response.data.photo.unprocessed_image.url; - this._addFinishedPhoto(id, url); - this.trigger('change'); + this._addFinishedPhoto(id, url); + this.trigger('change'); + } else { + this._cancelPhotoUpload(); + this.trigger('change'); + alert(Diaspora.I18n.t('photo_uploader.error', {file: fileName})); + } }, // replace the first photo placeholder with the finished uploaded image and @@ -79,7 +85,7 @@ app.views.PublisherUploader = Backbone.View.extend({ '' ); - // replace placeholder + // // replace placeholder var placeholder = publisher.el_photozone.find('li.loading').first(); placeholder .removeClass('loading') @@ -96,6 +102,14 @@ app.views.PublisherUploader = Backbone.View.extend({ } }, + _cancelPhotoUpload: function() { + var publisher = this.options.publisher; + var placeholder = publisher.el_photozone.find('li.loading').first(); + placeholder + .removeClass('loading') + .find('img').remove(); + }, + // remove an already uploaded photo _removePhoto: function(evt) { var self = this; diff --git a/config/locales/javascript/javascript.en.yml b/config/locales/javascript/javascript.en.yml index c52b13398..b24371dfd 100644 --- a/config/locales/javascript/javascript.en.yml +++ b/config/locales/javascript/javascript.en.yml @@ -81,6 +81,7 @@ en: photo_uploader: looking_good: "OMG, you look awesome!" completed: "<%= file %> completed" + error: "A problem occurred while uploading file <%= file %>" invalid_ext: "{file} has invalid extension. Only {extensions} are allowed." size_error: "{file} is too large, maximum file size is {sizeLimit}." empty: "{file} is empty, please select files again without it." From ff07bddd1b61f26947e24c312cbbf3a1f7f266d2 Mon Sep 17 00:00:00 2001 From: Richard Date: Thu, 28 Nov 2013 16:08:03 +1100 Subject: [PATCH 2/3] using status messages instead of popup and updated specs for publisher view. --- .../app/views/publisher/uploader_view.js | 2 +- .../app/views/publisher_view_spec.js | 31 ++++++++++++++++--- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/app/views/publisher/uploader_view.js b/app/assets/javascripts/app/views/publisher/uploader_view.js index f58807d2e..7f5830443 100644 --- a/app/assets/javascripts/app/views/publisher/uploader_view.js +++ b/app/assets/javascripts/app/views/publisher/uploader_view.js @@ -71,7 +71,7 @@ app.views.PublisherUploader = Backbone.View.extend({ } else { this._cancelPhotoUpload(); this.trigger('change'); - alert(Diaspora.I18n.t('photo_uploader.error', {file: fileName})); + this.el_info.text(Diaspora.I18n.t('photo_uploader.error', {file: fileName})); } }, diff --git a/spec/javascripts/app/views/publisher_view_spec.js b/spec/javascripts/app/views/publisher_view_spec.js index a6a7135b3..708f63180 100644 --- a/spec/javascripts/app/views/publisher_view_spec.js +++ b/spec/javascripts/app/views/publisher_view_spec.js @@ -382,15 +382,17 @@ describe("app.views.Publisher", function() { }); }); - context('completion', function() { + context('successful completion', function() { beforeEach(function() { Diaspora.I18n.loadLocale({ photo_uploader: { completed: '<%= file %> completed' }}); $('#photodropzone').html('
    • '); - this.uploader.onComplete(null, 'test.jpg', { data: { photo: { - id: '987', - unprocessed_image: { url: 'test.jpg' } - }}}); + this.uploader.onComplete(null, 'test.jpg', { + data: { photo: { + id: '987', + unprocessed_image: { url: 'test.jpg' } + }}, + success: true }); }); it('shows it in text form', function() { @@ -417,6 +419,25 @@ describe("app.views.Publisher", function() { expect(this.view.el_preview.prop('disabled')).toBeFalsy(); }); }); + + context('unsuccessful completion', function() { + beforeEach(function() { + Diaspora.I18n.loadLocale({ photo_uploader: { completed: '<%= file %> completed' }}); + $('#photodropzone').html('
    • '); + + this.uploader.onComplete(null, 'test.jpg', { + data: { photo: { + id: '987', + unprocessed_image: { url: 'test.jpg' } + }}, + success: false }); + }); + + it('shows error message', function() { + var info = this.view.view_uploader.el_info; + expect(info.text()).toBe(Diaspora.I18n.t('photo_uploader.error', {file: 'test.jpg'})) + }); + }); }); context('photo removal', function() { From dbbead158769c0cd66492dde9ef8703a76ef710b Mon Sep 17 00:00:00 2001 From: Richard Date: Thu, 28 Nov 2013 21:48:41 +1100 Subject: [PATCH 3/3] removed double comment --- app/assets/javascripts/app/views/publisher/uploader_view.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/app/views/publisher/uploader_view.js b/app/assets/javascripts/app/views/publisher/uploader_view.js index 7f5830443..730850ebe 100644 --- a/app/assets/javascripts/app/views/publisher/uploader_view.js +++ b/app/assets/javascripts/app/views/publisher/uploader_view.js @@ -85,7 +85,7 @@ app.views.PublisherUploader = Backbone.View.extend({ '' ); - // // replace placeholder + // replace placeholder var placeholder = publisher.el_photozone.find('li.loading').first(); placeholder .removeClass('loading')