Use sticky positioning for the code review toolbar
authorojan@chromium.org <ojan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 Dec 2012 00:06:02 +0000 (00:06 +0000)
committerojan@chromium.org <ojan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 Dec 2012 00:06:02 +0000 (00:06 +0000)
https://bugs.webkit.org/show_bug.cgi?id=104056

Reviewed by Adam Barth.

This simplifies the code and gives a nicer user-experience.
Also, while here, I fixed up the CSS to not have toolbar items
overlap when you make the window too small.

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

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

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

index 2c05f7e..edb84b6 100644 (file)
@@ -1,5 +1,20 @@
 2012-12-04  Ojan Vafai  <ojan@chromium.org>
 
+        Use sticky positioning for the code review toolbar
+        https://bugs.webkit.org/show_bug.cgi?id=104056
+
+        Reviewed by Adam Barth.
+
+        This simplifies the code and gives a nicer user-experience.
+        Also, while here, I fixed up the CSS to not have toolbar items
+        overlap when you make the window too small.
+
+        * PrettyPatch/PrettyPatch.rb:
+        * code-review-test.html:
+        * code-review.js:
+
+2012-12-04  Ojan Vafai  <ojan@chromium.org>
+
         Properly create the header links in the code review tool
         https://bugs.webkit.org/show_bug.cgi?id=104037
 
index 668f331..9b351b4 100644 (file)
@@ -318,6 +318,7 @@ h1 :hover {
 .overallComments textarea {
   height: 2em;
   max-width: 100%;
+  min-width: 200px;
 }
 
 .comment textarea, .overallComments textarea {
@@ -334,13 +335,9 @@ h1 :hover {
   display: block;
 }
 
-body {
-  margin-bottom: 40px;
-}
-
 #toolbar {
-  display: -webkit-box;
-  display: -moz-box;
+  display: -webkit-flex;
+  display: -moz-flex;
   padding: 3px;
   left: 0;
   right: 0;
@@ -348,9 +345,6 @@ body {
   background-color: #eee;
   font-family: sans-serif;
   position: fixed;
-}
-
-#toolbar.anchored {
   bottom: 0;
 }
 
@@ -451,8 +445,8 @@ body {
 }
 
 .overallComments {
-  -webkit-box-flex: 1;
-  -moz-box-flex: 1;
+  -webkit-flex: 1;
+  -moz-flex: 1;
   margin-right: 3px;
 }
 
@@ -480,13 +474,6 @@ div:focus {
   vertical-align: middle;
 }
 
-.pseudo_resize_event_iframe {
-  height: 10%;
-  width: 10%;
-  position: absolute;
-  top: -11%;
-}
-
 .revision {
   display: none;
 }
index 185d43f..24ac9a8 100644 (file)
@@ -50,8 +50,7 @@ function testTracLinks() {
   ASSERT_EQUAL($('<div>').append(links).html(),
     '<a target="_blank" href="http://trac.webkit.org/log/trunk/foo/bar.cpp/qux.h">header</a>' +
     '<a target="_blank" href="http://trac.webkit.org/browser/trunk/foo/bar.cpp/qux.mm?annotate=blame">annotate</a>' +
-    '<a target="_blank" href="http://trac.webkit.org/log/trunk/foo/bar.cpp/qux.mm">revision log</a>
-');
+    '<a target="_blank" href="http://trac.webkit.org/log/trunk/foo/bar.cpp/qux.mm">revision log</a>');
 
   var links = tracLinks('foo/bar.h', '');
   ASSERT_EQUAL($('<div>').append(links).html(),
index 09b1a63..8102c6b 100644 (file)
@@ -1060,19 +1060,6 @@ var CODE_REVIEW_UNITTEST;
     g_overallCommentsInputTimer = setTimeout(saveDraftComments, 1000);
   }
 
-  function onBodyResize() {
-    updateToolbarAnchorState();
-  }
-
-  function updateToolbarAnchorState() {
-    var toolbar = $('#toolbar');
-    // Unanchor the toolbar and then see if it's bottom is below the body's bottom.
-    toolbar.toggleClass('anchored', false);
-    var toolbar_bottom = toolbar.offset().top + toolbar.outerHeight();
-    var should_anchor = toolbar_bottom >= document.body.clientHeight;
-    toolbar.toggleClass('anchored', should_anchor);
-  }
-
   function diffLinksHtml() {
     return '<a href="javascript:" class="unify-link">unified</a>' +
       '<a href="javascript:" class="side-by-side-link">side-by-side</a>';
@@ -1091,12 +1078,18 @@ var CODE_REVIEW_UNITTEST;
             '<button id="preview_comments">Preview</button>' +
             '<button id="post_comments">Publish</button> ' +
           '</span>' +
+          '<div class="clear_float"></div>' +
         '</div>' +
         '<div class="autosave-state"></div>' +
         '</div>');
 
     $('.overallComments textarea').bind('click', openOverallComments);
     $('.overallComments textarea').bind('input', handleOverallCommentsInput);
+
+    var toolbar = $('#toolbar');
+    toolbar.css('position', '-webkit-sticky');
+    var supportsSticky = toolbar.css('position') == '-webkit-sticky';
+    document.body.style.marginBottom = supportsSticky ? 0 : '40px';
   }
 
   function handleDocumentReady() {
@@ -1130,14 +1123,6 @@ var CODE_REVIEW_UNITTEST;
     $(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);
 
-    // Create a dummy iframe and monitor resizes in it's contentWindow to detect when the top document's body changes size.
-    // FIXME: Should we setTimeout throttle these?
-    var resize_iframe = $('<iframe class="pseudo_resize_event_iframe"></iframe>');
-    $(document.body).append(resize_iframe);
-    // Handle the event on a timeout to avoid crashing Firefox.
-    $(resize_iframe[0].contentWindow).bind('resize', function() { setTimeout(onBodyResize, 0)});
-
-    updateToolbarAnchorState();
     loadDiffState();
     generateFileDiffResizeStyleElement();
   };