2011-02-01 Ojan Vafai <ojan@chromium.org>
authorojan@chromium.org <ojan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 2 Feb 2011 06:00:17 +0000 (06:00 +0000)
committerojan@chromium.org <ojan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 2 Feb 2011 06:00:17 +0000 (06:00 +0000)
        Reviewed by Adam Barth.

        make draft comments focusable
        https://bugs.webkit.org/show_bug.cgi?id=53554

        Makes frozen draft comments focusable. The ones that are currently being edited are not.
        I'm on the fence whether they should be, but this seems good enough for now.

        * code-review.js:

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

Websites/bugs.webkit.org/ChangeLog
Websites/bugs.webkit.org/PrettyPatch/PrettyPatch.rb
Websites/bugs.webkit.org/code-review.js

index 6dc492f..f40eea6 100644 (file)
@@ -2,6 +2,18 @@
 
         Reviewed by Adam Barth.
 
+        make draft comments focusable
+        https://bugs.webkit.org/show_bug.cgi?id=53554
+
+        Makes frozen draft comments focusable. The ones that are currently being edited are not.
+        I'm on the fence whether they should be, but this seems good enough for now.
+
+        * code-review.js:
+
+2011-02-01  Ojan Vafai  <ojan@chromium.org>
+
+        Reviewed by Adam Barth.
+
         avoid jitter when loading the comments to a patch
         https://bugs.webkit.org/show_bug.cgi?id=53570
 
index ba10936..6ac98d2 100644 (file)
@@ -417,7 +417,7 @@ body {
 }
 </style>
 <script src="https://ajax.googleapis.com/ajax/libs/jquery/1.4.2/jquery.min.js"></script> 
-<script src="code-review.js?version=32"></script>
+<script src="code-review.js?version=33"></script>
 EOF
 
     def self.revisionOrDescription(string)
index f5f4b44..acbcc48 100644 (file)
@@ -1259,11 +1259,12 @@ var CODE_REVIEW_UNITTEST;
     $(document).scrollTop(node.addClass('focused').position().top - window.innerHeight / 2);
   }
 
-  function focusNext(className, is_backward) {
-    var focusable_nodes = $('.previousComment,.DiffBlock').filter(function() {
-      return ($(this).hasClass('previousComment') || $('.add,.remove', this).size());
+  function focusNext(filter, direction) {
+    var focusable_nodes = $('.frozenComment,.previousComment,.DiffBlock').filter(function() {
+      return ($(this).hasClass('frozenComment') || $(this).hasClass('previousComment') || $('.add,.remove', this).size());
     });
 
+    var is_backward = direction == DIRECTION.BACKWARD;
     var index = focusable_nodes.index($('.focused'));
     if (index == -1 && is_backward)
       index = focusable_nodes.length;
@@ -1272,18 +1273,28 @@ var CODE_REVIEW_UNITTEST;
     var end = is_backward ? -1 : focusable_nodes.size();
     for (var i = index + offset; i != end; i = i + offset) {
       var node = $(focusable_nodes[i]);
-      if (node.hasClass(className)) {
+      if (filter(node)) {
         focusOn(node);
         return;
       }
     }
   }
 
+  var DIRECTION = {FORWARD: 1, BACKWARD: 2};
+
   var kCharCodeForN = 'n'.charCodeAt(0);
   var kCharCodeForP = 'p'.charCodeAt(0);
   var kCharCodeForJ = 'j'.charCodeAt(0);
   var kCharCodeForK = 'k'.charCodeAt(0);
 
+  function isComment(node) {
+    return node.hasClass('frozenComment') || node.hasClass('previousComment');
+  }
+  
+  function isDiffBlock(node) {
+    return node.hasClass('DiffBlock');
+  }
+
   $('body').live('keypress', function() {
     // FIXME: There's got to be a better way to avoid seeing these keypress
     // events.
@@ -1292,19 +1303,19 @@ var CODE_REVIEW_UNITTEST;
 
     switch (event.charCode) {
     case kCharCodeForN:
-      focusNext('previousComment', false);
+      focusNext(isComment, DIRECTION.FORWARD);
       break;
 
     case kCharCodeForP:
-      focusNext('previousComment', true);
+      focusNext(isComment, DIRECTION.BACKWARD);
       break;
 
     case kCharCodeForJ:
-      focusNext('DiffBlock', false);
+      focusNext(isDiffBlock, DIRECTION.FORWARD);
       break;
 
     case kCharCodeForK:
-      focusNext('DiffBlock', true);
+      focusNext(isDiffBlock, DIRECTION.BACKWARD);
       break;
     }
   });