Merge lp:~julian-edwards/launchpad/mechanical-30-ui-changes-5 into lp:launchpad

[Julian Edwards]

[Julian Edwards (julian-edwards)]

= Summary =
Convert distroarchseries page index to 3.0

== Implementation details ==
The +search template 95% duplicated the +index template, so I deleted it and
moved the search functionality into the index page.

Talked to beuno about where to put the details portlet. Consensus was to have
it below the search form, but to disappear if search results were present.

== Tests ==
bin/test -vvt stories.soyuz

== Demo and Q/A ==
https://launchpad.dev/ubuntu/hoary/i386/

Please ignore this lint. Seeing the warning after I've typed this email out
is extremely irritating, so it'll be fixed after this MP appears.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/soyuz/templates/distroarchseries-index.pt
  lib/lp/soyuz/stories/soyuz/xx-distroarchseries.txt
  lib/lp/soyuz/browser/configure.zcml
  lib/lp/soyuz/browser/distroarchseries.py

== Pyflakes notices ==

lib/lp/soyuz/browser/distroarchseries.py
    15: 'provideAdapter' imported but unused
    26: 'INavigationMenu' imported but unused
    27: 'ContextMenu' imported but unused

== Pylint notices ==

lib/lp/soyuz/browser/distroarchseries.py
    26: [W0611] Unused import INavigationMenu
    15: [W0611] Unused import provideAdapter
    27: [W0611] Unused import ContextMenu

[Barry Warsaw (barry)]

 review approve code ui
 status approve

On Aug 21, 2009, at 04:05 PM, Julian Edwards wrote:

>https://launchpad.dev/ubuntu/hoary/i386/

Hi Julian,

As mentioned on IRC, I have a few ui comments. My UI review must be seconded
by a ui mentor. Here are the things I noticed:

 * Wrapping the "1" in "This archive contains 1 software packages" in a
   <bold class="registry-stats"> for consistency
 * Removing the info icon and "Show" prefix from the navmenu. However,
   because it is too expensive at this late date to retrofit all the already
   converted pages, we've decided to leave this the way it is.
 * The +builds page batch navigation shouldn't show the navbar when there is
   only one page of results, but this is out-of-scope for this branch
 * The placement of the "This archive contains" text to above the search box
   instead of below it, in normal narrative text

Other than these small issues, the ui looks great. I have only minor comments
about the code, so r=me, ui=me. merge-conditional

=== modified file 'lib/lp/soyuz/browser/configure.zcml'
--- lib/lp/soyuz/browser/configure.zcml 2009-08-19 02:58:06 +0000
+++ lib/lp/soyuz/browser/configure.zcml 2009-08-21 15:29:49 +0000
> @@ -591,7 +591,7 @@
> <browser:menus
> module="lp.soyuz.browser.distroarchseries"
> classes="
> - DistroArchSeriesContextMenu"/>
> + DistroArchSeriesActionMenu"/>

When I was making similar changes, I ended up putting the "/> on a separate
line underneath the first letter of the menu name. It made it much easier to
sort the lines alphabetically later. You only have one class in this case, so
I'll leave it up to you, but I just wanted to share that :).

> <browser:url
> for="lp.soyuz.interfaces.distroarchseries.IDistroArchSeries"
> path_expression="architecturetag"

=== modified file 'lib/lp/soyuz/browser/distroarchseries.py'
--- lib/lp/soyuz/browser/distroarchseries.py 2009-08-18 18:17:25 +0000
+++ lib/lp/soyuz/browser/distroarchseries.py 2009-08-21 15:29:49 +0000
> @@ -4,14 +4,17 @@
> __metaclass__ = type
>
> __all__ = [
> + 'DistroArchSeriesActionMenu',
> 'DistroArchSeriesAddView',
> 'DistroArchSeriesAdminView',
> 'DistroArchSeriesPackageSearchView',
> - 'DistroArchSeriesContextMenu',
> 'DistroArchSeriesNavigation',
> 'DistroArchSeriesView',
> ]
>
> +from zope.component import provideAdapter
> +from zope.interface import implements, Interface
> +
> from canonical.launchpad import _
> from lp.soyuz.browser.build import BuildRecordsView
> from canonical.launchpad.browser.packagesearch import PackageSearchViewBase
> @@ -20,8 +23,9 @@
> GetitemNavigation, LaunchpadEditFormView)
> from canonical.launchpad.webapp.launchpadform import (
> action, LaunchpadFormView)
> +from canonical.launchpad.webapp.interfaces import INavigationMenu
> from canonical.launchpad.webapp.menu import (
> - ContextMenu, enabled_with_permission, Link)
> + ContextMenu, enabled_with_permission, Link, NavigationMenu)
> from canonical.launchpad.webapp.publisher import canonical_url
> from canonical.lazr.utils import smartquote
>
...

