Merge lp:~rvb/maas/reveal-bug into lp:~maas-committers/maas/trunk

[Raphaël Badin]

Fix computation of height in reveal.js (take margin and padding into account).

The reveal.js module was computing the height of the 'revealed' panel incorrectly, not taking into account the padding and the margin. This branch fixes that.

Here is how it looks with that fix: http://people.canonical.com/~rvb/new_com_scripts_view.png.

[Jeroen T. Vermeulen (jtv)]

The screenshot is a nice touch. I tried to find some helpful utility function or attribute in YUI to simplify this job, but the documentation was so terse it was just meaningless to me. Might be docHeight, for instance, but it might not. :(

One question: should this really include the margin? It seems a bit "greedy" to take that space too. Then again I guess it depends on the page.

review: Approve

[Raphaël Badin (rvb)]

Thanks for the review.

> The screenshot is a nice touch. I tried to find some helpful utility function
> or attribute in YUI to simplify this job, but the documentation was so terse
> it was just meaningless to me. Might be docHeight, for instance, but it might
> not. :(

Yeah, I had the same reaction but I did not find any shortcut, I even found some that was doing exactly what I'm doing here: adding all the heights.

> One question: should this really include the margin? It seems a bit "greedy"
> to take that space too. Then again I guess it depends on the page.

In this case we have to include the margin because the HTML we're dealing with is like this:
<span class="ahah">
   <pre class="content"></pre>
<span>
And what we're doing is that we set the height of '.ahah' based on the height (and now margin and padding to) of '.content'.

=== modified file 'src/maasserver/static/js/reveal.js'
--- src/maasserver/static/js/reveal.js	2012-12-04 01:43:12 +0000
+++ src/maasserver/static/js/reveal.js	2012-12-20 10:54:22 +0000
@@ -85,10 +85,18 @@
  * @method _create_slide_out
  */
 module._create_slide_out = function(node, publisher) {
+    var content_node = node.one('.content');
+    var height = parseInt(content_node.getStyle('height'));
+    var padding_top = parseInt(content_node.getStyle('paddingTop'));
+    var padding_bottom = parseInt(content_node.getStyle('paddingBottom'));
+    var margin_top = parseInt(content_node.getStyle('marginTop'));
+    var margin_bottom = parseInt(content_node.getStyle('marginBottom'));
+    var new_height = (
+	height + padding_top + padding_bottom + margin_top + margin_bottom);
     var anim = new Y.Anim({
         node: node,
         duration: 0.2,
-        to: {height: parseInt(node.one('.content').getStyle('height'))}
+        to: {height: new_height}
     });
     anim.on('end', function () {
         publisher.fire('revealed');

=== modified file 'src/maasserver/static/js/tests/test_reveal.js'
--- src/maasserver/static/js/tests/test_reveal.js	2012-12-02 16:44:19 +0000
+++ src/maasserver/static/js/tests/test_reveal.js	2012-12-20 10:54:22 +0000
@@ -24,6 +24,12 @@
     },
 
     test_slides_out: function() {
+        var original_height = (
+            parseInt(Y.one('.panel .content').getStyle('height')) +
+            parseInt(Y.one('.panel .content').getStyle('marginTop')) +
+            parseInt(Y.one('.panel .content').getStyle('marginBottom')) +
+            parseInt(Y.one('.panel .content').getStyle('paddingTop')) +
+            parseInt(Y.one('.panel .content').getStyle('paddingBottom')));
         Y.one('.panel').setStyle('height', '0');
         Y.one('.link').set('text', 'View log');
         Y.Assert.areEqual('View log', Y.one('.link').get('text'));
@@ -36,6 +42,11 @@
                     parseInt(Y.one('.panel').getStyle('height')) > 0, 
                     'The panel should be revealed'
                     );
+                Y.Assert.areEqual(
+                    original_height,
+                    parseInt(Y.one('.panel').getStyle('height')), 
+                    'The panel has not been resized properly'
+                    );
                 Y.Assert.areEqual('Hide log', Y.one('.link').get('text'));
             });
         });