• 
      

    Use CSS for the comment flags, make them multi-line, and improve positioning.

    Review Request #1586 — Created May 8, 2010 and submitted

    Information

    Review Board
    master

    Reviewers

    Use CSS for the comment flags, make them multi-line, and improve positioning.
    
    Previously, we used images for the comment flags. This looked really bad
    when zoomed in. We also overlapped the line numbers.
    
    Now we render the comment flags as little rounded rectangles with
    shadows. The little pointer is gone, which is fine given that it's obvious
    which line the flag is pointing to. We also leave enough room for the flag
    so that the line number is not overlayed. This helps with seeing which
    line a comment flag is on, and also helps with selecting lines that already
    have a flag.
    
    The comment flags were always shown only on one line, which didn't really
    give a good idea as to how many lines the comment spanned. Now the comment
    flag spans the number of lines it represents.
    
    As a nice little touch, the comment flags are now slightly highlighted when
    the starting line or the flag itself is selected.
    Tested the new flags on IE6, IE7, Firefox 3.6, and Safari.
    
    Made sure the comments had the right height when lines wrapped and when resizing the browser.
    
    Tested all basic interaction (creation of them, loading, clicking).
    david
    1. This is super cute, and I haven't looked at the code yet, but what happens when regions overlap?
      1. It works pretty well. The higher comment flags (higher on the page) are lower on the stack. So, If you have a comment on lines 1-4, and a comment on line 3, the one on 3 will be on top of the range between 1-4.
      2. an you add a screenshot?
    2. 
        
    chipx86
    Review request changed
    Change Summary:
    Adding a screenshot showing overlapping comment flags.
    Screenshot Captions:
    overlapping-comment-flags.png:
    Overlapping comment flags
    david
    1. 
        
    2. Does this work when the text size is changed in the browser? If so, ship it.
      1. Yep, works fine. Thanks!
    3.