From 82bd92dd4686e3620db80d60204920c681f10e51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Fri, 5 Apr 2013 01:06:52 +0200 Subject: [PATCH] consistent behavior regarding file uploads --- .../discourse/components/utilities.js | 54 +++++++++++++++---- .../discourse/views/composer_view.js | 45 ++++------------ ...age_selector.js => image_selector_view.js} | 19 +++---- spec/javascripts/components/utilities_spec.js | 48 ++++++++++++++++- spec/javascripts/spec.js | 1 + 5 files changed, 109 insertions(+), 58 deletions(-) rename app/assets/javascripts/discourse/views/modal/{image_selector.js => image_selector_view.js} (53%) diff --git a/app/assets/javascripts/discourse/components/utilities.js b/app/assets/javascripts/discourse/components/utilities.js index b0f2932a4..673ad4747 100644 --- a/app/assets/javascripts/discourse/components/utilities.js +++ b/app/assets/javascripts/discourse/components/utilities.js @@ -9,17 +9,10 @@ Discourse.Utilities = { translateSize: function(size) { switch (size) { - case 'tiny': - size = 20; - break; - case 'small': - size = 25; - break; - case 'medium': - size = 32; - break; - case 'large': - size = 45; + case 'tiny': return 20; + case 'small': return 25; + case 'medium': return 32; + case 'large': return 45; } return size; }, @@ -163,6 +156,45 @@ Discourse.Utilities = { range.moveStart('character', pos); return range.select(); } + }, + + /** + validateFilesForUpload + + **/ + /** + Validate a list of files to be uploaded + + @method validateFilesForUpload + @param {Array} files The list of files we want to upload + **/ + validateFilesForUpload: function(files) { + if (files) { + // can only upload one file at a time + if (files.length > 1) { + bootbox.alert(Em.String.i18n('post.errors.upload_too_many_images')); + return false; + } else if (files.length > 0) { + // check that the uploaded file is an image + // TODO: we should provide support for other types of file + if (files[0].type && files[0].type.indexOf('image/') !== 0) { + bootbox.alert(Em.String.i18n('post.errors.only_images_are_supported')); + return false; + } + // check file size + if (files[0].size && files[0].size > 0) { + var fileSizeInKB = files[0].size / 1024; + if (fileSizeInKB > Discourse.SiteSettings.max_upload_size_kb) { + bootbox.alert(Em.String.i18n('post.errors.upload_too_large', { max_size_kb: Discourse.SiteSettings.max_upload_size_kb })); + return false; + } + // everything is fine + return true; + } + } + } + // there has been an error + return false; } }; diff --git a/app/assets/javascripts/discourse/views/composer_view.js b/app/assets/javascripts/discourse/views/composer_view.js index 0a3d9a060..91264658f 100644 --- a/app/assets/javascripts/discourse/views/composer_view.js +++ b/app/assets/javascripts/discourse/views/composer_view.js @@ -276,43 +276,18 @@ Discourse.ComposerView = Discourse.View.extend({ formData: { topic_id: 1234 } }); - var addFiles = function (e, data) { - // can only upload one file at a time - if (data.files.length > 1) { - bootbox.alert(Em.String.i18n('post.errors.upload_too_many_images')); - return false; - } else if (data.files.length > 0) { - // check file size - var fileSizeInKB = data.files[0].size / 1024; - if (fileSizeInKB > Discourse.SiteSettings.max_upload_size_kb) { - bootbox.alert(Em.String.i18n('post.errors.upload_too_large', { max_size_kb: Discourse.SiteSettings.max_upload_size_kb })); - return false; - } - // check that the uploaded file is an image - // TODO: we should provide support for other types of file - if (data.files[0].type.indexOf('image/') !== 0) { - bootbox.alert(Em.String.i18n('post.errors.only_images_are_supported')); - return false; - } - // everything is fine, reset upload status - _this.setProperties({ - uploadProgress: 0, - loadingImage: true - }); - return true; - } - // we need to return true here, otherwise it prevents the default paste behavior - return true; - }; + // submit - this event is triggered for each upload + $uploadTarget.on('fileuploadsubmit', function (e, data) { + var result = Discourse.Utilities.validateFilesForUpload(data.files); + // reset upload status when everything is ok + if (result) _this.setProperties({ uploadProgress: 0, loadingImage: true }); + return result; + }); - // paste - $uploadTarget.on('fileuploadpaste', addFiles); - - // drop - $uploadTarget.on('fileuploaddrop', addFiles); - - // send + // send - this event is triggered when the upload request is about to start $uploadTarget.on('fileuploadsend', function (e, data) { + // hide the "image selector" modal + $('#discourse-modal').modal('hide'); // cf. https://github.com/blueimp/jQuery-File-Upload/wiki/API#how-to-cancel-an-upload var jqXHR = data.xhr(); // need to wait for the link to show up in the DOM diff --git a/app/assets/javascripts/discourse/views/modal/image_selector.js b/app/assets/javascripts/discourse/views/modal/image_selector_view.js similarity index 53% rename from app/assets/javascripts/discourse/views/modal/image_selector.js rename to app/assets/javascripts/discourse/views/modal/image_selector_view.js index 2b52cca87..897bb5301 100644 --- a/app/assets/javascripts/discourse/views/modal/image_selector.js +++ b/app/assets/javascripts/discourse/views/modal/image_selector_view.js @@ -13,30 +13,27 @@ Discourse.ImageSelectorView = Discourse.View.extend({ init: function() { this._super(); - return this.set('localSelected', true); + this.set('localSelected', true); }, selectLocal: function() { - return this.set('localSelected', true); + this.set('localSelected', true); }, selectRemote: function() { - return this.set('localSelected', false); + this.set('localSelected', false); }, - remoteSelected: (function() { + remoteSelected: function() { return !this.get('localSelected'); - }).property('localSelected'), + }.property('localSelected'), upload: function() { - this.get('uploadTarget').fileupload('send', { fileInput: $('#filename-input') }); - return $('#discourse-modal').modal('hide'); + this.get('uploadTarget').fileupload('add', { fileInput: $('#filename-input') }); }, add: function() { - this.get('composer').addMarkdown("![image](" + ($('#fileurl-input').val()) + ")"); - return $('#discourse-modal').modal('hide'); + this.get('composer').addMarkdown("![image](" + $('#fileurl-input').val() + ")"); + $('#discourse-modal').modal('hide'); } }); - - diff --git a/spec/javascripts/components/utilities_spec.js b/spec/javascripts/components/utilities_spec.js index a89a39d83..9f4685a8a 100644 --- a/spec/javascripts/components/utilities_spec.js +++ b/spec/javascripts/components/utilities_spec.js @@ -1,4 +1,4 @@ -/*global waitsFor:true expect:true describe:true beforeEach:true it:true */ +/*global waitsFor:true expect:true describe:true beforeEach:true it:true spyOn:true */ describe("Discourse.Utilities", function() { @@ -30,4 +30,50 @@ describe("Discourse.Utilities", function() { }); + describe("validateFilesForUpload", function() { + + it("returns false when file is undefined", function() { + expect(Discourse.Utilities.validateFilesForUpload(null)).toBe(false); + expect(Discourse.Utilities.validateFilesForUpload(undefined)).toBe(false); + }); + + it("returns false when file there is no file", function() { + expect(Discourse.Utilities.validateFilesForUpload([])).toBe(false); + }); + + it("supports only one file", function() { + spyOn(bootbox, 'alert'); + spyOn(Em.String, 'i18n'); + expect(Discourse.Utilities.validateFilesForUpload([1, 2])).toBe(false); + expect(bootbox.alert).toHaveBeenCalled(); + expect(Em.String.i18n).toHaveBeenCalledWith('post.errors.upload_too_many_images'); + }); + + it("supports only an image", function() { + var html = { type: "text/html" }; + spyOn(bootbox, 'alert'); + spyOn(Em.String, 'i18n'); + expect(Discourse.Utilities.validateFilesForUpload([html])).toBe(false); + expect(bootbox.alert).toHaveBeenCalled(); + expect(Em.String.i18n).toHaveBeenCalledWith('post.errors.only_images_are_supported'); + }); + + it("prevents the upload of a too large image", function() { + var image = { type: "image/png", size: 10 * 1024 }; + Discourse.SiteSettings.max_upload_size_kb = 5; + spyOn(bootbox, 'alert'); + spyOn(Em.String, 'i18n'); + expect(Discourse.Utilities.validateFilesForUpload([image])).toBe(false); + expect(bootbox.alert).toHaveBeenCalled(); + expect(Em.String.i18n).toHaveBeenCalledWith('post.errors.upload_too_large', { max_size_kb: 5 }); + }); + + it("works", function() { + var image = { type: "image/png", size: 10 * 1024 }; + Discourse.SiteSettings.max_upload_size_kb = 15; + expect(Discourse.Utilities.validateFilesForUpload([image])).toBe(true); + }); + + }); + }); diff --git a/spec/javascripts/spec.js b/spec/javascripts/spec.js index 7a2a282b1..85dd2f51b 100644 --- a/spec/javascripts/spec.js +++ b/spec/javascripts/spec.js @@ -18,6 +18,7 @@ // The rest of the externals //= require_tree ../../app/assets/javascripts/external +//= require ../../app/assets/javascripts/locales/i18n //= require ../../app/assets/javascripts/discourse/helpers/i18n_helpers //= require ../../app/assets/javascripts/discourse