Summary

Upload Attachment View - Submit form without specifying a file

Review Request #7149 — Created April 1, 2015 and submitted April 1, 2015, 9:39 p.m.

Information

Owner

VTL-Developer

Repository

Review Board

Branch

master

Bugs

3829

Depends On

Commit

4956318...

Reviewers

Groups

reviewboard, students

People

Description

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.

Testing Done

Manual testing was performed.

Issues

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	April 1, 2015, 6:04 p.m.
These should be pseudo-private (e.g., this._$path and this._$uploadBtn, respectively).	brennie	April 1, 2015, 9:17 p.m.
Can we give this a better name? Perhaps updateUploadButtonEnabledState?	brennie	April 1, 2015, 9:17 p.m.
Could you condense the body of this to to: this._$uploadBtn.enable(this._$path.val());	brennie	April 1, 2015, 9:20 p.m.

Tool: Pyflakes
Ignored Files:
    reviewboard/static/rb/js/views/uploadAttachmentView.js



Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/static/rb/js/views/uploadAttachmentView.js

Change Summary:

Removed white space.

Commit:

bd5a2fbff0fc28fcff3d0297fd893f37f7fd1e47

242ea429d73f07132908bf82775b723162ddf88d

Diff:

Revision 2 (+19 -1)

Show changes

reviewboard/static/rb/js/views/uploadAttachmentView.js

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)

The issue has been resolved. Show all issues

These are fairly expensive queries. We should instead cache the elements in render and reference them directly.

We also don't want to look up by input value. We should instead set an ID and look that up.

Change Summary:

Changed the JQuery look ups to use IDs instead of properties. These look ups cached into elements for later recalls.

Commit:

242ea429d73f07132908bf82775b723162ddf88d

ec4af1d80885306eff5e2178353895a50fa8cb5b

Diff:

Revision 3 (+22 -2)

Show changes

reviewboard/static/rb/js/views/uploadAttachmentView.js

Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/static/rb/js/views/uploadAttachmentView.js



Tool: Pyflakes
Ignored Files:
    reviewboard/static/rb/js/views/uploadAttachmentView.js

Ship it!

```
Just a few nit picks :)
```
reviewboard/static/rb/js/views/uploadAttachmentView.js (Diff revision 3)
The issue has been resolved. Show all issues
```
These should be pseudo-private (e.g., this._$path and this._$uploadBtn, respectively).
```
reviewboard/static/rb/js/views/uploadAttachmentView.js (Diff revision 3)
The issue has been resolved. Show all issues
```
Can we give this a better name? Perhaps updateUploadButtonEnabledState?
```

reviewboard/static/rb/js/views/uploadAttachmentView.js (Diff revision 3)

The issue has been resolved. Show all issues

Could you condense the body of this to to:

this._$uploadBtn.enable(this._$path.val());

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:

ec4af1d80885306eff5e2178353895a50fa8cb5b

495631806bc638b32542106a909c96e4d0f78b22

Diff:

Revision 4 (+16 -2)

Show changes

reviewboard/static/rb/js/views/uploadAttachmentView.js

Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/static/rb/js/views/uploadAttachmentView.js



Tool: Pyflakes
Ignored Files:
    reviewboard/static/rb/js/views/uploadAttachmentView.js

Status:: Completed
Change Summary:: Pushed to release-2.5.x (eace550)