2011-02-16 Ojan Vafai <ojan@chromium.org>

        Reviewed by Adam Barth.

        keyboard support for extending/shrinking comment context
        https://bugs.webkit.org/show_bug.cgi?id=54612

        ctrl+shift+up/down will extend/shrink the comment context when
        a comment is focused or when one is being edited.

        Also, switch over to using keydown instead of keypress events.
        This lets us share code for handling escape and other key events.
        Also, keypress is evil and should die.

        * code-review.js:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@79191 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Websites/bugs.webkit.org/ChangeLog b/Websites/bugs.webkit.org/ChangeLog
index ef57a7756eb403e600b25a600841264be10deb00..bae1bd017bf682c610e84f568e550ddaa28c8cc4 100644 (file)
--- a/Websites/bugs.webkit.org/ChangeLog
+++ b/Websites/bugs.webkit.org/ChangeLog
@@ -1,3 +1,19 @@
+2011-02-16  Ojan Vafai  <ojan@chromium.org>
+
+        Reviewed by Adam Barth.
+
+        keyboard support for extending/shrinking comment context
+        https://bugs.webkit.org/show_bug.cgi?id=54612
+
+        ctrl+shift+up/down will extend/shrink the comment context when
+        a comment is focused or when one is being edited.
+
+        Also, switch over to using keydown instead of keypress events.
+        This lets us share code for handling escape and other key events.
+        Also, keypress is evil and should die.
+
+        * code-review.js:
+
 2011-02-20  Ojan Vafai  <ojan@chromium.org>
 
         Recommit accidental revert.

diff --git a/Websites/bugs.webkit.org/PrettyPatch/PrettyPatch.rb b/Websites/bugs.webkit.org/PrettyPatch/PrettyPatch.rb
index deab82920ee4bf86736352922b03ed927752b7c1..4c84aa0f509aaecda166f90ac74c59d797826e79 100644 (file)
--- a/Websites/bugs.webkit.org/PrettyPatch/PrettyPatch.rb
+++ b/Websites/bugs.webkit.org/PrettyPatch/PrettyPatch.rb
@@ -451,7 +451,7 @@ div:focus {
 }
 </style>
 <script src="https://ajax.googleapis.com/ajax/libs/jquery/1.4.2/jquery.min.js"></script> 
-<script src="code-review.js?version=37"></script>
+<script src="code-review.js?version=38"></script>
 EOF
 
     def self.revisionOrDescription(string)

diff --git a/Websites/bugs.webkit.org/code-review.js b/Websites/bugs.webkit.org/code-review.js
index a5081245700a09580deea7337624520a398ef40a..4bd67fb81fd1f3e022a7e5940b32df3d584464b8 100644 (file)
--- a/Websites/bugs.webkit.org/code-review.js
+++ b/Websites/bugs.webkit.org/code-review.js
@@ -71,13 +71,15 @@ var CODE_REVIEW_UNITTEST;
   var SIDE_BY_SIDE_DIFFS_KEY = 'sidebysidediffs';
   var g_displayed_draft_comments = false;
   var KEY_CODE = {
+    down: 40,
     enter: 13,
     escape: 27,
     j: 74,
     k: 75,
     n: 78,
     p: 80,
-    r: 82
+    r: 82,
+    up: 38
   }
 
   function idForLine(number) {
@@ -149,9 +151,7 @@ var CODE_REVIEW_UNITTEST;
     var start = numberFrom(start_line_id);
     var end = numberFrom(end_line_id);
     for (var i = start; i <= end; i++) {
-      var line = $('#line' + i);
-      line.addClass('commentContext');
-      addDataCommentBaseLine(line, end_line_id);
+      addDataCommentBaseLine($('#line' + i), end_line_id);
     }
 
     var comment_block = createCommentFor(line);
@@ -1075,6 +1075,8 @@ var CODE_REVIEW_UNITTEST;
               '<tr><td>n</td><td>focus next comment</td></tr>' +
               '<tr><td>p</td><td>focus previous comment</td></tr>' +
               '<tr><td>r</td><td>focus review select element</td></tr>' +
+              '<tr><td>ctrl + shift + up</td><td>extend context of the focused comment</td></tr>' +
+              '<tr><td>ctrl + shift + down</td><td>shrink context of the focused comment</td></tr>' +
             '</table></div>' +
           '</div>' +
         '</div>' +
@@ -1425,7 +1427,71 @@ var CODE_REVIEW_UNITTEST;
     return node.hasClass('Line');
   }
 
