Hi Curtis,

This is an elegant fix with a nicely big payoff.

Just a couple of comments.

Gavin.


> === modified file 'lib/lp/registry/browser/configure.zcml'
> --- lib/lp/registry/browser/configure.zcml	2009-11-26 06:41:27 +0000
> +++ lib/lp/registry/browser/configure.zcml	2009-12-02 12:04:29 +0000
> @@ -1310,6 +1310,7 @@
>      <browser:menus
>          classes="
>              MilestoneContextMenu
> +            MilestoneInlineNavigationMenu
>              MilestoneOverviewMenu
>              MilestoneOverviewNavigationMenu"
>          module="lp.registry.browser.milestone"/>
>
> === modified file 'lib/lp/registry/browser/milestone.py'
> --- lib/lp/registry/browser/milestone.py	2009-11-24 21:27:17 +0000
> +++ lib/lp/registry/browser/milestone.py	2009-12-02 12:04:29 +0000
> @@ -11,6 +11,7 @@
>      'MilestoneContextMenu',
>      'MilestoneDeleteView',
>      'MilestoneEditView',
> +    'MilestoneInlineNavigationMenu',
>      'MilestoneNavigation',
>      'MilestoneOverviewNavigationMenu',
>      'MilestoneSetNavigation',
> @@ -22,6 +23,7 @@
>
>  from zope.component import getUtility
>  from zope.formlib import form
> +from zope.interface import implements, Interface
>  from zope.schema import Choice
>
>  from canonical.cachedproperty import cachedproperty
> @@ -131,11 +133,22 @@
>      links = ('create_release', )
>
>
> +class IMilestoneInline(Interface):
> +    """A marker interface for views that show a milestone inline."""
> +
> +
> +class MilestoneInlineNavigationMenu(NavigationMenu, MilestoneLinkMixin):
> +    """An inline navigation menus for milestone views."""

s/menus/menu

> +    usedfor = IMilestoneInline
> +    facet = 'overview'
> +    links = ('edit', )
> +
> +
>  class MilestoneView(LaunchpadView, ProductDownloadFileMixin):
>      """A View for listing milestones and releases."""
>      # XXX sinzui 2009-05-29 bug=381672: Extract the BugTaskListingItem rules
>      # to a mixin so that MilestoneView and others can use it.
> -
> +    implements(IMilestoneInline)
>      show_series_context = False
>
>      def __init__(self, context, request):
>
> === modified file 'lib/lp/registry/browser/tests/milestone-views.txt'
> --- lib/lp/registry/browser/tests/milestone-views.txt	2009-09-30 22:49:58 +0000
> +++ lib/lp/registry/browser/tests/milestone-views.txt	2009-12-02 12:04:29 +0000
> @@ -21,8 +21,8 @@
>  Milestone defines several menus.
>
>      >>> from lp.registry.browser.milestone import (
> -    ...     MilestoneContextMenu, MilestoneOverviewMenu,
> -    ...     MilestoneOverviewNavigationMenu)
> +    ...     MilestoneContextMenu, MilestoneInlineNavigationMenu,
> +    ...     MilestoneOverviewMenu, MilestoneOverviewNavigationMenu)
>      >>> from lp.testing.menu import check_menu_links
>
>      >>> check_menu_links(MilestoneContextMenu(milestone))
> @@ -31,6 +31,9 @@
>      True
>      >>> check_menu_links(MilestoneOverviewNavigationMenu(milestone))
>      True
> +    >>> check_menu_links(MilestoneInlineNavigationMenu(milestone))
> +    True
> +

Should there also be a test to show that the
MilestoneInlineNavigationMenu is chosen for MilestoneView, or is that
going too far?

>
>  The MilestoneView provides access to the milestone and to its release if
>  it has one.
>
> === modified file 'lib/lp/registry/templates/productseries-milestone-table-row.pt'
> --- lib/lp/registry/templates/productseries-milestone-table-row.pt	2009-11-03 19:04:53 +0000
> +++ lib/lp/registry/templates/productseries-milestone-table-row.pt	2009-12-02 12:04:29 +0000
> @@ -2,8 +2,9 @@
>    xmlns:tal="http://xml.zope.org/namespaces/tal"
>    xmlns:metal="http://xml.zope.org/namespaces/metal"
>    xmlns:i18n="http://xml.zope.org/namespaces/i18n"
> -  define="milestone_menu view/milestone/menu:navigation;
> -          milestone view/milestone">
> +  define="milestone_menu view/menu:navigation;
> +          milestone view/milestone;
> +          release view/release">
>    <tr>
>      <td>
>        <img src="/@@/milestone" alt="" />
> @@ -43,13 +44,12 @@
>      </td>
>      <td tal:condition="not: view/is_distroseries_milestone">
>        <span
> -        tal:define="release milestone/product_release"
>          tal:condition="release"
>          tal:attributes="title release/datereleased/fmt:datetime"
>          tal:content="release/datereleased/fmt:approximatedate">
>          2005-12-13
>        </span>
> -      <tal:no-release condition="not: milestone/product_release">
> +      <tal:no-release condition="not: release">
>          <em tal:condition="not: milestone/active">
>            <img src="/@@/info"/> This is an inactive milestone
>          </em>
>