• 
      

    Rewrite the file attachment thumbnail code to be a Backbone view.

    Review Request #4050 — Created April 14, 2013 and submitted

    Information

    Review Board
    master

    Reviewers

    Rewrite the file attachment thumbnail code to be a Backbone view.
    
    The new FileAttachmentThumbnail view can be used with pre-rendered
    file attachment thumbnails (loaded in from the review request
    template) and also with newly uploaded files.
    
    It also replaces the old file attachment thumbnail placeholder code.
    It recognizes when we haven't yet finished an upload of a file, and
    will show a spinner in that case, eventually replacing it with the
    contents when it's loaded.
    Tested existing thumbnails with published captions and draft captions.
    
    Tested uploading files through drag-and-drop.
    
    Tested uploading files through Add File, with and without a caption.
    
    Tested changing captions.
    
    Tested deleting draft file attachments.
    
    Tested deleting published file attachments.
    
    Unit tests pass.
    Description From Last Updated

    You can use this.$('.file-caption'), etc

    daviddavid

    Same here with this.$

    daviddavid

    You don't need the quotes around the object keys here.

    daviddavid

    These can go inline in the var statement without adding much overhead.

    daviddavid

    How about using the :empty pseudo-class?

    daviddavid

    You can use view.$() here too. Same below.

    daviddavid
    reviewbot
    1. This is a review from Review Bot.
        Tool: PEP8 Style Checker
        Processed Files:
          reviewboard/settings.py
        Ignored Files:
          reviewboard/static/rb/js/reviews.js
          reviewboard/static/rb/js/views/fileAttachmentThumbnailView.js
          reviewboard/templates/reviews/review_request_box.html
          reviewboard/static/rb/js/views/tests/fileAttachmentThumbnailViewTests.js
          reviewboard/static/rb/js/views/dndUploaderView.js
      
      
    2. 
        
    david
    1. One thing I'd vaguely noticed was that file attachments that were on existing loaded pages and file attachments that got created from javascript didn't have identical HTML. Can you verify that it's the same after your change?
    2. While it's > 80 columns, I think this would make a lot more sense on one line.
      1. Going to try a different way of wrapping.
    3. Show all issues
      You can use this.$('.file-caption'), etc
    4. Show all issues
      Same here with this.$
    5. reviewboard/static/rb/js/views/fileAttachmentThumbnailView.js (Diff revision 1)
       
       
       
       
       
       
       
       
       
       
       
       
      Show all issues
      You don't need the quotes around the object keys here.
    6. Show all issues
      These can go inline in the var statement without adding much overhead.
    7. Show all issues
      How about using the :empty pseudo-class?
      1. It's not actually empty, so the pseudo-class won't work. We display content.
    8. Show all issues
      You can use view.$() here too. Same below.
    9. 
        
    chipx86
    reviewbot
    1. This is a review from Review Bot.
        Tool: PEP8 Style Checker
        Processed Files:
          reviewboard/settings.py
        Ignored Files:
          reviewboard/static/rb/js/reviews.js
          reviewboard/static/rb/js/views/fileAttachmentThumbnailView.js
          reviewboard/templates/reviews/review_request_box.html
          reviewboard/static/rb/js/views/tests/fileAttachmentThumbnailViewTests.js
          reviewboard/static/rb/js/views/dndUploaderView.js
      
      
    2. 
        
    david
    1. Ship It!
    2. 
        
    chipx86
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to master (4373ac9)