← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~cjwatson/launchpad/choose-icon into lp:launchpad

 

Colin Watson has proposed merging lp:~cjwatson/launchpad/choose-icon into lp:launchpad.

Commit message:
Show a search icon for pickers where possible rather than "Choose...".

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/choose-icon/+merge/327983

I always thought the green "Choose..." text looked rather ugly, and it gets particularly bad in a branch I'm working on where I have Git repository and branch input boxes displayed in a single row.  Requesting a picker is essentially a search operation, or at least the start of one, so I think it makes sense to use the search icon here.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/choose-icon into lp:launchpad.
=== modified file 'lib/lp/app/javascript/calendar.js'
--- lib/lp/app/javascript/calendar.js	2017-07-21 17:35:14 +0000
+++ lib/lp/app/javascript/calendar.js	2017-07-24 17:47:09 +0000
@@ -178,7 +178,11 @@
             }
         var parent_node = date_input.ancestor();
         var choose_link = Y.Node.create(
-            '<span>(<a class="js-action" href="#">Choose...</a>)</span>');
+            '<span>' +
+            '<a href="#" class="sprite search-icon action-icon js-action">' +
+            'Choose\u2026' +
+            '</a>' +
+            '</span>');
         parent_node.insertBefore(choose_link, date_input.next());
 
         // Add a class to flag that this date-input is setup.

=== modified file 'lib/lp/app/javascript/picker/picker_patcher.js'
--- lib/lp/app/javascript/picker/picker_patcher.js	2017-07-20 13:29:41 +0000
+++ lib/lp/app/javascript/picker/picker_patcher.js	2017-07-24 17:47:09 +0000
@@ -25,14 +25,13 @@
         return;
     }
     var picker_span = show_widget_node.get('parentNode');
-    var new_node = Y.Node.create('<span>(<a href="#"></a>)</span>');
-    show_widget_node = new_node.one('a');
-    show_widget_node
-        .set('id', show_widget_id)
-        .addClass('js-action')
-        .set('text', 'Choose\u2026');
+    show_widget_node = Y.Node.create(
+        '<a href="#" class="sprite search-icon action-icon js-action">' +
+        'Choose\u2026' +
+        '</a>');
+    show_widget_node.set('id', show_widget_id)
     picker_span.empty();
-    picker_span.appendChild(new_node);
+    picker_span.appendChild(show_widget_node);
     picker_span.removeClass('hidden');
     show_widget_node.on('click', function (e) {
         if (picker === null) {


Follow ups