← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~deryck/launchpad/listing-config-widget-integration into lp:launchpad

 

Deryck Hodge has proposed merging lp:~deryck/launchpad/listing-config-widget-integration into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #887646 in Launchpad itself: "Advanced search showing new bug sorting widget"
  https://bugs.launchpad.net/launchpad/+bug/887646
  Bug #888731 in Launchpad itself: "BugListingConfigUtil should be integrated into CustomBugListings work"
  https://bugs.launchpad.net/launchpad/+bug/888731

For more details, see:
https://code.launchpad.net/~deryck/launchpad/listing-config-widget-integration/+merge/82024

This branch integrates BugListingConfigUtil in new style bug listings.  This widget provides controls for turning fields on and off in bug listings.  It doesn't yet have a persistence mechanism, but that's known.  I'll fix that in a later branch.

The bulk of the new work is in:

lib/lp/bugs/templates/buglisting-default.pt

This is where I wired up the new widget.  While I was there, I also added an additional tal:condition to fix a bug where the orderby bar was showing on advanced search pages.

The following files were changed to sync field_visibility defaults in js code with those in browser code:

lib/lp/bugs/javascript/buglisting_utils.js
lib/lp/bugs/javascript/tests/test_buglisting_utils.js

I also fixed a bug in the widget where the form overlay would not hide when the form was submitted.  I added a new test for this, too.

The rest of the changes are CSS changes to get the new widget and listings looking more like the mockups.  It's not pixel perfect yet, but getting closer.  I also removed the link around bug number to match original mockups and per discussion with Huw and my team.

-- 
https://code.launchpad.net/~deryck/launchpad/listing-config-widget-integration/+merge/82024
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~deryck/launchpad/listing-config-widget-integration into lp:launchpad.
=== modified file 'lib/canonical/launchpad/icing/style-3-0.css'
--- lib/canonical/launchpad/icing/style-3-0.css	2011-11-02 21:19:13 +0000
+++ lib/canonical/launchpad/icing/style-3-0.css	2011-11-11 21:33:34 +0000
@@ -1021,7 +1021,31 @@
     text-decoration: underline;
     cursor: pointer;
     }
-
+.yui3-buglisting-config-util a {
+    position: relative;
+    top: 3px;
+    left: 4px;
+    }
+.yui3-baseconfigutil a {
+    cursor: pointer;
+    }
+.yui3-buglisting-config-util-overlay a.close-button {
+    visibility: inherit;
+    }
+.yui3-buglisting-config-util-overlay .buglisting-opts {
+    position: relative;
+    left: -110px;
+    }
+.yui3-buglisting-config-util-overlay .update-buglisting {
+    position: relative;
+    left: -240px;
+    }
+.yui3-buglisting-config-util-overlay .reset-buglisting {
+    position: relative;
+    top: 20px;
+    left: 75px;
+    cursor: pointer;
+    }
 .error.message, .warning.message, .informational.message {
     border: solid #666;
     border-width: 1px 2px 2px 1px;

=== modified file 'lib/canonical/launchpad/icing/style.css'
--- lib/canonical/launchpad/icing/style.css	2011-11-02 21:19:13 +0000
+++ lib/canonical/launchpad/icing/style.css	2011-11-11 21:33:34 +0000
@@ -603,6 +603,7 @@
 /* Rules for CustomBugListings bug search result lists. */
 div#client-listing {
     float: left;
+    color: #666;
     }
 div.buglisting-row {
     float: left;
@@ -631,14 +632,15 @@
     padding: 0 5px;
     border-radius: 2px;
     text-transform: uppercase;
-    line-height: 1.5em;
-    margin: 5px 50px 5px 0;
+    line-height: 1.25em;
+    margin: 5px 50px 2px 0;
     text-align: center;
     position: relative;
     left: 10px;
     }
 div#client-listing .buglisting-col2 {
-    color: #999;
+    position: relative;
+    top: 2px;
     }
 div#client-listing .bug-related-icons {
     float: right;

