launchpad-reviewers team mailing list archive

[Bayard 'kit' Randel <kit.randel@xxxxxxxxxxxxx>]

Bayard 'kit' Randel has proposed merging lp:~blr/launchpad/diff-code-select-bug-1483925 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~blr/launchpad/diff-code-select-bug-1483925/+merge/268017

This branch provides a simple .unselectable class, and applies it to both line-no and line-no-ss on preview diffs - a small usability improvement to facilitate copy/pasting diffs.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~blr/launchpad/diff-code-select-bug-1483925 into lp:launchpad.

=== modified file 'lib/canonical/launchpad/icing/style.css'
--- lib/canonical/launchpad/icing/style.css	2015-06-29 23:04:36 +0000
+++ lib/canonical/launchpad/icing/style.css	2015-08-13 23:15:46 +0000
@@ -35,6 +35,12 @@
     top: 0;
     }
 
+.unselectable {
+  -moz-user-select: none;
+  -webkit-user-select: none;
+  -ms-user-select: none;
+}
+
 /* === Fonts and colors === */
 
 .unavailable {

=== modified file 'lib/lp/app/browser/stringformatter.py'
--- lib/lp/app/browser/stringformatter.py	2015-07-21 09:04:01 +0000
+++ lib/lp/app/browser/stringformatter.py	2015-08-13 23:15:46 +0000
@@ -938,7 +938,7 @@
 
         for css_class, row, _, _, line in parse_diff(text):
             result.append('<tr id="diff-line-%s">' % row)
-            result.append('<td class="line-no">%s</td>' % row)
+            result.append('<td class="line-no unselectable">%s</td>' % row)
             result.append(
                 structured(
                     '<td class="%s">%s</td>', css_class, line).escapedtext)
@@ -953,7 +953,8 @@
         # work, but displaying it would be confusing since there are also
         # per-file line numbers.
         result.append(
-            '<td class="line-no" style="display: none">%s</td>' % row)
+            '<td class="line-no unselectable" '
+            'style="display: none">%s</td>' % row)
         result.extend(cells)
         result.append('</tr>')
 
@@ -968,11 +969,11 @@
             else:
                 added_diff_row, added_row, added_line = 0, '', ''
             cells = (
-                '<td class="ss-line-no">%s</td>' % removed_row,
+                '<td class="ss-line-no unselectable">%s</td>' % removed_row,
                 structured(
                     '<td class="diff-removed text">%s</td>',
                     removed_line).escapedtext,
-                '<td class="ss-line-no">%s</td>' % added_row,
+                '<td class="ss-line-no unselectable">%s</td>' % added_row,
                 structured(
                     '<td class="diff-added text">%s</td>',
                     added_line).escapedtext,
@@ -1022,10 +1023,10 @@
                     if line.startswith(' '):
                         line = line[1:]
                     cells = [
-                        '<td class="ss-line-no">%s</td>' % orig_row,
+                        '<td class="ss-line-no unselectable">%s</td>' % orig_row,
                         structured(
                             '<td class="text">%s</td>', line).escapedtext,
-                        '<td class="ss-line-no">%s</td>' % mod_row,
+                        '<td class="ss-line-no unselectable">%s</td>' % mod_row,
                         structured(
                             '<td class="text">%s</td>', line).escapedtext,
                         ]

=== modified file 'lib/lp/app/browser/tests/test_stringformatter.py'
--- lib/lp/app/browser/tests/test_stringformatter.py	2015-06-25 11:16:06 +0000
+++ lib/lp/app/browser/tests/test_stringformatter.py	2015-08-13 23:15:46 +0000
@@ -483,14 +483,15 @@
         # White space doesn't count as empty, and is formatted.
         self.assertEqual(
             '<table class="diff unidiff"><tr id="diff-line-1">'
-            '<td class="line-no">1</td><td class="text"> </td></tr></table>',
+            '<td class="line-no unselectable">1</td><td class="text"> '
+            '</td></tr></table>',
             FormattersAPI(' ').format_diff())
 
     def test_format_unicode(self):
         # Sometimes the strings contain unicode, those should work too.
         self.assertEqual(
             u'<table class="diff unidiff"><tr id="diff-line-1">'
-            u'<td class="line-no">1</td><td class="text">'
+            u'<td class="line-no unselectable">1</td><td class="text">'
             u'Unicode \u1010</td></tr></table>',
             FormattersAPI(u'Unicode \u1010').format_diff())
 
@@ -576,10 +577,10 @@
         # White space doesn't count as empty, and is formatted.
         self.assertEqual(
             '<table class="diff ssdiff"><tr id="diff-line-1">'
-            '<td class="line-no" style="display: none">1</td>'
-            '<td class="ss-line-no">0</td>'
+            '<td class="line-no unselectable" style="display: none">1</td>'
+            '<td class="ss-line-no unselectable">0</td>'
             '<td class="text"></td>'
-            '<td class="ss-line-no">0</td>'
+            '<td class="ss-line-no unselectable">0</td>'
             '<td class="text"></td>'
             '</tr></table>',
             FormattersAPI(' ').format_ssdiff())
@@ -588,10 +589,10 @@
         # Sometimes the strings contain unicode, those should work too.
         self.assertEqual(
             u'<table class="diff ssdiff"><tr id="diff-line-1">'
-            u'<td class="line-no" style="display: none">1</td>'
-            u'<td class="ss-line-no">0</td>'
+            u'<td class="line-no unselectable" style="display: none">1</td>'
+            u'<td class="ss-line-no unselectable">0</td>'
             u'<td class="text">Unicode \u1010</td>'
-            u'<td class="ss-line-no">0</td>'
+            u'<td class="ss-line-no unselectable">0</td>'
             u'<td class="text">Unicode \u1010</td>'
             u'</tr></table>',
             FormattersAPI(u'Unicode \u1010').format_ssdiff())