Upload Attachment View - Submit form without specifying a file
Review Request #7149 — Created April 1, 2015 and submitted
Currently, if you open the modal for file uploads, you would be presented a form for uploads. Currently, the upload button is enabled, and submission is allowed without specifying a file to submit. This patch would allow set the button to disabled by default, and only enable it if a file is specified.
Manual testing was performed.
| Description | From | Last Updated |
|---|---|---|
|
These are fairly expensive queries. We should instead cache the elements in render and reference them directly. We also don't … |
 chipx86
|
|
|
These should be pseudo-private (e.g., this._$path and this._$uploadBtn, respectively). |
 brennie
|
|
|
Can we give this a better name? Perhaps updateUploadButtonEnabledState? |
brennie
|
|
|
Could you condense the body of this to to: this._$uploadBtn.enable(this._$path.val()); |
brennie
|
VT
Change Summary:
Removed white space.
Commit: |
|
||||
|---|---|---|---|---|---|
Diff: |
Revision 2 (+19 -1) |
-
Tool: Pyflakes Ignored Files: reviewboard/static/rb/js/views/uploadAttachmentView.js Tool: PEP8 Style Checker Ignored Files: reviewboard/static/rb/js/views/uploadAttachmentView.js
-
-
reviewboard/static/rb/js/views/uploadAttachmentView.js (Diff revision 2) These are fairly expensive queries. We should instead cache the elements in
renderand reference them directly.We also don't want to look up by input value. We should instead set an ID and look that up.
VT
Change Summary:
Changed the JQuery look ups to use IDs instead of properties. These look ups cached into elements for later recalls.
Commit: |
|
||||
|---|---|---|---|---|---|
Diff: |
Revision 3 (+22 -2) |
-
Tool: PEP8 Style Checker Ignored Files: reviewboard/static/rb/js/views/uploadAttachmentView.js Tool: Pyflakes Ignored Files: reviewboard/static/rb/js/views/uploadAttachmentView.js
-
Just a few nit picks :)
-
reviewboard/static/rb/js/views/uploadAttachmentView.js (Diff revision 3) These should be pseudo-private (e.g.,
this._$pathandthis._$uploadBtn, respectively). -
reviewboard/static/rb/js/views/uploadAttachmentView.js (Diff revision 3) Can we give this a better name? Perhaps
updateUploadButtonEnabledState? -
reviewboard/static/rb/js/views/uploadAttachmentView.js (Diff revision 3) Could you condense the body of this to to:
this._$uploadBtn.enable(this._$path.val());
VT
Change Summary:
Updated the naming to match a more private variable feel, and better naming convention.
Also changed the check for enabling the upload button to be 1 line (the function
updateUploadButtonEnabledState).
Commit: |
|
||||
|---|---|---|---|---|---|
Diff: |
Revision 4 (+16 -2) |
Tool: Pyflakes Ignored Files: reviewboard/static/rb/js/views/uploadAttachmentView.js Tool: PEP8 Style Checker Ignored Files: reviewboard/static/rb/js/views/uploadAttachmentView.js