Changeset 136587 in webkit

Timestamp:

Dec 4, 2012, 4:06:02 PM (12 years ago)

Author:

ojan@chromium.org

Message:

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:

Location:

trunk/Websites/bugs.webkit.org

Files:

• ChangeLog (modified) (1 diff)
• PrettyPatch/PrettyPatch.rb (modified) (5 diffs)
• code-review-test.html (modified) (1 diff)
• code-review.js (modified) (4 diffs)

trunk/Websites/bugs.webkit.org/ChangeLog

@@ +1 @@
+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>
 

trunk/Websites/bugs.webkit.org/PrettyPatch/PrettyPatch.rb

@@ -319 +319 @@
   height: 2em;
   max-width: 100%;
+  min-width: 200px;
 }
 
@@ -335 +336 @@
 }
 
-body {
-  margin-bottom: 40px;
-}
-
 #toolbar {
-  display: -webkit-box;
-  display: -moz-box;
+  display: -webkit-flex;
+  display: -moz-flex;
   padding: 3px;
   left: 0;
@@ -349 +346 @@
   font-family: sans-serif;
   position: fixed;
-}
-
-#toolbar.anchored {
   bottom: 0;
 }
@@ -452 +446 @@
 
 .overallComments {
-  -webkit-box-flex: 1;
-  -moz-box-flex: 1;
+  -webkit-flex: 1;
+  -moz-flex: 1;
   margin-right: 3px;
 }
@@ -479 +473 @@
   border: none;
   vertical-align: middle;
-}
-
-.pseudo_resize_event_iframe {
-  height: 10%;
-  width: 10%;
-  position: absolute;
-  top: -11%;
 }
 

trunk/Websites/bugs.webkit.org/code-review-test.html

@@ -51 +51 @@
     '<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', '');

trunk/Websites/bugs.webkit.org/code-review.js

@@ -1061 +1061 @@
   }
 
-  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>' +
@@ -1092 +1079 @@
             '<button id="post_comments">Publish</button> ' +
           '</span>' +
+          '<div class="clear_float"></div>' +
         '</div>' +
         '<div class="autosave-state"></div>' +
@@ -1098 +1086 @@
     $('.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';
   }
 
@@ -1131 +1124 @@
     $('#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();