review: Approve (code ui)

=== modified file 'lib/lp/soyuz/browser/configure.zcml'
--- lib/lp/soyuz/browser/configure.zcml	2009-08-19 02:58:06 +0000
+++ lib/lp/soyuz/browser/configure.zcml	2009-08-21 15:29:49 +0000
@@ -591,7 +591,7 @@
         <browser:menus
             module="lp.soyuz.browser.distroarchseries"
             classes="
-                DistroArchSeriesContextMenu"/>
+                DistroArchSeriesActionMenu"/>
         <browser:url
             for="lp.soyuz.interfaces.distroarchseries.IDistroArchSeries"
             path_expression="architecturetag"
@@ -614,22 +614,19 @@
                 name="+builds"
                 facet="overview"
                 template="../templates/distroarchseries-builds.pt"/>
-            <browser:page
-                name="+builds-list"
-                template="../templates/builds-list.pt"/>
         </browser:pages>
         <browser:page
+            permission="zope.Public"
+            for="lp.soyuz.interfaces.distroarchseries.IDistroArchSeries"
+            class="lp.soyuz.browser.distroarchseries.DistroArchSeriesBuildsView"
+            name="+builds-list"
+            template="../templates/builds-list.pt"/>
+        <browser:page
             for="lp.soyuz.interfaces.distroarchseries.IDistroArchSeries"
             name="+macros"
             permission="zope.Public"
             template="../../registry/templates/packagesearch-macros.pt"/>
         <browser:page
-            for="lp.soyuz.interfaces.distroarchseries.IDistroArchSeries"
-            permission="zope.Public"
-            name="+search"
-            class="lp.soyuz.browser.distroarchseries.DistroArchSeriesPackageSearchView"
-            template="../templates/distroarchseries-search.pt"/>
-        <browser:page
             name="+admin"
             for="lp.soyuz.interfaces.distroarchseries.IDistroArchSeries"
             class="lp.soyuz.browser.distroarchseries.DistroArchSeriesAdminView"

=== modified file 'lib/lp/soyuz/browser/distroarchseries.py'
--- lib/lp/soyuz/browser/distroarchseries.py	2009-08-18 18:17:25 +0000
+++ lib/lp/soyuz/browser/distroarchseries.py	2009-08-21 15:29:49 +0000
@@ -4,14 +4,17 @@
 __metaclass__ = type
 
 __all__ = [
+    'DistroArchSeriesActionMenu',
     'DistroArchSeriesAddView',
     'DistroArchSeriesAdminView',
     'DistroArchSeriesPackageSearchView',
-    'DistroArchSeriesContextMenu',
     'DistroArchSeriesNavigation',
     'DistroArchSeriesView',
     ]
 
+from zope.component import provideAdapter
+from zope.interface import implements, Interface
+
 from canonical.launchpad import _
 from lp.soyuz.browser.build import BuildRecordsView
 from canonical.launchpad.browser.packagesearch import PackageSearchViewBase
@@ -20,8 +23,9 @@
     GetitemNavigation, LaunchpadEditFormView)
 from canonical.launchpad.webapp.launchpadform import (
     action, LaunchpadFormView)