=== modified file 'lib/lp/bugs/javascript/buglisting_utils.js'
--- lib/lp/bugs/javascript/buglisting_utils.js	2011-11-10 19:08:16 +0000
+++ lib/lp/bugs/javascript/buglisting_utils.js	2011-11-11 21:33:34 +0000
@@ -39,13 +39,18 @@
      * Object to reference defaults for field_visibility.
      */
     BugListingConfigUtil.field_visibility_defaults = {
-        show_bugtarget: true,
-        show_bug_heat: true,
+        show_title: true,
         show_id: true,
         show_importance: true,
         show_status: true,
-        show_title: true,
-        show_milestone_name: false
+        show_bug_heat: true,
+        show_bugtarget: true,
+        show_age: false,
+        show_last_updated: false,
+        show_assignee: false,
+        show_reporter: false,
+        show_milestone_name: false,
+        show_tags: false
     };
 
     /**
@@ -53,14 +58,18 @@
      * form inputs.
      */
     BugListingConfigUtil.field_display_names = {
-        show_bugtarget: 'Bug target',
-        show_bug_heat: 'Bug heat',
+        show_title: 'Bug title',
         show_id: 'Bug number',
         show_importance: 'Importance',
         show_status: 'Status',
-        show_title: 'Bug title',
-        show_milestone_name: 'Milestone name'
-
+        show_bug_heat: 'Bug heat',
+        show_bugtarget: 'Package/Project/Series name',
+        show_age: 'Bug age',
+        show_last_updated: 'Date bug last updated',
+        show_assignee: 'Assignee',
+        show_reporter: 'Reporter',
+        show_milestone_name: 'Milestone',
+        show_tags: 'Bug tags'
     };
 
     BugListingConfigUtil.ATTRS = {
@@ -231,6 +240,7 @@
                 }
             }
             this.set(FIELD_VISIBILITY, fields);
