Upload Attachment View - Submit form without specifying a file

Review Request #7149 — Created April 1, 2015 and submitted

Information

Review Board
master
4956318...

Reviewers

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 …

chipx86chipx86

These should be pseudo-private (e.g., this._$path and this._$uploadBtn, respectively).

brenniebrennie

Can we give this a better name? Perhaps updateUploadButtonEnabledState?

brenniebrennie

Could you condense the body of this to to: this._$uploadBtn.enable(this._$path.val());

brenniebrennie
reviewbot
  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
    
    
  2. 
      
VT
reviewbot
  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
    
    
  2. 
      
chipx86
  1. 
      
  2. reviewboard/static/rb/js/views/uploadAttachmentView.js (Diff revision 2)
     
     
     
     
     
     

    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.

  3. 
      
VT
reviewbot
  1. Tool: PEP8 Style Checker
    Ignored Files:
        reviewboard/static/rb/js/views/uploadAttachmentView.js
    
    
    
    Tool: Pyflakes
    Ignored Files:
        reviewboard/static/rb/js/views/uploadAttachmentView.js
    
    
  2. 
      
brennie
  1. Just a few nit picks :)

  2. These should be pseudo-private (e.g., this._$path and this._$uploadBtn, respectively).

  3. Can we give this a better name? Perhaps updateUploadButtonEnabledState?

  4. 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());
    
  5. 
      
VT
reviewbot
  1. Tool: PEP8 Style Checker
    Ignored Files:
        reviewboard/static/rb/js/views/uploadAttachmentView.js
    
    
    
    Tool: Pyflakes
    Ignored Files:
        reviewboard/static/rb/js/views/uploadAttachmentView.js
    
    
  2. 
      
VT
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-2.5.x (eace550)
Loading...