← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~gary/launchpad/bug-772754-1 into lp:launchpad/db-devel

 

Gary Poster has proposed merging lp:~gary/launchpad/bug-772754-1 into lp:launchpad/db-devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #772754 in Launchpad itself: "After better-bug-notification changes, list of bug subscribers is confusing"
  https://bugs.launchpad.net/launchpad/+bug/772754

For more details, see:
https://code.launchpad.net/~gary/launchpad/bug-772754-1/+merge/62185

This branch simply rearranges the bug subscription portlet to coincide with the basic plan for fixing bug 772754 (if you want to see the mockup, a somewhat old one that is on the right track is https://launchpadlibrarian.net/71552495/all-in-one.png).  Subsequent branches will change the interior of the subscription and subscriber portlet more significantly.

The stuff deleted from lib/lp/bugs/templates/bug-portlet-subscribers.pt is now in lib/lp/bugs/templates/bug-portlet-subscription.pt .

I touched the CSS files for two reasons.  Hopefully the comment in lib/canonical/launchpad/icing/style.css describes one sufficiently.  The other one was because I needed two levels of headers in the portlets, and we didn't support that, so I added a second level.  Having a slightly different visual characteristic for text in the portlets versus text in the main body seemed reasonable, and I was building on the idea, so I deleted the complaining comment in the portlet h2 css.

Lint is happy for the Python files.  It is not happy for the CSS files, because of properties it doesn't know about and because of some line length issues.  I did not touch the CSS changes because I believe that the properties are intended, and I'm paranoid that the line length issues might be there for a reason as well, and I'm being lazy (at least for the line length issues).  If you really want me to make the line length issues, they will be easy enough to do.

Thanks

Gary
-- 
https://code.launchpad.net/~gary/launchpad/bug-772754-1/+merge/62185
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gary/launchpad/bug-772754-1 into lp:launchpad/db-devel.
=== modified file 'lib/canonical/launchpad/icing/style-3-0.css'
--- lib/canonical/launchpad/icing/style-3-0.css	2011-05-11 18:05:10 +0000
+++ lib/canonical/launchpad/icing/style-3-0.css	2011-05-24 19:36:34 +0000
@@ -1366,9 +1366,13 @@
     background: #fbfbfb;
     }
 .side h2 {
-    font-size: 16px; /* BAD: the markup should be h3? */
+    font-size: 16px;
     line-height: 20px;
     }
+.side h3 {
+    font-size: 14px;
+    line-height: 18px;
+    }
 .side ul {
     background: #fbfbfb;
     }

=== modified file 'lib/canonical/launchpad/icing/style.css'
--- lib/canonical/launchpad/icing/style.css	2011-04-11 16:12:18 +0000
+++ lib/canonical/launchpad/icing/style.css	2011-05-24 19:36:34 +0000
@@ -550,6 +550,11 @@
   background-image: url(/@@/spinner) !important;
 }
 
+/* This lets us have a help link sprite and a normal link sprite on the same
+   row, which the .vertical .sprite rule in the 3.0 file does not allow. */
+div#mute-link-container a.sprite {
+  width: auto;
+}
 
 /* === Translations === */
 

=== modified file 'lib/lp/bugs/browser/configure.zcml'
--- lib/lp/bugs/browser/configure.zcml	2011-04-28 12:51:56 +0000
+++ lib/lp/bugs/browser/configure.zcml	2011-05-24 19:36:34 +0000
@@ -535,6 +535,12 @@
             for="lp.bugs.interfaces.bug.IBug"
             class="lp.bugs.browser.bug.BugView"
             permission="launchpad.View"
+            name="+portlet-subscription"
+            template="../templates/bug-portlet-subscription.pt"/>
+        <browser:page
+            for="lp.bugs.interfaces.bug.IBug"
+            class="lp.bugs.browser.bug.BugView"
+            permission="launchpad.View"
             name="+portlet-subscribers"
             template="../templates/bug-portlet-subscribers.pt"/>
         <browser:page

=== modified file 'lib/lp/bugs/browser/tests/test_bug_context_menu.py'
--- lib/lp/bugs/browser/tests/test_bug_context_menu.py	2011-05-17 12:00:48 +0000
+++ lib/lp/bugs/browser/tests/test_bug_context_menu.py	2011-05-24 19:36:34 +0000
@@ -79,6 +79,6 @@
                 request = LaunchpadTestRequest()
                 request.features = get_relevant_feature_controller()
                 view = create_initialized_view(
-                    self.bug, name="+portlet-subscribers", request=request)
+                    self.bug, name="+portlet-subscription", request=request)
                 html = view.render()
         self.assertTrue('class="sprite maybe mute-help"' in html)

