Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix drag and drop #1094

Merged
merged 9 commits into from
Aug 25, 2017
Merged

Fix drag and drop #1094

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2d7f3e9
Fix resources drag & drop endpoint detection and user better endpoint…
noirbizarre Aug 22, 2017
ca08776
Restore missing dropzone highlight in resource form
noirbizarre Aug 22, 2017
7a95a3a
Fix file size handling
noirbizarre Aug 22, 2017
21fe774
Update fineuploader to 5.15.0 to fix early submission
noirbizarre Aug 22, 2017
246c2c9
Fix inline upload layout and style
noirbizarre Aug 22, 2017
cc03f48
Properly remove uploaded files from resource list
noirbizarre Aug 22, 2017
243cb3f
Remove files in error from resource list
noirbizarre Aug 22, 2017
3c3d308
Added changelog
noirbizarre Aug 22, 2017
db9edfc
Post review fixes
noirbizarre Aug 25, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
[#1078](https://github.com/opendatateam/udata/pull/1078)
- Improve elasticsearch configurability
[#1096](https://github.com/opendatateam/udata/pull/1096)
- Lots of fixes admin files upload
[1094](https://github.com/opendatateam/udata/pull/1094)

## 1.1.1 (2017-07-31)

Expand Down
29 changes: 16 additions & 13 deletions js/components/dataset/resource/file-form.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
<style lang="less">
@import '~less/admin/variables';

.resource-upload-dropzone {
border: 4px dashed #bbb;
border: 4px dashed @gray-lte;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@noirbizarre I can see you hooked on the variables thingy 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

min-height: 150px;
padding-top: 10px;
padding-bottom: 10px;

.lead {
margin-bottom: 0;
}

.drop-active > & {
border: 4px dashed @green;
}
}
</style>

Expand All @@ -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>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :/

<div class="progress">
<div class="progress-bar" :style="{width: progress+'%'}"></div>
</div>
Expand Down Expand Up @@ -124,22 +130,19 @@ export default {
upload_endpoint() {
const operations = API.datasets.operations;
let params = {};
let endpoint = 'upload_';
if (typeof this.dataset !== 'undefined') {
params = {dataset: this.dataset.id};
}
if (this.is_community) {
endpoint += 'community_';
params.community = this.resource.id;
} else {
endpoint += 'dataset_';
}
if (this.resource.id) {
endpoint += 'resource';
params.rid = this.resource.id;
} else {
endpoint += 'resources';
if (this.is_community) {
params.community = this.resource.id;
} else {
params.rid = this.resource.id;
}
}
const route_new = this.resource.id ? '' : 'new_';
const route_namespace = this.is_community ? 'community_' : 'dataset_';
const endpoint = `upload_${route_new}${route_namespace}resource`;
return operations[endpoint].urlify(params);
}
},
Expand Down
28 changes: 19 additions & 9 deletions js/components/dataset/resource/list.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
text-overflow: ellipsis;
white-space: nowrap;
}

td .progress {
margin-top: 5px;
}
}

.drop-active > & {
Expand Down Expand Up @@ -53,13 +57,15 @@
<td v-if="reordering"></td>
<td>
<div class="ellipsis">{{ file.name }}</div>
</td>
<td>{{ file.name | fileext }}</td>
<td>{{ file.size | filesize }}</td>
<td colspan="2">
<div class="progress progress-xs progress-striped active">
<div class="progress-bar progress-bar-primary"
id="progress-{{file.id}}" style="width: 0%"></div>
<div class="progress-bar progress-bar-primary"
id="progress-{{file.id}}" style="width: 0%"></div>
</div>
</td>
<td>{{ file.type }}</td>
<td>{{ file.size | filesize }}</td>
</tr>
<tr v-for="resource in dataset.resources" @click="display(resource)"
:class="{ 'pointer': !reodering, 'move': reordering }"
Expand Down Expand Up @@ -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}`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't we breaking the rules of the Vue component? Right now it seems we are looking for something outside of the component itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :/

el.style.width = `${Math.round(uploaded * 100 / total)}%`;
},
'uploader:complete': function(id, response) {
const file = this.$uploader.getFile(id);
this.files.$remove(this.files.indexOf(file));
'uploader:complete': function(id, response, file) {
this.files.$remove(file);
this.dataset.resources.unshift(response);
},
'uploader:error': function(id) {
// Remove the progressing file (an error is already displayed globally)
const file = this.$uploader.getFile(id);
this.files.splice(this.files.indexOf(file), 1);
}
},
ready() {
Expand Down Expand Up @@ -210,7 +220,7 @@ export default {
watch: {
'dataset.id': function(id) {
if (id) {
this.upload_endpoint = API.datasets.operations.upload_dataset_resource.urlify({dataset: id});
this.upload_endpoint = API.datasets.operations.upload_new_dataset_resource.urlify({dataset: id});
}
}
}
Expand Down
7 changes: 5 additions & 2 deletions js/mixins/uploader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,11 @@ export default {
},

filters: {
fileext(filename) {
return filename.split('.').pop();
},
filesize(size) {
if (size <= 0) {
if (!size || size <= 0) {
return '-';
}
if (size > _1GO) {
Expand Down Expand Up @@ -161,7 +164,7 @@ export default {
on_complete(id, name, response) {
if (!response.success) return;
const file = this.$uploader.getFile(id);
this.files.$remove(this.files.indexOf(file));
this.files.$remove(file);
this.$emit('uploader:complete', id, response, file);
},

Expand Down
2 changes: 1 addition & 1 deletion less/admin/vendor.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
// @import '~bootstrap/less/jumbotron';
@import '~bootstrap/less/thumbnails';
@import '~bootstrap/less/alerts';
// @import '~bootstrap/less/progress-bars';
@import '~bootstrap/less/progress-bars';
@import '~bootstrap/less/media';
// @import '~bootstrap/less/list-group';
// @import '~bootstrap/less/panels';
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@
"bootstrap-markdown": "^2.10.0",
"chart.js": "^1.1.1",
"diff": "^2.2.3",
"fine-uploader": "~5.11.10",
"fine-uploader": "~5.15.0",
"font-awesome": "^4.7.0",
"highlight.js": "^9.9.0",
"i18next": "3.3.1",
Expand Down
12 changes: 6 additions & 6 deletions udata/core/dataset/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,11 +259,11 @@ def extract_infos_from_args(self, args, dataset):
}


@ns.route('/<dataset:dataset>/upload/', endpoint='upload_dataset_resources')
@ns.route('/<dataset:dataset>/upload/', endpoint='upload_new_dataset_resource')
@api.doc(parser=upload_parser, **common_doc)
class UploadDatasetResources(UploadMixin, API):
class UploadNewDatasetResource(UploadMixin, API):
@api.secure
@api.doc('upload_dataset_resources')
@api.doc('upload_new_dataset_resource')
@api.marshal_with(upload_fields)
def post(self, dataset):
'''Upload a new dataset resource'''
Expand All @@ -278,11 +278,11 @@ def post(self, dataset):


@ns.route('/<dataset:dataset>/upload/community/',
endpoint='upload_community_resources')
endpoint='upload_new_community_resource')
@api.doc(parser=upload_parser, **common_doc)
class UploadCommunityResources(UploadMixin, API):
class UploadNewCommunityResources(UploadMixin, API):
@api.secure
@api.doc('upload_community_resources')
@api.doc('upload_new_community_resource')
@api.marshal_with(upload_fields)
def post(self, dataset):
'''Upload a new community resource'''
Expand Down
6 changes: 3 additions & 3 deletions udata/tests/api/test_datasets_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,7 @@ def test_create_with_file(self):
])
dataset = DatasetFactory(organization=org)
response = self.post(
url_for('api.upload_dataset_resources', dataset=dataset),
url_for('api.upload_new_dataset_resource', dataset=dataset),
{'file': (StringIO(b'aaa'), 'test.txt')}, json=False)
self.assert201(response)
data = json.loads(response.data)
Expand Down Expand Up @@ -926,7 +926,7 @@ def test_community_resource_api_create(self):
dataset = VisibleDatasetFactory()
self.login()
response = self.post(
url_for('api.upload_community_resources', dataset=dataset),
url_for('api.upload_new_community_resource', dataset=dataset),
{'file': (StringIO(b'aaa'), 'test.txt')}, json=False)
self.assert201(response)
data = json.loads(response.data)
Expand All @@ -950,7 +950,7 @@ def test_community_resource_api_create_as_org(self):
Member(user=user, role='admin')
])
response = self.post(
url_for('api.upload_community_resources', dataset=dataset),
url_for('api.upload_new_community_resource', dataset=dataset),
{'file': (StringIO(b'aaa'), 'test.txt')}, json=False)
self.assert201(response)
data = json.loads(response.data)
Expand Down