+from canonical.launchpad.webapp.interfaces import INavigationMenu
 from canonical.launchpad.webapp.menu import (
-    ContextMenu, enabled_with_permission, Link)
+    ContextMenu, enabled_with_permission, Link, NavigationMenu)
 from canonical.launchpad.webapp.publisher import canonical_url
 from canonical.lazr.utils import smartquote
 
@@ -31,9 +35,15 @@
     usedfor = IDistroArchSeries
 
 
-class DistroArchSeriesContextMenu(ContextMenu):
-
-    usedfor = IDistroArchSeries
+class IDistroArchSeriesActionMenu(Interface):
+    """Marker interface for the action menu."""
+
+
+class DistroArchSeriesActionMenu(NavigationMenu):
+    """Action menu for distro arch series."""
+    usedfor = IDistroArchSeriesActionMenu
+    facet = "overview"
+    title = "Actions"
     links = ['admin', 'builds']
 
     @enabled_with_permission('launchpad.Admin')
@@ -49,10 +59,6 @@
         return Link('+builds', text, icon='info')
 
 
-class DistroArchSeriesView(BuildRecordsView):
-    """Default DistroArchSeries view class."""
-
-
 class DistroArchSeriesPackageSearchView(PackageSearchViewBase):
     """Customised PackageSearchView for DistroArchSeries"""
 
@@ -61,6 +67,16 @@
         return self.context.searchBinaryPackages(self.text)
 
 
+class DistroArchSeriesView(BuildRecordsView,
+                           DistroArchSeriesPackageSearchView):
+    """Default DistroArchSeries view class."""
+    implements(IDistroArchSeriesActionMenu)
+
+
+class DistroArchSeriesBuildsView(BuildRecordsView):
+    """View for +builds on a distro arch series."""
+
+
 class DistroArchSeriesAddView(LaunchpadFormView):
 
     schema = IDistroArchSeries

=== modified file 'lib/lp/soyuz/stories/soyuz/xx-distroarchseries.txt'
--- lib/lp/soyuz/stories/soyuz/xx-distroarchseries.txt	2009-08-17 13:10:51 +0000
+++ lib/lp/soyuz/stories/soyuz/xx-distroarchseries.txt	2009-08-21 15:48:52 +0000
@@ -25,6 +25,11 @@
     Ubuntu Warty for i386
     Search binary packages
     This archive contains 5 software packages.
+    Details for Ubuntu Warty i386
+    Architecture tag: i386
+    Processor family: Intel 386 compatible chips
+    Port registrant: Mark Shuttleworth
+    5 binary packages
 
 
 == Searching BinaryPackages ==
@@ -38,7 +43,7 @@
     >>> anon_browser.getControl(name="text").value = "firefox"
     >>> anon_browser.getControl("Search Packages").click()
     >>> anon_browser.url
-    'http://launchpad.dev/ubuntu/warty/i386/+search?text=firefox'
+    'http://launchpad.dev/ubuntu/warty/i386/+index?text=firefox'
 
 Searching for "firefox" finds several binary packages. Each search
 result is displayed as the binary package name followed by the binary
@@ -68,15 +73,16 @@
 All users can browse to the builds page from the DistroArchSeries
 page. The builds page is described in 23-builds-page.txt.
 
-    >>> print extract_text(find_tag_by_id(anon_browser.contents, 'actions'))
-    Administer (disabled)
+    >>> print extract_text(
+    ...     find_tag_by_id(anon_browser.contents, 'global-actions'))
     Show builds
 
 Only administrators can edit ('administer', in fact) the
 distroarchseries details.
 
     >>> admin_browser.open("http://launchpad.dev/ubuntu/warty/i386/")
-    >>> print extract_text(find_tag_by_id(admin_browser.contents, 'actions'))
+    >>> print extract_text(
+    ...     find_tag_by_id(admin_browser.contents, 'global-actions'))
     Administer
     Show builds
 

=== modified file 'lib/lp/soyuz/templates/distroarchseries-index.pt'
--- lib/lp/soyuz/templates/distroarchseries-index.pt	2009-07-17 17:59:07 +0000
+++ lib/lp/soyuz/templates/distroarchseries-index.pt	2009-08-21 15:29:49 +0000
@@ -3,19 +3,13 @@
   xmlns:tal="http://xml.zope.org/namespaces/tal"
   xmlns:metal="http://xml.zope.org/namespaces/metal"
   xmlns:i18n="http://xml.zope.org/namespaces/i18n"