+  function commentTextareaForKeyTarget(key_target) {
+    if (key_target.nodeName == 'TEXTAREA')
+      return $(key_target);
+
+    var comment_textarea = $(document.activeElement).prev().find('textarea');
+    if (!comment_textarea.size())
+      return null;
+    return comment_textarea;
+  }
+
+  function extendCommentContextUp(key_target) {
+    var comment_textarea = commentTextareaForKeyTarget(key_target);
+    if (!comment_textarea)
+      return;
+
+    var comment_base_line = comment_textarea.attr('data-comment-for');
+    var diff_section = diffSectionFor(comment_textarea);
+    var lines = $('.Line', diff_section);
+    for (var i = 0; i < lines.length - 1; i++) {
+      if (hasDataCommentBaseLine(lines[i + 1], comment_base_line)) {
+        addDataCommentBaseLine(lines[i], comment_base_line);
+        break;
+      }
+    }
+  }
+
+  function shrinkCommentContextDown(key_target) {
+    var comment_textarea = commentTextareaForKeyTarget(key_target);
+    if (!comment_textarea)
+      return;
+
+    var comment_base_line = comment_textarea.attr('data-comment-for');
+    var diff_section = diffSectionFor(comment_textarea);
+    var lines = contextLinesFor(comment_base_line, diff_section);
+    if (lines.size() > 1)
+      removeDataCommentBaseLine(lines[0], comment_base_line);
+  }
+
+  function handleModifyContextKey(e) {
+    var handled = false;
+
+    if (e.shiftKey && e.ctrlKey) {
+      switch (e.keyCode) {
+      case KEY_CODE.up:
+        extendCommentContextUp(e.target);
+        handled = true;
+        break;
+
+      case KEY_CODE.down:
+        shrinkCommentContextDown(e.target);
+        handled = true;
+        break;
+      }
+    }
+
+    if (handled)
+      e.preventDefault();
+
+    return handled;
+  }
+
   $('textarea').live('keydown', function(e) {
+    if (handleModifyContextKey(e))
+      return;
+
     if (e.keyCode == KEY_CODE.escape)
       handleEscapeKeyInTextarea(this);
   });
@@ -1435,9 +1501,11 @@ var CODE_REVIEW_UNITTEST;
     // events.
     if (e.target.nodeName == 'TEXTAREA')
       return;
-    
-    var handled = false;
 
+    if (handleModifyContextKey(e))
+      return;
+
+    var handled = false;
     switch (e.keyCode) {
     case KEY_CODE.r:
       $('.review select').focus();
@@ -1467,7 +1535,7 @@ var CODE_REVIEW_UNITTEST;
       break;
       
     case KEY_CODE.enter:
-      handled = handleEnterKeyPress();
+      handled = handleEnterKey();
       break;
       
     case KEY_CODE.escape:
@@ -1489,7 +1557,7 @@ var CODE_REVIEW_UNITTEST;
     document.body.focus();
   }
   
-  function handleEnterKeyPress() {
+  function handleEnterKey() {
     if (document.activeElement.nodeName == 'BODY')
       return;
 
@@ -1543,8 +1611,6 @@ var CODE_REVIEW_UNITTEST;
         return;
 
       removeDataCommentBaseLine(this, comment_base_line);
-      if (!$(this).attr('data-comment-base-line'))
-        $(this).removeClass('commentContext');
     });
   }
 
@@ -1666,7 +1732,6 @@ var CODE_REVIEW_UNITTEST;
       return;
 
     var already_has_comment = lines.last().hasClass('commentContext');
-    lines.addClass('commentContext');
 
     var comment_base_line;
     if (already_has_comment)
@@ -1685,15 +1750,26 @@ var CODE_REVIEW_UNITTEST;
     saveDraftComments();
   }
 
-  function addDataCommentBaseLine(line, id) {
+  function hasDataCommentBaseLine(line, id) {
     var val = $(line).attr('data-comment-base-line');
+    if (!val)
+      return false;
 
-    var parts = val ? val.split(' ') : [];
+    var parts = val.split(' ');
     for (var i = 0; i < parts.length; i++) {
       if (parts[i] == id)
-        return;
+        return true;
     }
+    return false;
+  }
 
+  function addDataCommentBaseLine(line, id) {
+    $(line).addClass('commentContext');
+    if (hasDataCommentBaseLine(line, id))
+      return;
+
+    var val = $(line).attr('data-comment-base-line');
+    var parts = val ? val.split(' ') : [];
     parts.push(id);
     $(line).attr('data-comment-base-line', parts.join(' '));
   }
@@ -1705,13 +1781,19 @@ var CODE_REVIEW_UNITTEST;
 
     var base_lines = comment_base_lines.split(' ');
     var parts = val.split(' ');
-    var newVal = [];
+    var new_parts = [];
     for (var i = 0; i < parts.length; i++) {
       if (base_lines.indexOf(parts[i]) == -1)
-        newVal.push(parts[i]);
+        new_parts.push(parts[i]);
     }
 
-    $(line).attr('data-comment-base-line', newVal.join(' '));
+    var new_comment_base_line = new_parts.join(' ');
+    if (new_comment_base_line)
+      $(line).attr('data-comment-base-line', new_comment_base_line);
+    else {
+      $(line).removeAttr('data-comment-base-line');
+      $(line).removeClass('commentContext');
+    }
   }
 
   function lineFromLineDescendant(descendant) {