Add a smart tooltip to show descriptions in the diffviewer

Review Request #7256 — Created April 27, 2015 and submitted

brennie
Review Board
dvcs
fe4fb6d...
reviewboard

Hovering over a summary in the commit description table now shows a
tooltip with copyable text if the commit's description is longer than
he summary. Moving the mouse off of the tooltip causes it to disappear.

Verified that the description was correctly shown.

The tooltip's text align's pixel-perfectly with the table's text in
Chrome, Firefox, and Safari on OS X.

Description From Last Updated

Blank line after this.

daviddavid

Undo this change.

daviddavid

If you're using .text(), you shouldn't also need to escape.

daviddavid

These should go up below background-color. Also, no blank lines between the rules (as it gets harder to differentiate rules ...

chipx86chipx86

Should be .box-shadow(...).

chipx86chipx86

We have a new .transition for this that does all the vendor-specific prefixes (though you'll need to pass duration as ...

chipx86chipx86

This will put the cid key in diffCommit.attributes, which probably isn't what you want. You can instead put {} as ...

chipx86chipx86

_.noop is built for this purpose. Not a big difference, except that it's one less function to create.

chipx86chipx86

Where does the 9 come from?

chipx86chipx86

Can you put the function body on its own line?

chipx86chipx86

This doesn't seem like a useful rule :)

daviddavid
reviewbot
  1. Tool: Pyflakes
    Ignored Files:
        reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.js
        reviewboard/static/rb/css/pages/diffviewer.less
    
    
    
    Tool: PEP8 Style Checker
    Ignored Files:
        reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.js
        reviewboard/static/rb/css/pages/diffviewer.less
    
    
  2. 
      
david
  1. 
      
  2. Blank line after this.

  3. Undo this change.

  4. If you're using .text(), you shouldn't also need to escape.

    1. Oops that should be .html().

  5. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Ignored Files:
        reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.js
        reviewboard/static/rb/css/pages/diffviewer.less
    
    
    
    Tool: PEP8 Style Checker
    Ignored Files:
        reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.js
        reviewboard/static/rb/css/pages/diffviewer.less
    
    
  2. 
      
chipx86
  1. 
      
  2. reviewboard/static/rb/css/pages/diffviewer.less (Diff revision 2)
     
     
     
     
     

    These should go up below background-color. Also, no blank lines between the rules (as it gets harder to differentiate rules and nested styles).

    Also, should use .border-radius(...).

  3. Should be .box-shadow(...).

  4. We have a new .transition for this that does all the vendor-specific prefixes (though you'll need to pass duration as part of the value).

  5. This will put the cid key in diffCommit.attributes, which probably isn't what you want. You can instead put {} as the first value to _.extend(), or use _.defaults. I think the latter is probably the best way to go.

    I also think, for future expansion, that this should be more like:

    tr = this._itemTemplate(_.defaults(
        {
            cid: diffCommit.cid
        },
        diffCommit.attributes));
    
  6. _.noop is built for this purpose. Not a big difference, except that it's one less function to create.

  7. Where does the 9 come from?

    1. The 9 comes from me trying to figure out how to make the tooltip pixel perfect. It isn't and I'm not sure how to get it there.

    2. Okay. Usually, the problem has to do with the padding of the parent, or border sizes if not using .box-sizing(border-box).

      Assuming it's padding, you can use $myElement.getExtents('p', 'l') and .getExtents('p', 't') to get the left and top padding
      of the element. You can also stick b and m in there if you need to calculate borders and margins, respectively.

    3. Alright, I'll give this a shot.

    4. So its quite close to being pixel perfect, but its off by just a bit now. I'm not sure how to get it any closer.

  8. Can you put the function body on its own line?

  9. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/datagrids/columns.py
        reviewboard/reviews/templatetags/reviewtags.py
    
    Ignored Files:
        reviewboard/static/rb/js/newReviewRequest/views/repositoryView.js
        reviewboard/templates/datagrids/datagrid.html
        reviewboard/static/rb/js/newReviewRequest/views/newReviewRequestView.js
        reviewboard/templates/reviews/review_reply.html
        reviewboard/static/rb/css/pages/newReviewRequest.less
        reviewboard/static/rb/js/views/uploadDiffView.js
        reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
        reviewboard/static/rb/css/pages/diffviewer.less
        reviewboard/static/rb/css/ui/sidebars.less
        reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.js
        reviewboard/static/rb/css/pages/base.less
        reviewboard/static/rb/css/ui/datagrids.less
        reviewboard/static/rb/css/pages/admin.less
        reviewboard/static/rb/js/newReviewRequest/views/commitView.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/templates/reviews/new_review_request.html
        reviewboard/static/rb/css/defs.less
        reviewboard/static/rb/js/newReviewRequest/views/repositorySelectionView.js
        reviewboard/static/rb/css/mixins/style.less
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/datagrids/columns.py
        reviewboard/reviews/templatetags/reviewtags.py
    
    Ignored Files:
        reviewboard/static/rb/js/newReviewRequest/views/repositoryView.js
        reviewboard/templates/datagrids/datagrid.html
        reviewboard/static/rb/js/newReviewRequest/views/newReviewRequestView.js
        reviewboard/templates/reviews/review_reply.html
        reviewboard/static/rb/css/pages/newReviewRequest.less
        reviewboard/static/rb/js/views/uploadDiffView.js
        reviewboard/static/rb/js/newReviewRequest/views/postCommitView.js
        reviewboard/static/rb/css/pages/diffviewer.less
        reviewboard/static/rb/css/ui/sidebars.less
        reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.js
        reviewboard/static/rb/css/pages/base.less
        reviewboard/static/rb/css/ui/datagrids.less
        reviewboard/static/rb/css/pages/admin.less
        reviewboard/static/rb/js/newReviewRequest/views/commitView.js
        reviewboard/static/rb/css/pages/reviews.less
        reviewboard/templates/reviews/new_review_request.html
        reviewboard/static/rb/css/defs.less
        reviewboard/static/rb/js/newReviewRequest/views/repositorySelectionView.js
        reviewboard/static/rb/css/mixins/style.less
    
    
  2. 
      
brennie
reviewbot
  1. Tool: PEP8 Style Checker
    Ignored Files:
        reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.js
        reviewboard/static/rb/css/pages/diffviewer.less
    
    
    
    Tool: Pyflakes
    Ignored Files:
        reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.js
        reviewboard/static/rb/css/pages/diffviewer.less
    
    
  2. 
      
brennie
reviewbot
  1. Tool: PEP8 Style Checker
    Ignored Files:
        reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.js
        reviewboard/static/rb/css/pages/diffviewer.less
    
    
    
    Tool: Pyflakes
    Ignored Files:
        reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.js
        reviewboard/static/rb/css/pages/diffviewer.less
    
    
  2. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Ignored Files:
        reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.js
        reviewboard/static/rb/css/pages/diffviewer.less
    
    
    
    Tool: PEP8 Style Checker
    Ignored Files:
        reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.js
        reviewboard/static/rb/css/pages/diffviewer.less
    
    
  2. 
      
brennie
  1. 
      
  2. So I've managed to replace one magic number (9) with another (2), but this time it aligns pixel perfectly on all of my browsers.

    I'm not sure if there is a better way to calculate this without it being a gigantic pain.

  3. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Ignored Files:
        reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.js
        reviewboard/static/rb/css/pages/diffviewer.less
    
    
    
    Tool: PEP8 Style Checker
    Ignored Files:
        reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.js
        reviewboard/static/rb/css/pages/diffviewer.less
    
    
  2. 
      
david
  1. 
      
  2. This doesn't seem like a useful rule :)

    1. Meant to fix that and uploaded the same commit instead! 1am coding is best coding.

  3. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Ignored Files:
        reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.js
        reviewboard/static/rb/css/pages/diffviewer.less
    
    
    
    Tool: PEP8 Style Checker
    Ignored Files:
        reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.js
        reviewboard/static/rb/css/pages/diffviewer.less
    
    
  2. 
      
brennie
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to dvcs (a220c2d)
Loading...