=== modified file 'lib/lp/bugs/browser/tests/test_bug_views.py'
--- lib/lp/bugs/browser/tests/test_bug_views.py	2011-05-24 17:41:47 +0000
+++ lib/lp/bugs/browser/tests/test_bug_views.py	2011-05-24 19:36:34 +0000
@@ -97,14 +97,14 @@
             request = LaunchpadTestRequest()
             request.features = get_relevant_feature_controller()
             view = create_initialized_view(
-                self.bug, name="+portlet-subscribers", request=request)
+                self.bug, name="+portlet-subscription", request=request)
             html = view.render()
         self.assertTrue('menu-link-editsubscriptions' in html)
         self.assertTrue('/+subscriptions' in html)
 
     def test_edit_subscriptions_link_not_shown_when_feature_disabled(self):
         view = create_initialized_view(
-            self.bug, name="+portlet-subscribers")
+            self.bug, name="+portlet-subscription")
         html = view.render()
         self.assertTrue('menu-link-editsubscriptions' not in html)
         self.assertTrue('/+subscriptions' not in html)
@@ -115,7 +115,7 @@
         with person_logged_in(person):
             with FeatureFixture({self.feature_flag_1: None}):
                 view = create_initialized_view(
-                    self.bug, name="+portlet-subscribers")
+                    self.bug, name="+portlet-subscription")
                 self.assertFalse(view.user_should_see_mute_link)
                 html = view.render()
                 self.assertFalse('mute_subscription' in html)
@@ -136,7 +136,7 @@
                 self.target.addBugSubscription(person, person)
                 self.assertFalse(self.bug.isMuted(person))
                 view = create_initialized_view(
-                    self.bug, name="+portlet-subscribers")
+                    self.bug, name="+portlet-subscription")
                 self.assertTrue(view.user_should_see_mute_link,
                                 "User should see mute link.")
                 contents = view.render()
@@ -166,7 +166,7 @@
                     self.bug.personIsAlsoNotifiedSubscriber(
                         person), "Person should be a notified subscriber")
                 view = create_initialized_view(
-                    self.bug, name="+portlet-subscribers")
+                    self.bug, name="+portlet-subscription")
                 self.assertTrue(view.user_should_see_mute_link,
                                 "User should see mute link.")
                 contents = view.render()
@@ -193,7 +193,7 @@
                     self.bug.personIsAlsoNotifiedSubscriber(
                         person))
                 view = create_initialized_view(
-                    self.bug, name="+portlet-subscribers")
+                    self.bug, name="+portlet-subscription")
                 self.assertFalse(view.user_should_see_mute_link)
                 html = view.render()
                 self.assertTrue('mute_subscription' in html)
@@ -217,7 +217,7 @@
                         person))
                 self.assertTrue(self.bug.isMuted(person))
                 view = create_initialized_view(
-                    self.bug, name="+portlet-subscribers")
+                    self.bug, name="+portlet-subscription")
                 self.assertTrue(view.user_should_see_mute_link,
                                 "User should see mute link.")
                 contents = view.render()

=== modified file 'lib/lp/bugs/templates/bug-portlet-subscribers-content.pt'
--- lib/lp/bugs/templates/bug-portlet-subscribers-content.pt	2011-05-18 18:34:19 +0000
+++ lib/lp/bugs/templates/bug-portlet-subscribers-content.pt	2011-05-24 19:36:34 +0000
@@ -8,7 +8,7 @@
     direct_subscriptions view/sorted_direct_subscriptions;
   ">
   <div class="section" id="subscribers-direct">
-    <h2>Subscribers</h2>
+    <h3>Directly subscribed</h3>
       <div id="subscribers-links">
       <div
         tal:condition="direct_subscriptions"
@@ -64,7 +64,7 @@
       id="subscribers-indirect"
       class="Section"
     >
-      <h2>Also notified</h2>
+      <h3>Also notified</h3>
       <div
         tal:repeat="subscriber also_notified_subscribers"
         tal:content="structure subscriber/fmt:link"

=== modified file 'lib/lp/bugs/templates/bug-portlet-subscribers.pt'
--- lib/lp/bugs/templates/bug-portlet-subscribers.pt	2011-05-10 11:30:46 +0000
+++ lib/lp/bugs/templates/bug-portlet-subscribers.pt	2011-05-24 19:36:34 +0000
@@ -6,26 +6,9 @@
   id="portlet-subscribers"
   metal:define-macro="custom"
 >
-  <div class="section" tal:define="context_menu context/menu:context"
-       metal:define-slot="heading">
-    <div
-      tal:attributes="class view/current_user_subscription_class"
-      tal:content="structure context_menu/subscription/render" />
-    <div id="sub-unsub-spinner">Subscribing...</div>
+  <h2>Other bug subscribers</h2>
+  <div tal:define="context_menu context/menu:context">
     <div tal:content="structure context_menu/addsubscriber/render" />
