• 
      

    Tweak a bunch of things with file attachments.

    Review Request #3569 — Created Nov. 27, 2012 and submitted

    Information

    Review Board
    release-1.7.x

    Reviewers

    Tweak a bunch of things with file attachments.
    
    This change makes a bunch of tweaks to the file attachments view. Probably some
    similar tweaks could be made to screenshots but seeing as they're effectively
    deprecated, I didn't see the need.
    
    First of all, I've linked the file's thumbnail to the review UI, if appropriate.
    This was probably my #1 annoyance with the new system.
    
    Next, the spacing was pretty ugly. This ended up being the product of several
    different issues:
    - The filename had way too much spacing, which forced it down into an ugly
      position instead of seeming like a title. I've eliminated the top and bottom
      padding.
    - The thumbnail image had a max-height of 6em, and the entire file had a height
      of 13em, which meant that the thumbnail image got scaled down in an ugly way
      (to about 68px instead of 100px) and the caption bar got way too much vertical
      space. To fix this, I made the height of the thumbnail always be 100px and
      eliminated the hard-coded height of the file div, letting it scale to fit its
      children.
    - After this, attachments with no caption would end up with a collapsed caption
      area. I've given it an explicit height so it's always pretty.
    - The empty captions were pretty crappy. I've made it so if there's no caption,
      it explicitly says "no caption" in grey italic and added some javascript to
      the inlineEditor's "complete" handler to toggle the approprate classes.
    Attached several images to a review request and double checked all the spacing.
    Saved one caption as empty and reloaded the page to see the "No caption" text.
    Edited that caption and saw the style switch back to normal. Removed the caption
    and saw it be replaced with the italic "No caption". Verified that both draft
    and non-draft captions worked as expected.
    
    Clicked on the thumbnail images to load up the review UI.
    
    Verified appearance in Chrome, Firefox, and IE.

    Description From Last Updated

    === What happens if someone has a caption of "No caption" ? Can we instead check the empty-caption class?

    chipx86 chipx86

    Feels like we should be able to condense this. The two things that change are the classes and the text. …

    chipx86 chipx86

    You can now do elif

    chipx86 chipx86
    chipx86
    1. 
        
      1. Oh, what does this look like with other browsers? Curious about Firefox and IE.
      2. It looks the same as the screenshots in both firefox and IE.
    2. reviewboard/static/rb/js/reviews.js (Diff revision 1)
       
       
      Show all issues
      ===
      
      What happens if someone has a caption of "No caption" ? Can we instead check the empty-caption class?
    3. reviewboard/templates/reviews/review_request_box.html (Diff revision 1)
       
       
       
       
       
       
       
       
       
       
       
       
       
       
      Show all issues
      Feels like we should be able to condense this. The two things that change are the classes and the text. Maybe instead of all this, we can use {% definevar %} from djblets to set the class and text, and then just output the <a> once.
    4. Show all issues
      You can now do elif
    5. 
        
    david
    chipx86
    1. Ship It!
    2. 
        
    david
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-1.7.x (a1e7fef).