+            this.get(FORM).hide();
         },
 
         /**
@@ -244,7 +254,7 @@
             var on_submit_callback = Y.bind(this.handleOverlaySubmit, this);
             util_overlay = new Y.lazr.FormOverlay({
                 align: 'left',
-                headerContent: '<h2>Items to display</h2>',
+                headerContent: '<h2>Visible information</h2>',
                 centered: true,
                 form_content: form_content,
                 form_submit_button: Y.Node.create(
@@ -253,10 +263,12 @@
                 ),
                 form_cancel_button: Y.Node.create(
                     '<button type="button" name="field.actions.cancel" ' +
-                    'class="lazr-neg lazr-btn" >Cancel</button>'
+                    'class="hidden" >Cancel</button>'
                 ),
                 form_submit_callback: on_submit_callback
             });
+            util_overlay.get(
+                'boundingBox').addClass(this.getClassName('overlay'));
             this.set(FORM, util_overlay);
             this.addListeners();
             util_overlay.render();

=== modified file 'lib/lp/bugs/javascript/tests/test_buglisting_utils.js'
--- lib/lp/bugs/javascript/tests/test_buglisting_utils.js	2011-11-10 19:22:55 +0000
+++ lib/lp/bugs/javascript/tests/test_buglisting_utils.js	2011-11-11 21:33:34 +0000
@@ -50,13 +50,18 @@
     test_default_field_visibility_config: function() {
         // The default field_visibility should exist in a new widget.
         var expected_config = {
-            show_bugtarget: true,
+            show_title: true,
+            show_id: true,
+            show_importance: true,
+            show_status: true,
             show_bug_heat: true,
-            show_id: true,
-            show_importance: true,
-            show_status: true,
-            show_title: true,
-            show_milestone_name: false
+            show_bugtarget: true,
+            show_age: false,
+            show_last_updated: false,
+            show_assignee: false,
+            show_reporter: false,
+            show_milestone_name: false,
+            show_tags: false
         };
         this.list_util = new Y.lp.buglisting_utils.BugListingConfigUtil();
         ObjectAssert.areEqual(
@@ -67,21 +72,27 @@
         // field_visibility can be changed at the call site.
         // Supplied fields will be merged with the defaults.
         var supplied_config = {
-            show_bugtarget: false,
-            show_bug_heat: false
+            show_bug_heat: false,
+            show_status: false,
         };
         var expected_config = {
-            show_bugtarget: false,
-            show_bug_heat: false,
+            show_title: true,
             show_id: true,
             show_importance: true,
-            show_status: true,
-            show_title: true,
-            show_milestone_name: false
+            show_status: false,
+            show_bug_heat: false,
+            show_bugtarget: true,
+            show_age: false,
+            show_last_updated: false,
+            show_assignee: false,
+            show_reporter: false,
+            show_milestone_name: false,
+            show_tags: false
         };
         this.list_util = new Y.lp.buglisting_utils.BugListingConfigUtil({
             field_visibility: supplied_config
         });
+        this.list_util.render();
         ObjectAssert.areEqual(
             expected_config, this.list_util.get('field_visibility'));
     },
@@ -99,13 +110,18 @@
         this.list_util = new Y.lp.buglisting_utils.BugListingConfigUtil();
         this.list_util.render();
         var expected_names = [
-            'show_bugtarget',
-            'show_bug_heat',
+            'show_title',
             'show_id',
             'show_importance',
             'show_status',
-            'show_title',
-            'show_milestone_name'
+            'show_bug_heat',
+            'show_bugtarget',
+            'show_age',
+            'show_last_updated',
+            'show_assignee',
+            'show_reporter',
+            'show_milestone_name',
+            'show_tags'
         ];
         var expected_checked = [
             true,
@@ -114,6 +130,11 @@
             true,
             true,
             true,
+            false,
+            false,
+            false,
+            false,
+            false,
             false
         ];
         var actual_inputs = this.getActualInputData();
@@ -125,7 +146,7 @@
         // The form checkboxes should also match the user supplied
         // config values.
         var field_visibility = {
-            show_bugtarget: false,
+            show_status: false,
             show_bug_heat: false
         };
         this.list_util = new Y.lp.buglisting_utils.BugListingConfigUtil({
@@ -133,21 +154,31 @@
         });
         this.list_util.render();
         var expected_names = [
-            'show_bugtarget',
-            'show_bug_heat',
+            'show_title',
             'show_id',
             'show_importance',
             'show_status',
-            'show_title',
-            'show_milestone_name'
+            'show_bug_heat',
+            'show_bugtarget',
+            'show_age',
+            'show_last_updated',
+            'show_assignee',
+            'show_reporter',
+            'show_milestone_name',
+            'show_tags'
         ];
         var expected_checked = [
-            false,
-            false,
-            true,
-            true,
-            true,
-            true,
+            true,
+            true,
+            true,
+            false,
+            false,
+            true,
+            false,
+            false,
+            false,
+            false,
+            false,
             false
         ];
         var actual_inputs = this.getActualInputData();
@@ -180,18 +211,37 @@
         var update = Y.one('.update-buglisting');
         update.simulate('click');
         var expected_config = {
-            show_bugtarget: false,
-            show_bug_heat: false,
+            show_title: true,
             show_id: true,
             show_importance: true,
             show_status: true,
-            show_title: true,
-            show_milestone_name: false
+            show_bug_heat: false,
+            show_bugtarget: false,
+            show_age: false,
+            show_last_updated: false,
+            show_assignee: false,
+            show_reporter: false,
+            show_milestone_name: false,
+            show_tags: false
         };
         var actual_config = this.list_util.get('field_visibility');
         ObjectAssert.areEqual(expected_config, actual_config);
     },
 
+    test_form_update_hides_overlay: function() {
+        // Updating the form overlay hides the overlay.
+        this.list_util = new Y.lp.buglisting_utils.BugListingConfigUtil();
+        this.list_util.render();
+        var config = Y.one('.config');
+        config.simulate('click');
+        var show_bugtarget = Y.one('.show_bugtarget');
+        show_bugtarget.simulate('click');
+        var update = Y.one('.update-buglisting');
+        update.simulate('click');
+        var overlay = this.list_util.get('form').get('boundingBox');
+        Assert.isTrue(overlay.hasClass('yui3-lazr-formoverlay-hidden'));
+    },
+
     test_update_config_fires_event: function() {
         // A custom event fires when the field_visibility config
         // is updated.
@@ -217,7 +267,7 @@
         // Clicking "reset to defaults" on the form returns
         // field_visibility to its default values.
         var field_visibility = {
-            show_bugtarget: false,
+            show_bugtarget: true,
             show_bug_heat: false
         };
         this.list_util = new Y.lp.buglisting_utils.BugListingConfigUtil({
@@ -243,7 +293,7 @@
     test_fields_visibility_form_reset_hides_overlay: function() {
         // Reseting to defaults should hide the form overlay.
         var field_visibility = {
-            show_bugtarget: false,
+            show_bugtarget: true,
             show_bug_heat: false
         };
         this.list_util = new Y.lp.buglisting_utils.BugListingConfigUtil({
@@ -268,18 +318,19 @@
             field_visibility: field_visibility
         });
         this.list_util.render();
-        // Poke at the form to reset defaults.
-        var config = Y.one('.config');
-        config.simulate('click');
-        Y.one('.reset-buglisting').simulate('click');
         var expected_names = [
-            'show_bugtarget',
-            'show_bug_heat',
+            'show_title',
             'show_id',
             'show_importance',
             'show_status',
-            'show_title',
-            'show_milestone_name'
+            'show_bug_heat',
+            'show_bugtarget',
+            'show_age',
+            'show_last_updated',
+            'show_assignee',
+            'show_reporter',
+            'show_milestone_name',
+            'show_tags'
         ];
         var expected_checked = [
             true,
@@ -288,8 +339,17 @@
             true,
             true,
             true,
+            false,
+            false,
+            false,
+            false,
+            false,
             false
         ];
+        // Poke at the form to reset defaults.
+        var config = Y.one('.config');
+        config.simulate('click');
+        Y.one('.reset-buglisting').simulate('click');
         var actual_inputs = this.getActualInputData();
         ArrayAssert.itemsAreSame(expected_names, actual_inputs[0]);
         ArrayAssert.itemsAreSame(expected_checked, actual_inputs[1]);

=== modified file 'lib/lp/bugs/templates/buglisting-default.pt'
--- lib/lp/bugs/templates/buglisting-default.pt	2011-11-02 18:28:23 +0000
+++ lib/lp/bugs/templates/buglisting-default.pt	2011-11-11 21:33:34 +0000
@@ -26,8 +26,12 @@
     });
   </script>
   <script type="text/javascript"
-    tal:condition="request/features/bugs.dynamic_bug_listings.enabled">
-    LPS.use('lp.bugs.buglisting', 'lp.ordering', function(Y) {
+    tal:define="
+      show_new_listings request/features/bugs.dynamic_bug_listings.enabled;
+      advanced_search view/shouldShowAdvancedForm"
+    tal:condition="python: show_new_listings and not advanced_search">
+    LPS.use('lp.bugs.buglisting', 'lp.ordering', 'lp.buglisting_utils',
+      function(Y) {
         Y.on('domready', function() {
             var navigator = Y.lp.bugs.buglisting.ListingNavigator.from_page();
             var orderby = new Y.lp.ordering.OrderByBar({
@@ -40,12 +44,21 @@
                     ['heat', 'Bug heat']
                 ],
                 active: 'importance',
-                sort_order: 'desc'
+                sort_order: 'desc',
+                config_slot: true
             });
             orderby.render();
             Y.on('orderbybar:sort', function(e) {
                 navigator.first_batch(e);
             });
+            var config_node = orderby.get('config_node');
+            var list_util = new Y.lp.buglisting_utils.BugListingConfigUtil({
+                srcNode: config_node
+            });
+            list_util.render();
+            Y.on('buglisting-config-util:fields-changed', function(e) {
+                navigator.change_fields(list_util.get('field_visibility'));
+            });
         });
     });
   </script>

=== modified file 'lib/lp/bugs/templates/buglisting.mustache'
--- lib/lp/bugs/templates/buglisting.mustache	2011-11-04 17:52:41 +0000
+++ lib/lp/bugs/templates/buglisting.mustache	2011-11-11 21:33:34 +0000
@@ -18,7 +18,7 @@
     <div class="buglisting-col2">
         <div class="buginfo">
             {{#show_id}}
-                <a class="bugnumber" href="{{bug_url}}">#{{id}}</a>
+                #{{id}}
             {{/show_id}}
             {{#show_title}}
                 <a href="{{bug_url}}" class="bugtitle">{{title}}</a>