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

        Reviewed by Adam Barth.

        save overall comments when saving drafts in the review tool
        https://bugs.webkit.org/show_bug.cgi?id=54165

        -save overall comments in localstorage as well
        -save all draft comments as you type
        -give a *subtle* indicator of saved state

        The latter should also make it super easy if someone wanted to do
        the work to store draft comments in appengine/s3/bugzilla/etc.

        * PrettyPatch/PrettyPatch.rb:
        * code-review-test.html:
        * code-review.js:

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

diff --git a/Websites/bugs.webkit.org/ChangeLog b/Websites/bugs.webkit.org/ChangeLog
index fced929e539e4b80848995637e5bf5992edbff62..0977cd611de4a44944c141ac740c17137e2893ed 100644 (file)
--- a/Websites/bugs.webkit.org/ChangeLog
+++ b/Websites/bugs.webkit.org/ChangeLog
@@ -1,3 +1,21 @@
+2011-02-09  Ojan Vafai  <ojan@chromium.org>
+
+        Reviewed by Adam Barth.
+
+        save overall comments when saving drafts in the review tool
+        https://bugs.webkit.org/show_bug.cgi?id=54165
+
+        -save overall comments in localstorage as well
+        -save all draft comments as you type
+        -give a *subtle* indicator of saved state
+
+        The latter should also make it super easy if someone wanted to do
+        the work to store draft comments in appengine/s3/bugzilla/etc.
+
+        * PrettyPatch/PrettyPatch.rb:
+        * code-review-test.html:
+        * code-review.js:
+
 2011-02-08  Ojan Vafai  <ojan@chromium.org>
 
         Reviewed by Adam Barth.

diff --git a/Websites/bugs.webkit.org/PrettyPatch/PrettyPatch.rb b/Websites/bugs.webkit.org/PrettyPatch/PrettyPatch.rb
index ad22bf5954f3d78b66e9aab296ef5ce17a4beda1..53a1da1ac0aa17cef5260e5a62cc85b1ccdb8daf 100644 (file)
--- a/Websites/bugs.webkit.org/PrettyPatch/PrettyPatch.rb
+++ b/Websites/bugs.webkit.org/PrettyPatch/PrettyPatch.rb
@@ -313,6 +313,7 @@ body {
   right: 10%;
   top: 10%;
   bottom: 10%;
+  background: white;
 }
 
 .lightbox iframe {
@@ -423,12 +424,30 @@ body {
   display: none;
 }
 
+.autosave-state {
+  position: absolute;
+  right: 0;
+  top: -1.3em;
+  padding: 0 3px;
+  outline: 1px solid #DDD;
+  color: #8FDF5F;
+  font-size: small;   
+  background-color: #EEE;
+}
+
+.autosave-state:empty {
+  outline: 0px;
+}
+.autosave-state.saving {
+  color: #E98080;
+}
+
 .clear_float {
     clear: both;
 }
 </style>
 <script src="https://ajax.googleapis.com/ajax/libs/jquery/1.4.2/jquery.min.js"></script> 
-<script src="code-review.js?version=33"></script>
+<script src="code-review.js?version=34"></script>
 EOF
 
     def self.revisionOrDescription(string)

diff --git a/Websites/bugs.webkit.org/code-review-test.html b/Websites/bugs.webkit.org/code-review-test.html
index 8e74dc541dcf8c92cda88236861dbe99699c03c5..520fd955eecf552b1324d4161668c3d254eb9164 100644 (file)
--- a/Websites/bugs.webkit.org/code-review-test.html
+++ b/Websites/bugs.webkit.org/code-review-test.html
@@ -142,7 +142,7 @@ ls.localStorageStore = {
     'draft-comments-for-attachment-1': '{"born-on": 100, "comments":[]}',
     'draft-comments-for-attachment-2': '{"born-on": 100, "comments":[{"start_line_id": 1, "end_line_id": 2, "contents": "DUMMY CONTENTS"}, {"start_line_id": 3, "end_line_id": 4, "contents": "DUMMY CONTENTS 2"}]}'
 };
-var comments = new MockDraftCommentSaver('2', ls).saved_comments();
+var comments = new MockDraftCommentSaver('2', ls).saved_comments().comments;
 ASSERT_EQUAL(comments.length, 2);
 ASSERT_EQUAL(ls.log_string(), 'getItem:draft-comments-for-attachment-2');
 
@@ -150,14 +150,14 @@ var ls = new MockLocalStorage();
 ls.localStorageStore = {
     'draft-comments-for-attachment-1': 'corrupt comments'
 };
-var comments = new MockDraftCommentSaver('1', ls).saved_comments();
+var comments = new MockDraftCommentSaver('1', ls).saved_comments().comments;
 ASSERT_EQUAL(ls.log_string(), 'getItem:draft-comments-for-attachment-1\nremoveItem:draft-comments-for-attachment-1');
 
 var ls = new MockLocalStorage();
 ls.localStorageStore = {
     'draft-comments-for-attachment-1': '["also corrupt comments"]'
 };
-var comments = new MockDraftCommentSaver('1', ls).saved_comments();
+var comments = new MockDraftCommentSaver('1', ls).saved_comments().comments;
 ASSERT_EQUAL(ls.log_string(), 'getItem:draft-comments-for-attachment-1\nremoveItem:draft-comments-for-attachment-1');
 
 

diff --git a/Websites/bugs.webkit.org/code-review.js b/Websites/bugs.webkit.org/code-review.js
index dc68ad8428868ce6250f96fd004e607301df4708..8b035a3e2b48c0eaeaf2944d7071a8d6f5490f2e 100644 (file)
--- a/Websites/bugs.webkit.org/code-review.js
+++ b/Websites/bugs.webkit.org/code-review.js
@@ -152,9 +152,15 @@ var CODE_REVIEW_UNITTEST;
     g_displayed_draft_comments = true;
 
     var comments = g_draftCommentSaver.saved_comments();
-    $(comments).each(function() {
+    $(comments.comments).each(function() {
       addDraftComment(this.start_line_id, this.end_line_id, this.contents);
     });
+    
+    var overall_comments = comments['overall-comments'];
+    if (overall_comments) {
+      openOverallComments();
+      $('.overallComments textarea').val(overall_comments);
+    }
   }
 
   function DraftCommentSaver(opt_attachment_id, opt_localStorage) {
@@ -187,7 +193,8 @@ var CODE_REVIEW_UNITTEST;
       });
     });
 
