launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #01676
[Merge] lp:~jcsackett/launchpad/hidden-project-configuration-636000 into lp:launchpad/devel
j.c.sackett has proposed merging lp:~jcsackett/launchpad/hidden-project-configuration-636000 into lp:launchpad/devel.
Requested reviews:
Launchpad UI Reviewers (launchpad-ui-reviewers)
Launchpad code reviewers (launchpad-reviewers): code
Summary
=======
As part of bridging-the-gap, a progress bar and other visual indicators were introduced to encourage users to fully configure their projects so we could get upstream data.
Now, when you finish configuring everything, you're still shown all the configuration links despite also being told that you're "done" configuring. To make this less visually irritating but still allow configuration to be changed easily on a fully configured product, this branch introduces a show/hide control around the configuration links. It defaults to showing if configuration isn't done, and hiding otherwise. It's largely borrowed from the show/hide of "Extra options" on other parts of Launchpad (e.g. merge proposals).
Proposed fix
============
Create show/hide control on the configuration links so you're not bothered with the links when you no longer need them, but can still access them.
Preimplementation talk
======================
Spoke with Curtis Hovey about branch goals. Spoke with Edwin Grubbs about setting up javascript in Launchpad and testing it.
Implementation details
======================
The product view has a new method that returns a boolean showing whether or not registration is done. This is used to set the classes on the block showing the configuration links.
A new javascript file, pillar.js, provides a way to set up div marked as collapsible in the same way collapsible fieldsets work in other places in the site. This uses the same toggle collapsible function to provide the actual functionality.
Tests
=====
bin/test -m lp.registry.browser.tests.test_product
bin/test -m lp.registry.windmill.tests.test_product_configuration_hidden
Screenshots
===========
The section shown: http://people.canonical.com/~jc/images/shown.png
The section hidden: http://people.canonical.com/~jc/images/hidden.png
QA
==
Open http://launchpad.dev/firefox. It should start with the configuration area shown, with two configuration steps left. Configure them both (it doesn't matter how, just make sure they're set up beyond a default UNKNOWN state).
Open http://launchpad.dev/firefox again. It should start with the configuration links hidden.
Lint
====
make lint output:
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/app/templates/base-layout-macros.pt
lib/lp/registry/browser/product.py
lib/lp/registry/browser/tests/test_product.py
lib/lp/registry/javascript/pillar.js
lib/lp/registry/javascript/team.js
lib/lp/registry/templates/pillar-involvement-portlet.pt
lib/lp/registry/templates/product-index.pt
lib/lp/registry/windmill/tests/test_product_configuration_hidden.py
lib/lp/testing/service_usage_helpers.py
./lib/lp/registry/javascript/team.js
137: Line exceeds 78 characters.
The line that exceeds 78 characters isn't part of the diff, and is arguably most readable at its current length.
--
https://code.launchpad.net/~jcsackett/launchpad/hidden-project-configuration-636000/+merge/39306
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/hidden-project-configuration-636000 into lp:launchpad/devel.
=== modified file 'lib/lp/app/templates/base-layout-macros.pt'
--- lib/lp/app/templates/base-layout-macros.pt 2010-10-04 12:23:40 +0000
+++ lib/lp/app/templates/base-layout-macros.pt 2010-10-25 20:03:50 +0000
@@ -189,6 +189,8 @@
<script type="text/javascript"
tal:attributes="src string:${lp_js}/registry/timeline.js"></script>
<script type="text/javascript"
+ tal:attributes="src string:${lp_js}/registry/pillar.js"></script>
+ <script type="text/javascript"
tal:attributes="src string:${lp_js}/sorttable/sorttable.js"></script>
<script type="text/javascript"
tal:attributes="src string:${lp_js}/inlinehelp/inlinehelp.js"></script>
=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py 2010-10-06 15:00:24 +0000
+++ lib/lp/registry/browser/product.py 2010-10-25 20:03:50 +0000
@@ -473,6 +473,11 @@
undone = scale - done
return dict(done=done, undone=undone)
+ @property
+ def registration_done(self):
+ """A boolean indicating that the services are fully configured."""
+ return (self.registration_completeness['done'] == 100)
+
class ProductNavigationMenu(NavigationMenu):
=== modified file 'lib/lp/registry/browser/tests/test_product.py'
--- lib/lp/registry/browser/tests/test_product.py 2010-09-27 18:18:11 +0000
+++ lib/lp/registry/browser/tests/test_product.py 2010-10-25 20:03:50 +0000
@@ -12,6 +12,7 @@
from zope.security.proxy import removeSecurityProxy
from canonical.testing.layers import DatabaseFunctionalLayer
+from lp.app.enums import ServiceUsage
from lp.registry.browser.product import ProductLicenseMixin
from lp.registry.interfaces.product import License
from lp.testing import (
@@ -19,6 +20,7 @@
TestCaseWithFactory,
)
from lp.testing.mail_helpers import pop_notifications
+from lp.testing.service_usage_helpers import set_service_usage
from lp.testing.views import create_view
@@ -102,5 +104,59 @@
self.assertEqual('2005-06-15', result)
+class TestProductConfiguration(TestCaseWithFactory):
+ """Tests the configuration links and helpers."""
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(TestProductConfiguration, self).setUp()
+ self.product = self.factory.makeProduct()
+
+ def _configure_all_services(self, product):
+ # Sets all the service usage enums for a product.
+ # This doesn't set them all to LAUNCHPAD because a service should
+ # be considered configured as long as they're not UNKNOWN.
+ naked_product = removeSecurityProxy(product)
+
+ #codehosting_usage
+ branch = self.factory.makeProductBranch(product=product)
+ product_series = self.factory.makeProductSeries(
+ product=product,
+ branch=branch)
+ naked_product.development_focus = product_series
+
+ #bug_tracking_usage
+ bugtracker = self.factory.makeBugTracker()
+ naked_product.bugtracker = bugtracker
+
+ #translations_usage
+ naked_product.translations_usage = ServiceUsage.LAUNCHPAD
+
+ #answers_usage
+ naked_product.answers_usage = ServiceUsage.EXTERNAL
+
+ def test_registration_not_done(self):
+ # The registration done property on the product index view
+ # tells you if all the configuration work is done, based on
+ # usage enums.
+
+ # At least one usage enum is unknown, so registration done is false.
+ self.assertEqual(
+ self.product.codehosting_usage,
+ ServiceUsage.UNKNOWN)
+ view = create_view(self.product, '+get-involved')
+ self.assertFalse(view.registration_done)
+
+ set_service_usage(
+ self.product.name,
+ codehosting_usage="EXTERNAL",
+ bug_tracking_usage="LAUNCHPAD",
+ answers_usage="EXTERNAL",
+ translations_usage="NOT_APPLICABLE")
+ view = create_view(self.product, '+get-involved')
+ self.assertTrue(view.registration_done)
+
+
def test_suite():
return unittest.TestLoader().loadTestsFromName(__name__)
=== added file 'lib/lp/registry/javascript/pillar.js'
--- lib/lp/registry/javascript/pillar.js 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/javascript/pillar.js 2010-10-25 20:03:50 +0000
@@ -0,0 +1,108 @@
+
+/** Copyright (c) 2010, Canonical Ltd. All rights reserved.
+ *
+ * Pillar configuration ui hiding.
+ *
+ * @module lp.registry.pillar
+ */
+
+YUI.add('lp.registry.pillar', function(Y) {
+
+var module = Y.namespace('lp.registry.pillar');
+
+module.activate_collapsible_div = function() {
+ // Grab the collapsibles.
+ Y.all('div.collapsible').each(function(collapsible) {
+
+ var legend = collapsible.one('span#legend');
+ if (legend === null) {
+ // If there's no span#legend there's not much we can do,
+ // so just exit this iteration.
+ return;
+ }
+
+ var icon = Y.Node.create(
+ '<img src="/@@/treeExpanded" class="collapseIcon" />');
+
+ // We use javascript:void(0) here (though it will cause
+ // lint to complain) because it prevents clicking on the
+ // anchor from altering the page URL, which can subtly
+ // break things.
+ var anchor = Y.Node.create(
+ '<a href="javascript:void(0);"></a>');
+ anchor.appendChild(icon);
+
+ // Move the contents of the legend into the span. We use
+ // the verbose version of <span /> to avoid silly
+ // breakages in Firefox.
+ var span = Y.Node.create('<span></span>');
+ var legend_children = legend.get('children');
+ var len;
+
+ // XXX 2009-07-06 gmb Account for oddness from
+ // Node.get('children'); (see YUI ticket 2528028 for
+ // details).
+ len = legend_children.size ?
+ legend_children.size() : legend_children.length;
+
+ span.set('innerHTML', legend.get('innerHTML'));
+ legend.set('innerHTML', '');
+
+ // Replace the contents of the legend with the anchor.
+ anchor.appendChild(span);
+ legend.appendChild(anchor);
+
+ // Put a wrapper around the fieldset contents for ease
+ // of hiding.
+ var wrapper_div = Y.Node.create(
+ '<div class="collapseWrapper" />');
+
+ // Loop over the children of the collapsible and move them
+ // into the wrapper div. We remove the legend from the
+ // collapsible at this point to make sure it gets left
+ // outside the wrapper div; we'll add it again later.
+ collapsible.removeChild(legend);
+
+ // "Why do this as a while?" I hear you cry. Well, it's
+ // because using Y.each() leads to interesting results
+ // in FF3.5, Opera and Chrome, since by doing
+ // appendChild() with each child node (and thus removing
+ // them from the collapsible) means you're altering the
+ // collection as you're looping over it, which is a Bad
+ // Thing. This isn't as pretty but it actually works.
+ var first_child = collapsible.one(':first-child');
+ while (Y.Lang.isValue(first_child)) {
+ wrapper_div.appendChild(first_child);
+ first_child = collapsible.one(':first-child');
+ }
+
+ // Put the legend and the new wrapper div into the
+ // collapsible in the right order.
+ collapsible.appendChild(legend);
+ collapsible.appendChild(wrapper_div);
+
+ // If the collapsible is to be collapsed on pageload, do
+ // so.
+ if (collapsible.hasClass('collapsed')) {
+ // Strip out the 'collapsed' class as it's no longer
+ // needed.
+ collapsible.removeClass('collapsed');
+ // We use the slide_in effect to hide the
+ // collapsible because it sets up all the properties
+ // and classes for the element properly and saves us
+ // from embarrasment later on.
+ var slide_in = Y.lazr.effects.slide_in(wrapper_div);
+ slide_in.run();
+
+ icon.set('src', '/@@/treeCollapsed');
+ }
+
+ // Finally, add toggle_collapsible() as an onclick
+ // handler to the anchor.
+ anchor.on('click', function(e) {
+ Y.lp.toggle_collapsible(collapsible);
+ });
+ });
+}
+}, '0.1', {requires: [
+ 'node', 'lazr.anim', 'lp']});
=== modified file 'lib/lp/registry/javascript/team.js'
--- lib/lp/registry/javascript/team.js 2010-07-15 10:59:22 +0000
+++ lib/lp/registry/javascript/team.js 2010-10-25 20:03:50 +0000
@@ -1,8 +1,8 @@
-/** Copyright (c) 2009, Canonical Ltd. All rights reserved.
- *
- * Objects for subscription handling.
- *
- * @module lp.subscriber
+/** Copyright (c) 2009-2010, Canonical Ltd. All rights reserved.
+ *
+ * Team add member animations and ui.
+ *
+ * @module lp.registry.team
*/
YUI.add('lp.registry.team', function(Y) {
=== modified file 'lib/lp/registry/templates/pillar-involvement-portlet.pt'
--- lib/lp/registry/templates/pillar-involvement-portlet.pt 2010-09-14 21:45:15 +0000
+++ lib/lp/registry/templates/pillar-involvement-portlet.pt 2010-10-25 20:03:50 +0000
@@ -37,9 +37,9 @@
<tal:show_configuration condition="registration">
<h2>Configuration Progress</h2>
<div class="centered" id="progressbar">
- <table width="80%" border="1">
- <tr>
- <td>
+ <table width="80%" border="1">
+ <tr>
+ <td>
<img
tal:attributes="alt string:${registration/done}% registration complete;
title string:${registration/done}% registration complete;
@@ -47,28 +47,36 @@
style string:width: ${registration/done}%;"
src="/@@/green-bar"
height="10"/>
- </td>
- </tr>
- </table>
+ </td>
+ </tr>
+ </table>
</div>
</tal:show_configuration>
- <table style="width: 100%; padding-top: 1em"
- tal:condition="view/configuration_links"
- id="configuration_links">
- <tal:item repeat="link_status view/configuration_links">
- <tr>
- <td>
- <a tal:replace="structure link_status/link/fmt:link" />
- </td>
-
- <td style="text-align: right;" tal:condition="registration">
- <tal:yes-no replace="structure link_status/configured/image:boolean" />
- </td>
-
- </tr>
- </tal:item>
- </table>
-
+ <div id="show-hide-configs"
+ tal:attributes="class
+ python: 'collapsible collapsed'
+ if view.registration_done
+ else 'collapsible'">
+ <span id="legend">Configuration links</span>
+ <br />
+ <table
+ style="width: 100%; padding-top: 1em"
+ id="configuration_links"
+ tal:condition="view/configuration_links">
+ <tal:item repeat="link_status view/configuration_links">
+ <tr>
+ <td>
+ <a tal:replace="structure link_status/link/fmt:link" />
+ </td>
+
+ <td style="text-align: right;" tal:condition="registration">
+ <tal:yes-no
+ replace="structure link_status/configured/image:boolean"/>
+ </td>
+ </tr>
+ </tal:item>
+ </table>
+ </div>
</tal:editor>
</div>
=== modified file 'lib/lp/registry/templates/product-index.pt'
--- lib/lp/registry/templates/product-index.pt 2010-09-30 16:54:32 +0000
+++ lib/lp/registry/templates/product-index.pt 2010-10-25 20:03:50 +0000
@@ -12,6 +12,39 @@
<tal:registrant replace="structure context/registrant/fmt:link" />
</tal:registering>
+<head>
+ <tal:head-epilogue metal:fill-slot="head_epilogue">
+ <style type="text/css">
+ div.collapsible {
+ border:medium none;
+ margin:0;
+ padding:16px 0 0;
+ }
+
+ div.collapsed {
+ display: None
+ }
+ </style>
+
+ <noscript>
+ <style type="text/css">
+ div.collapsible, div.collapsed {display: block;}
+ </style>
+ </noscript>
+
+ <script type="text/javascript"
+ tal:content="string:
+ YUI().use('lp.registry.pillar', function(Y) {
+ Y.on('load',
+ function(e) {
+ Y.lp.registry.pillar.activate_collapsible_div();
+ },
+ window);
+ });
+ "/>
+ </tal:head-epilogue>
+</head>
+
<body>
<tal:main metal:fill-slot="main"
define="overview_menu context/menu:overview">
=== added file 'lib/lp/registry/windmill/tests/test_product_configuration_hidden.py'
--- lib/lp/registry/windmill/tests/test_product_configuration_hidden.py 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/windmill/tests/test_product_configuration_hidden.py 2010-10-25 20:03:50 +0000
@@ -0,0 +1,90 @@
+# Copyright 2009 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+import unittest
+import transaction
+
+from canonical.launchpad.windmill.testing import (
+ constants,
+ lpuser,
+ )
+from lp.registry.windmill.testing import RegistryWindmillLayer
+from lp.testing import WindmillTestCase
+from lp.testing.service_usage_helpers import set_service_usage
+
+
+class TestProductHiddenConfiguration(WindmillTestCase):
+
+ layer = RegistryWindmillLayer
+ suite_name = "Product configuration links hidden"
+
+ def setUp(self):
+ super(TestProductHiddenConfiguration, self).setUp()
+ self.product = self.factory.makeProduct(name='hidden-configs')
+ transaction.commit()
+
+ def test_not_fully_configured_starts_shown(self):
+ """Test the Configuration links on a product.
+
+ This test ensures that, with Javascript enabled, the configuration
+ links start closed on a fully configured project, and show all
+ configuration links when opened.
+
+ Additionally, on a not fully configured project, it starts by showing
+ the links, and can be closed.
+ """
+ client = self.client
+
+ client.open(url=u'http://launchpad.dev:8085/hidden-configs')
+ client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
+ lpuser.FOO_BAR.ensure_login(client)
+ # We can only safe use this class selector in this test b/c there's
+ # only one collapsible element on this page.
+ client.asserts.assertNotProperty(
+ classname='collapseWrapper',
+ validator='className|lazr-closed')
+
+ # When the Show link is clicked when it's open, it closes it.
+ client.click(link=u"Configuration links")
+ client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
+ client.asserts.assertProperty(
+ classname='collapseWrapper',
+ validator='className|lazr-closed')
+
+ def test_configured_starts_collapsed(self):
+ """Test the Configuration links on a product.
+
+ This test ensures that, with Javascript enabled, the configuration
+ links start closed on a fully configured project, and show all
+ configuration links when opened.
+
+ Additionally, on a not fully configured project, it starts by showing
+ the links, and can be closed.
+ """
+ set_service_usage(
+ self.product.name,
+ codehosting_usage="EXTERNAL",
+ bug_tracking_usage="LAUNCHPAD",
+ answers_usage="EXTERNAL",
+ translations_usage="NOT_APPLICABLE")
+ transaction.commit()
+
+ client = self.client
+
+ client.open(url=u'http://launchpad.dev:8085/hidden-configs')
+ client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
+ lpuser.FOO_BAR.ensure_login(client)
+ client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
+ client.asserts.assertProperty(
+ classname='collapseWrapper',
+ validator='className|lazr-closed')
+
+ client.click(link=u"Configuration links")
+ client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
+ client.asserts.assertProperty(
+ classname='collapseWrapper',
+ validator='className|lazr-open')
+
+
+def test_suite():
+ return unittest.TestLoader().loadTestsFromName(__name__)
=== modified file 'lib/lp/testing/service_usage_helpers.py'
--- lib/lp/testing/service_usage_helpers.py 2010-10-06 20:20:46 +0000
+++ lib/lp/testing/service_usage_helpers.py 2010-10-25 20:03:50 +0000
@@ -4,11 +4,12 @@
"""Helper functions dealing with IServiceUsage."""
__metaclass__ = type
-import transaction
from zope.component import getUtility
from lp.app.enums import ServiceUsage
+from lp.code.enums import BranchType
from lp.registry.interfaces.pillar import IPillarNameSet
+from lp.registry.interfaces.product import IProduct
from lp.testing import (
login,
logout,
@@ -27,7 +28,28 @@
pillar.official_malone = (service_usage == ServiceUsage.LAUNCHPAD)
if service_usage == ServiceUsage.EXTERNAL:
pillar.bugtracker = factory.makeBugTracker()
+
+ # if we're setting codehosting on product things get trickier.
+ elif attr == 'codehosting_usage' and IProduct.providedBy(pillar):
+ if service_usage == ServiceUsage.LAUNCHPAD:
+ branch = factory.makeProductBranch(product=pillar)
+ product_series = factory.makeProductSeries(
+ product=pillar,
+ branch=branch)
+ pillar.development_focus = product_series
+ elif service_usage == ServiceUsage.EXTERNAL:
+ branch = factory.makeProductBranch(
+ product=pillar,
+ branch_type=BranchType.MIRRORED)
+ product_series = factory.makeProductSeries(
+ product=pillar,
+ branch=branch)
+ pillar.development_focus = product_series
+ elif service_usage == ServiceUsage.UNKNOWN:
+ branch = factory.makeProductBranch(product=pillar)
+ product_series = factory.makeProductSeries(
+ product=pillar)
+ pillar.development_focus = product_series
else:
setattr(pillar, attr, service_usage)
- #transaction.commit()
logout()