-  xml:lang="en"
-  lang="en"
-  dir="ltr"
-  metal:use-macro="context/@@main_template/master"
+  metal:use-macro="view/macro:page/main_side"
   i18n:domain="launchpad"
 >
   <body>
 
-<metal:portlets fill-slot="portlets">
-  <div tal:replace="structure context/@@+portlet-details" />
-</metal:portlets>
 
-<div metal:fill-slot="main"
+<div metal:fill-slot="heading"
      tal:define="distroseries context/distroseries;
                  distro distroseries/distribution">
 
@@ -25,26 +19,42 @@
       <tal:series replace="distroseries/displayname">3.1</tal:series> for
       <tal:arch replace="context/architecturetag">i386</tal:arch>
   </h1>
-
-  <h2>Search binary packages</h2>
-
-  <form name="search" action="+search" method="GET">
-
-    <input type="text"
-      name="text"
-      size="35"
-      tal:attributes="value request/text|nothing" />
-
-    <input type="submit" value="Search Packages"/>
-
-  </form>
-
-  <p class="discreet">
-    This archive contains
-    <tal:count replace="context/package_count">bazillions of</tal:count> software packages.
-  </p>
-
-</div>
+</div>
+
+<div metal:fill-slot="side">
+  <tal:menu replace="structure view/@@+global-actions" />
+</div>
+
+<div metal:fill-slot="main">
+
+    <div class="top-portlet">
+      <h2>Search binary packages</h2>
+
+      <form name="search" method="GET">
+
+        <input type="text"
+          name="text"
+          size="35"
+          tal:attributes="value request/text|nothing" />
+
+        <input type="submit" value="Search Packages"/>
+
+      </form>
+
+      <p class="discreet">
+        This archive contains
+        <tal:count replace="context/package_count">bazillions of</tal:count> software packages.
+      </p>
+    </div><!--portlet-->
+
+    <metal:package-search use-macro="context/@@+macros/search-results" />
+
+    <tal:details condition="not: view/search_requested"
+                 replace="structure context/@@+portlet-details" />
+
+
+
+</div><!--metal-->
 
 </body>
 </html>

=== removed file 'lib/lp/soyuz/templates/distroarchseries-search.pt'
--- lib/lp/soyuz/templates/distroarchseries-search.pt	2009-07-17 17:59:07 +0000
+++ lib/lp/soyuz/templates/distroarchseries-search.pt	1970-01-01 00:00:00 +0000
@@ -1,48 +0,0 @@
-<html
-  xmlns="http://www.w3.org/1999/xhtml"
-  xmlns:tal="http://xml.zope.org/namespaces/tal"
-  xmlns:metal="http://xml.zope.org/namespaces/metal"
-  xmlns:i18n="http://xml.zope.org/namespaces/i18n"
-  xml:lang="en"
-  lang="en"
-  dir="ltr"
-  metal:use-macro="context/@@main_template/master"
-  i18n:domain="launchpad"
->
-  <body>
-    <metal:portlets fill-slot="portlets">
-      <div tal:replace="structure context/@@+portlet-details" /> 
-    </metal:portlets>
-
-<div metal:fill-slot="main">
-  <h1>Binary package search results</h1>
-
-  <form name="search" method="GET">
-    <div>
-      <div>
-        <label for="name">Find binary packages in
-        <span tal:replace="context/title" />
-        matching:
-        </label>
-      </div>
-      <div>
-      <input type="text"
-        name="text"
-        size="35"
-        tal:attributes="value request/text|nothing"
-      />
-      <input type="submit" value="Search"/>
-      </div>
-      <div class="discreet">
-        Includes package names and descriptions.
-      </div>
-    </div>
-  </form>
-
-  <metal:package-search use-macro="context/@@+macros/search-results" />
-
-</div>
-
-</body>
-
-</html>