launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/bmp-line-number-display into lp:launchpad.

Commit message:
Fix td.line-no display property when checking "Show line numbers".

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/bmp-line-number-display/+merge/262699

Unchecking and then checking "Show line numbers" on a merge proposal preview diff causes a huge gap to appear between the line numbers and the diff text.  A hint as to why can be found in the test for this feature, which inexplicably tests that this sequence of actions takes the display property from "table-cell" to "none" to "block".  If we send it back to "table-cell" instead, then everything works properly.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/bmp-line-number-display into lp:launchpad.

=== modified file 'lib/lp/code/javascript/branchmergeproposal.reviewcomment.js'
--- lib/lp/code/javascript/branchmergeproposal.reviewcomment.js	2014-04-03 20:33:08 +0000
+++ lib/lp/code/javascript/branchmergeproposal.reviewcomment.js	2015-06-23 12:57:07 +0000
@@ -257,7 +257,7 @@
 var update_nos = function(){
     var new_display = 'none';
     if (this.get('checked')) {
-        new_display = 'block';
+        new_display = 'table-cell';
     }
     Y.all('td.line-no').setStyle('display', new_display);
 };

=== modified file 'lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.js'
--- lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.js	2015-04-09 03:36:08 +0000
+++ lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.js	2015-06-23 12:57:07 +0000
@@ -420,7 +420,7 @@
             line_no_check.simulate('change');
             this.wait(function() {}, 1);
             Y.Assert.areEqual(
-                'block', Y.one('td.line-no').getStyle('display'));
+                'table-cell', Y.one('td.line-no').getStyle('display'));
         },
 
         test_diff_inline_show_hide: function() {