Fix drag and drop #1094
Fix drag and drop #1094
Conversation
| .resource-upload-dropzone { | ||
| border: 4px dashed #bbb; | ||
| border: 4px dashed @gray-lte; |
There was a problem hiding this comment.
@noirbizarre I can see you hooked on the variables thingy 🙂
There was a problem hiding this comment.
Yes, I always had, but there was no proper @import ~ support in vue-loader initially so it was not possible to properly use them.
When I encounter a case were I can use them, I try to do/fix it
| @@ -22,7 +28,7 @@ | |||
| </span> | |||
| <div class="info-box-content"> | |||
| <span class="info-box-text">{{file.name}}</span> | |||
| <span class="info-box-number">{{file.filesize | filesize}}</span> | |||
| <span class="info-box-number">{{file.size | filesize}}</span> | |||
There was a problem hiding this comment.
I am pretty sure a linter would have caught this issue — @noirbizarre @vinyll do you know of anything to enforce data models for Vue.js?
There was a problem hiding this comment.
Not sure that a linter can find that a variable name is removed from the spec on a non-typed variable. File(s) in this component (is)are provided by fineuploader. The fact it's a File object is not indicated anywhere else than the fineuploader documentation :/
| } | ||
| endpoint += this.is_community ? 'community_' : 'dataset_'; |
There was a problem hiding this comment.
Seems very error prone. Could we rather build a string like this?
const route_new = this.resource.id ? '' : 'new_';
const route_namespace = this.is_community ? 'community_' : 'dataset_';
const endpoint = `upload_${route_new}${route_namespace}resource`;Would make stuff more explicit, and probably simplify the above logic.
There was a problem hiding this comment.
Yes we can (and I will), but that's only working for the string building part, not the parameters.
| @@ -155,13 +161,17 @@ export default { | |||
| }, | |||
| events: { | |||
| 'uploader:progress': function(id, uploaded, total) { | |||
| const el = this.$el.getElementById(`progress-${id}`); | |||
| const el = document.getElementById(`progress-${id}`); | |||
There was a problem hiding this comment.
Aren't we breaking the rules of the Vue component? Right now it seems we are looking for something outside of the component itself.
There was a problem hiding this comment.
No, we are looking for a component inside (defined in the associated template). Sadly, getElementById is not defined on Element this is why I had to use document (which is weird because I though I was able to do it before)
In Vue.js 2.0, referencing raw Elements for direct access in a loop is supported (using the ref attribute on components or elements and accessing them using this.$refs. Sadly, it's only supported on component in Vue.js 1.x :/
| const file = this.$uploader.getFile(id); | ||
| this.files.$remove(this.files.indexOf(file)); | ||
| 'uploader:complete': function(id, response, file) { | ||
| this.files.splice(this.files.indexOf(file), 1); |
There was a problem hiding this comment.
What is the intent of this line? Loos like we are modifying the files list in place but not sure why.
The previously used $remove invocation did not work for any specific reason?
There was a problem hiding this comment.
Yes, we remove the file from the array, and for Vue.js to be able to detect the change, we need to use splice() (See https://v1.vuejs.org/guide/list.html#Array-Change-Detection)
The previous $remove() was removed (:wink:) from Vue.js 1.0.
There was a problem hiding this comment.
My bad, the syntax is just not the same (and there is not a dedicated section about it): https://v1.vuejs.org/guide/list.html#Caveats
I wil change to this.files.$remove(file) and see if it's working 👍
This PR fixes a lot of issue on file upload:
File.fileSizetoBlob.size)