Merge lp:~dooferlad/launchpad/upcomingwork_show_incomplete_bp into lp:launchpad

[James Tunnicliffe]

The upcomingwork view shows blueprints that a team or individual is working on. Currently all blueprint work items start in the collapsed view, but we have had a request to have incomplete blueprints to be initially expanded.

This change adds an alt tag to the percentage complete image as well as a title, so it can be reliably found in the DOM and the information encoded in the alt tag used to find if a blueprint is complete or not. Of course, this has a pleasing side-effect of making the page more accessible.

[Graham Binns (gmb)]

Hi James,

As mentioned on IRC the other day, I think a better way to do this is to work out whether or not the expander should be open on the server rather than doing it all in the JS. I've created a branch for this solution here: lp:~gmb/launchpad/upcomingwork_show_incomplete_bp - feel free to merge it into your branch.

I'm not hugely familiar with work items, so I don't know how to do QA on my solution (I can't seem to make items appear in the +upcomingwork view on launchpad.dev, no matter what I try). Please let me know if my solution's not working.

We also need to find something to remove from the LP codebase if we're going to land this, or we need to get it blessed as feature work by lifeless or flacoste. I'll look into the possible options.

Cheers,

Graham

review: Needs Fixing

[James Tunnicliffe (dooferlad)]

OK, have tested locally and got back to just your fix, so I hope you like it!

review: Resubmit

[James Tunnicliffe (dooferlad)]

Just trying to QA with more complex page on a clean checkout and I see the expander sprite (arrow picture) isn't there. Will attempt to track down.

[James Tunnicliffe (dooferlad)]

Ah, seems to be class="expander expanded" is causing problems. Will chase down a solution.

[James Tunnicliffe (dooferlad)]

Humm, would be because the JavaScript that adds the expander arrow looks for class="expander", so it would need a second search, if that one failed, to find class="expander expanded" and then do something different. What I think should be done is, if possible, add the CSS attribute that (if memory serves) is how the expander class knows if it is expanded or not in the template.

[James Tunnicliffe (dooferlad)]

Got it, but not a solution. <tbody class="collapsible-body"> needs to be <tbody class="collapsible-body expanded"> for elements that start open, but due to the fact that the template must be valid XML I can't do something like this:

<tbody class="collapsible-body"
       tal:condition="not: container/has_incomplete_work">
<tbody class="collapsible-body expanded"
       tal:condition="container/has_incomplete_work">

Still hunting...

=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py	2012-05-07 16:43:13 +0000
+++ lib/lp/registry/browser/person.py	2012-05-18 17:08:19 +0000
@@ -4520,6 +4520,11 @@
         return '{0:.0f}'.format(
             100.0 * len(self.done_items) / len(self._items))
 
+    @property
+    def has_incomplete_work(self):
+        """Return True if there are incomplete work items."""
+        return len(self.done_items) < len(self._items)
+
     def append(self, item):
         self._items.append(item)
 

=== modified file 'lib/lp/registry/browser/tests/test_person_upcomingwork.py'
--- lib/lp/registry/browser/tests/test_person_upcomingwork.py	2012-04-05 13:49:54 +0000
+++ lib/lp/registry/browser/tests/test_person_upcomingwork.py	2012-05-18 17:08:19 +0000
@@ -173,6 +173,16 @@
         container.append(self.MockWorkItem(True))
         self.assertEqual('67', container.percent_done)
 
+    def test_has_incomplete_work(self):
+        # If there are incomplete work items,
+        # WorkItemContainer.has_incomplete_work will return True.
+        container = WorkItemContainer()
+        item = self.MockWorkItem(False)
+        container.append(item)
+        self.assertTrue(container.has_incomplete_work)
+        item.is_complete = True
+        self.assertFalse(container.has_incomplete_work)
+
 
 class TestPersonUpcomingWork(BrowserTestCase):
 

=== modified file 'lib/lp/registry/templates/person-upcomingwork.pt'
--- lib/lp/registry/templates/person-upcomingwork.pt	2012-04-05 13:45:31 +0000
+++ lib/lp/registry/templates/person-upcomingwork.pt	2012-05-18 17:08:19 +0000
@@ -108,7 +108,16 @@
           </td>
         </tr>
       </tbody>
-      <tbody class="collapsible-body">
+
+      <tal:conditional condition="container/has_incomplete_work">
+        <div tal:define="global classname string:expanded"/>
+      </tal:conditional>
+
+      <tal:conditional condition="not: container/has_incomplete_work">
+        <div tal:define="global classname string:"/>
+      </tal:conditional>
+
+      <tbody tal:attributes="class string:collapsible-body ${classname}">
         <tr tal:repeat="workitem container/items" class="padded">
           <td>
             <span tal:condition="not: container/spec|nothing"