Tryton Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(93)

Issue 33671002: sao: Use filename when file download (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 months ago by xcodinas
Modified:
7 months ago
Reviewers:
pokoli, reviewbot, rietveld-bot, ced, perilla
Visibility:
Public.

Description

Patch Set 1 #

Patch Set 2 : Add a function in common #

Total comments: 12

Patch Set 3 : Fix comments #

Total comments: 4

Patch Set 4 : Improve download_file function #

Patch Set 5 : Reduce arguments to 2 and fix element remove #

Total comments: 9

Patch Set 6 : Fix comments #

Patch Set 7 : Always create a popup and delete it when click #

Total comments: 6

Patch Set 8 : Make popup visible, remove span #

Total comments: 14

Patch Set 9 : Add download dialog #

Total comments: 7

Patch Set 10 : Fix comments #

Total comments: 12

Patch Set 11 : Fix comments #

Total comments: 12

Patch Set 12 : Remove unnecessary code, add close as callback #

Total comments: 2

Patch Set 13 : Fix style, remove submit as it's not called #

Total comments: 1

Patch Set 14 : Avoid calling filename field twice #

Unified diffs Side-by-side diffs Delta from patch set Stats (+44 lines, -14 lines) Patch
M src/action.js View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -8 lines 0 comments Download
M src/common.js View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +37 lines, -0 lines 0 comments Download
M src/view/form.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +5 lines, -6 lines 0 comments Download

Messages

Total messages: 56
xcodinas
9 months ago (2016-12-23 08:17:14 UTC) #1
reviewbot
flake8 OK URL: https://codereview.tryton.org/33671002
9 months ago (2016-12-23 08:31:20 UTC) #2
xcodinas
Add a function in common
9 months ago (2016-12-23 08:47:37 UTC) #3
reviewbot
flake8 OK URL: https://codereview.tryton.org/33671002
9 months ago (2016-12-23 08:53:07 UTC) #4
pokoli
https://tryton-rietveld.appspot.com/33671002/diff/10003/src/common.js File src/common.js (right): https://tryton-rietveld.appspot.com/33671002/diff/10003/src/common.js#newcode3285 src/common.js:3285: var a = document.createElement("a"); a should not be created ...
9 months ago (2016-12-23 09:23:00 UTC) #5
xcodinas
https://tryton-rietveld.appspot.com/33671002/diff/10003/src/common.js File src/common.js (right): https://tryton-rietveld.appspot.com/33671002/diff/10003/src/common.js#newcode3285 src/common.js:3285: var a = document.createElement("a"); On 2016/12/23 09:23:00, pokoli wrote: ...
9 months ago (2016-12-23 09:31:04 UTC) #6
perilla
https://tryton-rietveld.appspot.com/33671002/diff/10003/src/common.js File src/common.js (right): https://tryton-rietveld.appspot.com/33671002/diff/10003/src/common.js#newcode3286 src/common.js:3286: if (typeof(a.download) !== "undefined" && name) { "undefined" -> ...
9 months ago (2016-12-23 10:27:58 UTC) #7
ced
https://tryton-rietveld.appspot.com/33671002/diff/10003/src/action.js File src/action.js (right): https://tryton-rietveld.appspot.com/33671002/diff/10003/src/action.js#newcode206 src/action.js:206: Sao.Action.report_blob_url = blob_url; I do not think it is ...
8 months, 3 weeks ago (2016-12-26 18:43:25 UTC) #8
xcodinas
Fix comments
8 months, 3 weeks ago (2016-12-27 09:18:02 UTC) #9
xcodinas
https://tryton-rietveld.appspot.com/33671002/diff/10003/src/common.js File src/common.js (right): https://tryton-rietveld.appspot.com/33671002/diff/10003/src/common.js#newcode3285 src/common.js:3285: var a = document.createElement("a"); On 2016/12/26 18:43:24, ced wrote: ...
8 months, 3 weeks ago (2016-12-27 09:20:30 UTC) #10
ced
https://tryton-rietveld.appspot.com/33671002/diff/10003/src/common.js File src/common.js (right): https://tryton-rietveld.appspot.com/33671002/diff/10003/src/common.js#newcode3285 src/common.js:3285: var a = document.createElement("a"); On 2016/12/27 09:20:30, xcodinas wrote: ...
8 months, 3 weeks ago (2016-12-27 10:01:58 UTC) #11
ced
https://tryton-rietveld.appspot.com/33671002/diff/30001/src/common.js File src/common.js (right): https://tryton-rietveld.appspot.com/33671002/diff/30001/src/common.js#newcode3284 src/common.js:3284: Sao.common.download_file = function(name, blob_url) { I think the blob_url ...
8 months, 3 weeks ago (2016-12-28 14:59:35 UTC) #12
xcodinas
Improve download_file function
8 months, 3 weeks ago (2016-12-29 09:52:36 UTC) #13
reviewbot
flake8 OK URL: https://codereview.tryton.org/33671002
8 months, 3 weeks ago (2016-12-29 09:58:16 UTC) #14
xcodinas
Reduce arguments to 2 and fix element remove
8 months, 3 weeks ago (2016-12-29 10:08:29 UTC) #15
xcodinas
console.log(data);
8 months, 3 weeks ago (2016-12-29 10:09:42 UTC) #16
reviewbot
flake8 OK URL: https://codereview.tryton.org/33671002
8 months, 3 weeks ago (2016-12-29 10:24:46 UTC) #17
ced
https://tryton-rietveld.appspot.com/33671002/diff/90001/src/action.js File src/action.js (right): https://tryton-rietveld.appspot.com/33671002/diff/90001/src/action.js#newcode202 src/action.js:202: Sao.common.download_file(file_name, [[data], type]); I think the API will be ...
8 months, 3 weeks ago (2016-12-30 21:40:54 UTC) #18
xcodinas
https://tryton-rietveld.appspot.com/33671002/diff/90001/src/common.js File src/common.js (right): https://tryton-rietveld.appspot.com/33671002/diff/90001/src/common.js#newcode3288 src/common.js:3288: window.URL.revokeObjectURL(this.blob_url); On 2016/12/30 21:40:54, ced wrote: > Maybe we ...
8 months, 2 weeks ago (2017-01-02 09:43:43 UTC) #19
ced
https://tryton-rietveld.appspot.com/33671002/diff/90001/src/common.js File src/common.js (right): https://tryton-rietveld.appspot.com/33671002/diff/90001/src/common.js#newcode3288 src/common.js:3288: window.URL.revokeObjectURL(this.blob_url); On 2017/01/02 09:43:43, xcodinas wrote: > On 2016/12/30 ...
8 months, 2 weeks ago (2017-01-02 09:55:30 UTC) #20
xcodinas
Fix comments
8 months, 2 weeks ago (2017-01-03 12:56:14 UTC) #21
reviewbot
flake8 OK URL: https://codereview.tryton.org/33671002
8 months, 2 weeks ago (2017-01-03 13:10:45 UTC) #22
xcodinas
Always create a popup and delete it when click
8 months, 2 weeks ago (2017-01-04 12:14:15 UTC) #23
reviewbot
flake8 OK URL: https://codereview.tryton.org/33671002
8 months, 2 weeks ago (2017-01-04 12:19:16 UTC) #24
ced
https://tryton-rietveld.appspot.com/33671002/diff/130001/src/common.js File src/common.js (right): https://tryton-rietveld.appspot.com/33671002/diff/130001/src/common.js#newcode3285 src/common.js:3285: if (typeof(type) == 'string') { Why testing typeof? https://tryton-rietveld.appspot.com/33671002/diff/130001/src/common.js#newcode3286 ...
8 months, 2 weeks ago (2017-01-05 00:34:03 UTC) #25
xcodinas
https://tryton-rietveld.appspot.com/33671002/diff/130001/src/common.js File src/common.js (right): https://tryton-rietveld.appspot.com/33671002/diff/130001/src/common.js#newcode3285 src/common.js:3285: if (typeof(type) == 'string') { On 2017/01/05 00:34:03, ced ...
8 months, 2 weeks ago (2017-01-05 10:40:25 UTC) #26
ced
https://tryton-rietveld.appspot.com/33671002/diff/130001/src/common.js File src/common.js (right): https://tryton-rietveld.appspot.com/33671002/diff/130001/src/common.js#newcode3285 src/common.js:3285: if (typeof(type) == 'string') { On 2017/01/05 10:40:25, xcodinas ...
8 months, 2 weeks ago (2017-01-05 11:05:09 UTC) #27
xcodinas
Make popup visible, remove span
8 months, 2 weeks ago (2017-01-05 11:23:10 UTC) #28
reviewbot
flake8 OK URL: https://codereview.tryton.org/33671002
8 months, 2 weeks ago (2017-01-05 11:42:20 UTC) #29
ced
https://tryton-rietveld.appspot.com/33671002/diff/150001/src/common.js File src/common.js (right): https://tryton-rietveld.appspot.com/33671002/diff/150001/src/common.js#newcode3284 src/common.js:3284: Sao.common.download_file = function(name, data, type) { Why not guess ...
8 months, 2 weeks ago (2017-01-06 12:13:38 UTC) #30
xcodinas
https://tryton-rietveld.appspot.com/33671002/diff/150001/src/common.js File src/common.js (right): https://tryton-rietveld.appspot.com/33671002/diff/150001/src/common.js#newcode3303 src/common.js:3303: if (name && this.compatible) { On 2017/01/06 12:13:38, ced ...
8 months, 1 week ago (2017-01-09 09:16:37 UTC) #31
ced
https://tryton-rietveld.appspot.com/33671002/diff/150001/src/common.js File src/common.js (right): https://tryton-rietveld.appspot.com/33671002/diff/150001/src/common.js#newcode3303 src/common.js:3303: if (name && this.compatible) { On 2017/01/09 09:16:36, xcodinas ...
8 months, 1 week ago (2017-01-09 10:35:44 UTC) #32
xcodinas
On 2017/01/09 10:35:44, ced wrote: > https://tryton-rietveld.appspot.com/33671002/diff/150001/src/common.js > File src/common.js (right): > > https://tryton-rietveld.appspot.com/33671002/diff/150001/src/common.js#newcode3303 > ...
8 months, 1 week ago (2017-01-09 10:48:48 UTC) #33
ced
https://tryton-rietveld.appspot.com/33671002/diff/150001/src/common.js File src/common.js (right): https://tryton-rietveld.appspot.com/33671002/diff/150001/src/common.js#newcode3307 src/common.js:3307: window.setTimeout(function() { On 2017/01/09 09:16:36, xcodinas wrote: > On ...
8 months, 1 week ago (2017-01-09 11:11:34 UTC) #34
xcodinas
https://tryton-rietveld.appspot.com/33671002/diff/150001/src/common.js File src/common.js (right): https://tryton-rietveld.appspot.com/33671002/diff/150001/src/common.js#newcode3307 src/common.js:3307: window.setTimeout(function() { On 2017/01/09 11:11:33, ced wrote: > On ...
8 months, 1 week ago (2017-01-09 11:22:24 UTC) #35
ced
https://tryton-rietveld.appspot.com/33671002/diff/150001/src/common.js File src/common.js (right): https://tryton-rietveld.appspot.com/33671002/diff/150001/src/common.js#newcode3307 src/common.js:3307: window.setTimeout(function() { On 2017/01/09 11:22:24, xcodinas wrote: > On ...
8 months, 1 week ago (2017-01-09 11:54:37 UTC) #36
xcodinas
Add download dialog
8 months ago (2017-01-16 10:47:25 UTC) #37
reviewbot
flake8 OK URL: https://codereview.tryton.org/33671002
8 months ago (2017-01-16 11:10:50 UTC) #38
ced
https://tryton-rietveld.appspot.com/33671002/diff/170001/src/common.js File src/common.js (right): https://tryton-rietveld.appspot.com/33671002/diff/170001/src/common.js#newcode3284 src/common.js:3284: Sao.common.download_dialog = Sao.class_(Sao.common.UniqueDialog, { Why make it unique? https://tryton-rietveld.appspot.com/33671002/diff/170001/src/common.js#newcode3292 ...
8 months ago (2017-01-16 11:12:57 UTC) #39
xcodinas
Fix comments
7 months, 4 weeks ago (2017-01-24 12:36:51 UTC) #40
xcodinas
https://tryton-rietveld.appspot.com/33671002/diff/170001/src/common.js File src/common.js (right): https://tryton-rietveld.appspot.com/33671002/diff/170001/src/common.js#newcode3292 src/common.js:3292: }).appendTo(dialog.body); On 2017/01/16 11:12:56, ced wrote: > Maybe we ...
7 months, 4 weeks ago (2017-01-24 12:37:06 UTC) #41
reviewbot
flake8 OK URL: https://codereview.tryton.org/33671002
7 months, 4 weeks ago (2017-01-24 12:45:27 UTC) #42
ced
https://tryton-rietveld.appspot.com/33671002/diff/190001/src/common.js File src/common.js (right): https://tryton-rietveld.appspot.com/33671002/diff/190001/src/common.js#newcode3284 src/common.js:3284: Sao.common.download_dialog = {}; I do not see the point ...
7 months, 3 weeks ago (2017-01-25 21:09:02 UTC) #43
xcodinas
Fix comments
7 months, 3 weeks ago (2017-01-26 10:56:40 UTC) #44
reviewbot
flake8 OK URL: https://codereview.tryton.org/33671002
7 months, 3 weeks ago (2017-01-26 11:16:39 UTC) #45
ced
https://tryton-rietveld.appspot.com/33671002/diff/210001/src/common.js File src/common.js (right): https://tryton-rietveld.appspot.com/33671002/diff/210001/src/common.js#newcode3285 src/common.js:3285: var type = Sao.common.guess_mimetype(); Why calling twice ? https://tryton-rietveld.appspot.com/33671002/diff/210001/src/common.js#newcode3287 ...
7 months, 3 weeks ago (2017-01-28 23:21:20 UTC) #46
xcodinas
Remove unnecessary code, add close as callback
7 months, 2 weeks ago (2017-02-02 10:11:39 UTC) #47
reviewbot
flake8 OK URL: https://codereview.tryton.org/33671002
7 months, 2 weeks ago (2017-02-02 10:41:23 UTC) #48
ced
https://tryton-rietveld.appspot.com/33671002/diff/230001/src/common.js File src/common.js (right): https://tryton-rietveld.appspot.com/33671002/diff/230001/src/common.js#newcode3301 src/common.js:3301: })).click(close); usually cleaner to start new line before each ...
7 months, 1 week ago (2017-02-07 23:37:10 UTC) #49
xcodinas
Fix style, remove submit as it's not called
7 months, 1 week ago (2017-02-08 11:21:09 UTC) #50
reviewbot
flake8 OK URL: https://codereview.tryton.org/33671002
7 months, 1 week ago (2017-02-08 11:37:16 UTC) #51
ced
https://tryton-rietveld.appspot.com/33671002/diff/250001/src/view/form.js File src/view/form.js (right): https://tryton-rietveld.appspot.com/33671002/diff/250001/src/view/form.js#newcode3247 src/view/form.js:3247: name = this.filename_field().get(this.record()); could avoid to call twice filename_field
7 months, 1 week ago (2017-02-13 23:27:40 UTC) #52
xcodinas
Avoid calling filename field twice
7 months, 1 week ago (2017-02-14 08:29:43 UTC) #53
reviewbot
flake8 OK URL: https://codereview.tryton.org/33671002
7 months, 1 week ago (2017-02-14 08:30:29 UTC) #54
ced
LGTM
7 months ago (2017-02-16 10:33:07 UTC) #55
rietveld-bot_tryton.org
7 months ago (2017-02-17 11:38:11 UTC) #56
New changeset 6a886a9fe843 by Xavier Codinas in branch 'default':
Use filename when file download
http://hg.tryton.org/sao/rev/6a886a9fe843
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cd18842