Open the appropriate comment flag when clicking on the line number.

Review Request #1524 — Created April 21, 2010 and submitted


Review Board


Open the appropriate comment flag when clicking on the line number.

When clicking on the line number of a row containing a comment flag, we
would display a brand new comment box, instead of allowing the existing
comment to be edited. We now properly find the comment box and show it.
Opened a line by clicking on the line number instead of the comment flag. Saw the comment box.
This was caught by the new Selenium test suite.
  2. "this range" is a little confusing here.
Review request changed

Change Summary:

Fixed a problem with clicking cells that didn't already contain comments.


Revision 3 (+28 -9)

Show changes

  2. reviewboard/htdocs/media/rb/js/diffviewer.js (Diff revisions 1 - 3)
    Can you call this "newCommentDlg"? Either way we show a comment dialog.
  1. The code looks alright to me.  I tried out the patch, and it fixes the problem, though I noticed one tiny bit of weird behaviour:
    This is me clicking a single line without existing comments:
    It opens normally, and the dialog appears just below the line that I clicked (line 56 in this case).
    This is me clicking a single line that has an existing comment on it:
    The dialog opens quite a bit further down the screen, causing the page to jump a little.  In this screenshot, I had to scroll back up to get the window where it roughly was for the first capture.
    It's really not a big deal, but I thought I'd bring it to your attention.
    1. Thanks for catching that. I'll look into a fix before this code goes in.
    2. Pretty sure this is due to multi-line comments. We show the top of the comment box near the bottom of the range. Definitely wrong. I'll fix this in another change.