-    <div tal:condition="request/features/malone.advanced-structural-subscriptions.enabled"
-        tal:content="structure context_menu/editsubscriptions/render" />
-    <tal:show-mute condition="
-        request/features/malone.advanced-subscriptions.enabled">
-      <div tal:attributes="class view/current_user_mute_class"
-           id="mute-link-container">
-        <span tal:replace="structure context_menu/mute_subscription/render"/>
-        <a target="help" class="sprite maybe mute-help"
-            href="/+help/subscription-mute.html"
-          >&nbsp;<span class="invisible-link">Mute help</span></a>
-        <div style="float: left" id="mute-unmute-spinner">Unmuting...</div>
-      </div>
-    </tal:show-mute>
   </div>
   <a id="subscribers-ids-link"
      tal:define="bug context/bug|context"
@@ -37,23 +20,4 @@
        style="text-align: center; display: none">
     <img src="/@@/spinner" />
   </div>
-  <script type="text/javascript">
-    LPS.use('io-base', 'node', 'lp.bugs.bugtask_index.portlets', function(Y) {
-        // Must be done inline here to ensure the load event fires.
-        // This is a work around for a YUI3 issue with event handling.
-        var subscription_link = Y.one('.menu-link-subscription');
-        var subscription_link_handler;
-        if (subscription_link) {
-            subscription_link_handler = subscription_link.on(
-                'click', function(e) { e.preventDefault(); });
-        }
-
-        Y.on('domready', function() {
-            if (Y.lp.bugs.bugtask_index.portlets) {
-                Y.lp.bugs.bugtask_index.portlets.load_subscribers_portlet(
-                    subscription_link, subscription_link_handler);
-            }
-        });
-    });
-  </script>
 </div>

=== added file 'lib/lp/bugs/templates/bug-portlet-subscription.pt'
--- lib/lp/bugs/templates/bug-portlet-subscription.pt	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/templates/bug-portlet-subscription.pt	2011-05-24 19:36:34 +0000
@@ -0,0 +1,49 @@
+<div
+  xmlns:tal="http://xml.zope.org/namespaces/tal";
+  xmlns:metal="http://xml.zope.org/namespaces/metal";
+  xmlns:i18n="http://xml.zope.org/namespaces/i18n";
+  class="portlet vertical"
+  id="portlet-subscription"
+  metal:define-macro="custom"
+>
+  <div class="section" tal:define="context_menu context/menu:context"
+       metal:define-slot="heading">
+    <div
+      tal:attributes="class view/current_user_subscription_class"
+      tal:content="structure context_menu/subscription/render" />
+    <div id="sub-unsub-spinner">Subscribing...</div>
+    <div tal:condition="request/features/malone.advanced-structural-subscriptions.enabled"
+        tal:content="structure context_menu/editsubscriptions/render" />
+    <tal:show-mute condition="
+        request/features/malone.advanced-subscriptions.enabled">
+      <div tal:attributes="class view/current_user_mute_class"
+           id="mute-link-container">
+        <span tal:replace="structure context_menu/mute_subscription/render"
+        />&nbsp;<a target="help" class="sprite maybe mute-help"
+            href="/+help/subscription-mute.html"
+          >&nbsp;<span class="invisible-link">Mute help</span></a>
+        <div style="float: left" id="mute-unmute-spinner">Unmuting...</div>
+      </div>
+    </tal:show-mute>
+  </div>
+  <script type="text/javascript">
+    LPS.use('io-base', 'node',
+            'lp.bugs.bugtask_index.portlets', function(Y) {
+        // Must be done inline here to ensure the load event fires.
+        // This is a work around for a YUI3 issue with event handling.
+        var subscription_link = Y.one('.menu-link-subscription');
+        var subscription_link_handler;
+        if (subscription_link) {
+            subscription_link_handler = subscription_link.on(
+                'click', function(e) { e.preventDefault(); });
+        }
+
+        Y.on('domready', function() {
+            if (Y.lp.bugs.bugtask_index.portlets) {
+                Y.lp.bugs.bugtask_index.portlets.load_subscribers_portlet(
+                    subscription_link, subscription_link_handler);
+            }
+        });
+    });
+  </script>
+</div>

=== modified file 'lib/lp/bugs/templates/bugtask-index.pt'
--- lib/lp/bugs/templates/bugtask-index.pt	2011-05-16 15:29:15 +0000
+++ lib/lp/bugs/templates/bugtask-index.pt	2011-05-24 19:36:34 +0000
@@ -80,6 +80,7 @@
       <div tal:replace="structure context/bug/@@+portlet-privacy" />
       <div tal:replace="structure context/bug/@@+portlet-actions" />
       <div tal:replace="structure context/bug/@@+portlet-duplicates" />
+      <div tal:replace="structure context/bug/@@+portlet-subscription" />
       <div tal:replace="structure context/bug/@@+portlet-subscribers" />
       <div tal:replace="structure context/bug/@@+portlet-questions" />
       <div tal:replace="structure context/bug/@@+portlet-specs" />