-    return JSON.stringify({'born-on': Date.now(), 'comments': comment_store});
+    var overall_comments = $('.overallComments textarea').val().trim();
+    return JSON.stringify({'born-on': Date.now(), 'comments': comment_store, 'overall-comments': overall_comments});
   }
   
   DraftCommentSaver.prototype.saved_comments = function() {
@@ -195,21 +202,22 @@ var CODE_REVIEW_UNITTEST;
     if (!serialized_comments)
       return [];
 
-    var comments = [];
+    var comments = {};
     try {
-      comments = JSON.parse(serialized_comments).comments;
+      comments = JSON.parse(serialized_comments);
     } catch (e) {
       this._erase_corrupt_comments();
-      return [];
+      return {};
     }
     
-    if (comments && !comments.length)
+    var individual_comments = comments.comments;
+    if (comments && !individual_comments.length)
       return comments;
     
     // Sanity check comments are as expected.
-    if (!comments || !comments[0].contents) {
+    if (!comments || !individual_comments[0].contents) {
       this._erase_corrupt_comments();
-      return [];
+      return {};
     }
     
     return comments;
@@ -307,6 +315,17 @@ var CODE_REVIEW_UNITTEST;
   function saveDraftComments() {
     ensureDraftCommentsDisplayed();
     g_draftCommentSaver.save();
+    setAutoSaveStateIndicator('saved');
+  }
+
+  function setAutoSaveStateIndicator(state) {
+    var container = $('.autosave-state');
+    container.text(state);
+    
+    if (state == 'saving')
+      container.addClass(state);
+    else
+      container.removeClass('saving');
   }
 
   function createCommentFor(line) {
@@ -320,6 +339,7 @@ var CODE_REVIEW_UNITTEST;
     line.addClass('commentContext');
 
     var comment_block = $('<div class="comment"><textarea data-comment-for="' + line.attr('id') + '"></textarea><div class="actions"><button class="ok">OK</button><button class="discard">Discard</button></div></div>');
+    $('textarea', comment_block).bind('input', handleOverallCommentsInput);
     insertCommentFor(line, comment_block);
     return comment_block;
   }
@@ -976,6 +996,16 @@ var CODE_REVIEW_UNITTEST;
     $('#statusBubbleContainer').addClass('wrap');
   }
 
+  var g_overallCommentsInputTimer;
+
+  function handleOverallCommentsInput() {
+    setAutoSaveStateIndicator('saving');
+    // Save draft comments after we haven't received an input event in 1 second.
+    if (g_overallCommentsInputTimer)
+      clearTimeout(g_overallCommentsInputTimer);
+    g_overallCommentsInputTimer = setTimeout(saveDraftComments, 1000);
+  }
+
   function onBodyResize() {
     updateToolbarAnchorState();
   }
@@ -994,7 +1024,6 @@ var CODE_REVIEW_UNITTEST;
       '<a href="javascript:" class="side-by-side-link">side-by-side</a>';
   }
 
-
   $(document).ready(function() {
     crawlDiff();
     fetchHistory();
@@ -1016,9 +1045,11 @@ var CODE_REVIEW_UNITTEST;
               '<button id="post_comments">Publish</button> ' +
           '</span>' +
         '</div>' +
+        '<div class="autosave-state"></div>' +
         '</div>');
 
     $('.overallComments textarea').bind('click', openOverallComments);
+    $('.overallComments textarea').bind('input', handleOverallCommentsInput);
 
     $(document.body).prepend('<div id="comment_form" class="inactive"><div class="winter"></div><div class="lightbox"><iframe id="reviewform" src="attachment.cgi?id=' + attachment_id + '&action=reviewform"></iframe></div></div>');
     $('#reviewform').bind('load', handleReviewFormLoad);