Fix for Javascript error to center selected diff chunk by pressing "Enter" -- added "c" as a supplemental hotkey

Review Request #1507 — Created April 5, 2010 and submitted

Information

Review Board

Reviewers

The following error is reported in 1.5 Beta 1 or 2 when selecting a diff chunk and pressing "enter":

function scrollToAnchor(anchor, noscroll) {
    if (anchor.length == 0) {
        return false;
    }
 
    if (anchor.parent().is(":hidden")) {
Uncaught TypeError: Object  has no method 'parent'
        return false;
    }
 
The fix is to make a call to jQuery before calling scrollToAnchor.

In addition, as I'm right handed, I found it inconvenient to select a chunk using the mouse in my right hand, and then having to press "enter".  I'm proposing to add another key, "c" for center, as it is not used already.  
Verified on Windows for Chrome, Firefox and IE.  Both "c" and "enter" behave as expected (re-centered the selected diff chunk).
LO
  1. 
      
  2. I just realized that "c" is already used to jump to the next comment.  So not a really good choice.
    
    Maybe "z", a little bit similar to vi.  Or no supplemental key at all.
    1. Probably fine to just keep Enter.
    2. Good for me.  Will you commit the fix for line 51?  Or do you want me to submit a new diff?
  3. 
      
chipx86
  1. Committed (with the "c" removed) to master (300f9d1). Thanks!
  2. 
      
Loading...