launchpad-dev team mailing list archive

[Curtis Hovey <curtis@xxxxxxxxxx>]

In reply to Michael Hudson:
> > I have seen menus show up in oopses, I have seen engineers on IRC
> > surprised to see a query cause by a menu, but I have not seen an outline
> > of the menu problem. I think I can summarise the problem and provide
> > some guidelines about how we want menus to behave.
> 
> I think I'm lacking some context here, like what a menu is in the
> Launchpad context.  Let me guess a little: a Menu is a collection of
> Links.  There is one menu per instance of a LaunchpadView subclass.
> Links can be disabled or enabled.

There are several menu types. Menus were for context objects. In 2.0 we
added the NavigationMenu for context objects and views. The menu is
responsible for creating the URL of a link, checking permissions, and
enabling the link.

ContextMenu (context) A menu to render the links in the sidebar that are
appropriate for all facets. This is now used as a bag to create inline
links.
tales: ISourcePackageRecipeBuild/menu:rescore

ApplicationMenu (context) A menu for the context when viewed on a facet
(overview, bugs, branches, etc..). This was a list in the side bar, but
now used like a bag to render inline links.
tales: IPerson/menu:branches/active_reviews

NavigationMenu (context or view) A menu or a context or a view of a
context. It contains links that bind pages with a common theme together.
It may also have a submenu (a linking a menu on a context object to a
menu for view). We tend to use this now to render action links in the
side bar (context), or as group a related pages inline (view).
tales: IPerson/menu:navigation/assignedbugs
tales: PersonRelatedSoftwareView/menu:navigation/projects

^^^ There looks to be a lot over overlap between these menus type.

FacetMenu (context) The applications that are available to the context.
It is the tabs/app buttons in the header. There is one direct
implementer, StandardLaunchpadFacets, and each object gets a subclass to
define its links. I would say this works well if it were not for the
fact that the tooltips for the links are very inconsistent for objects. 

> The Menu itself can be rendered, and renders each link one after the
> other, but we don't ever do this these days.
> 
> >       * I recently saw Martin Pitt could not see his +participation page
> >         because of a timeout reached querying all his uploads. This is
> >         because a link in the person overview menu is only enabled if
> >         the person has uploads. The page does *not* use this link.
> 

> Because the menu instance is associated with the view instance,
> rendering a portlet, even if it's a view on the same view class, will
> create the menu over again?

They are associate via adaption of interfaces (Request + Context or
View) The <context|view>/menu: call always instantiates using the
current request. The path adapter always creates a new instance.

Many pages do this
    tal:define="overview_menu context/menu:overview;"
so that a single instance of the overview menu will be used in the
template. Each context/@@/+a-portlet needs to create its own instance of
the menu. 

> >       * Even though a link is defined in the overview menu (mainsite),
> >         and a link's view defines its facet, the menu makes the link URL
> >         to the current host eg. we had a 404 on code.lp.net for
> >         +downloads and we could not use the menu to get the correct
> >         link.
> 
> Doesn't the link just use morally the same code as fmt:link?  If not, it
> should?  If it does, this bug should be fixed there?
> 
> facet is an obsolete idea of course. s/facet/layer/g.

Yes, but the menu is still looking to __launchpad_facetname__ that was
hacked as a view attribute by our metazcml. Facet will be obsolete when
we remove the metazcml directives and remove the two uses in menu.py and
tales.py (the menu path adapter). 

...

> >       * The menus assume that they are instantiated once per request
> >         because of the previous point.
> 
> This can never have been really true.

It was in the 1.0 layout because menus were only shown in the sidebar.
We rendered them like a list. The list was composed of the context's
ContextMenu and its ApplicationMenu. 

Inline links were crafted in tales, just string concatenation with
little to no guarantee that link should be enabled, that the user has
permission, that the link goes to the correct host.

In 2.0 we decided to move the links into the page content, quietly
subverting the menu's list feature into a bag. We also added
NavigationMenus to manage lists for for a group of views. 

...

> > Guidelines for expected behaviour
> > 
> >       * We build menus once per request. Getting an object's menu is
> >         free once it is instantiated.
> 
> So this means menus are associated with context objects, not with view
> instances?  (Maybe this is already true).

Only the NavigationMenu can be used with a view, and in 3.0 it is
usually called once per request to render a list of links. This is no
abused since we still use it as it much like it was intended. 

> >       * We know that at the point we start rendering, that no link can
> >         change--once a link object is setup, we must never do it
> >         again.
> 
> Right.  In fact, getting the object's menu should be trivial -- it's
> only getting a link out of that menu that should be non-trivial, and we
> should cache the link in the request once its accessed.

I think so.

...

> What on earth is a facet menu?

See above.

-- 
__Curtis C. Hovey_________
http://launchpad.net/