Code review comment for lp:~michael.nelson/launchpad/3-0-menu

Revision history for this message
Michael Nelson (michael.nelson) wrote :

= Summary =

This branch is a first implementation of Martin's new location-header +
navigation tabs (facets), the mockup of which can be seen at:

http://people.canonical.com/~michaeln/menu_3-0/0-LP_new_design_Bugs_v3.1-from-beuno.png

== Proposed fix ==

This branch removes the old facet tabs from base-layout.pt and adds in
their place a simple styled list via a macro.

Following are a few unresolved questions that I have regarding the design.

What happens for contexts that do not have an image:logo?
=========================================================
See source package screenshot at:
http://people.canonical.com/~michaeln/menu_3-0/2-DistroSourcepkg-with-disabled-tabs.png

IDistributionSourcePackage does not provide IHasLogo, so passes up to
the nearest object that does, which is the distribution.

Note: in this specific case we actually want to update
IDistributionSourcePackage to implement IHasLogo and use the logo of the
upstream project - if it has one.

Alignment of menu text when first tab is highlighted (Code)?
============================================================

The provided mockup has the text of the first tab aligned with the text
of the heading above, but does not specify what should be aligned when
the first tab is active (ie. still the text, or the border of the
highlight?).

As a last-minute decision, I've gone with aligning the text of the first
tab when it is not active, and the margin/highlight when it is, as shown
by the first two examples at:

http://people.canonical.com/~michaeln/menu_3-0/4-alignment-of-first-tab.png

The third example shows you the other alternative (aligning the text
even when the first tab is active). Let me know what you'd prefer.

The location-portlet now becomes the top-portlet
================================================

I'm assuming from the mockup that the location portlet will always be
the top portlet when it is present. That is, the first content below it
should be separated with a thin gray line (ie. a normal portlet).

If this is so, it'll mean updating the 3.0 design instructions as people
can simply add class="portlet" to their first main-area portlet.

Other unspecified design decisions
==================================

Hover behaviour
---------------
I've just used a lower-contrasting gray. See

http://people.canonical.com/~michaeln/menu_3-0/1-DistroSearch-with-hover.png

Another point (which I haven't got an image for, but you can see the
behaviour if you merge the branch) is hovering over the current selected
tab.

Disabled location tabs
----------------------
I've grayed them out as shown at:
 http://people.canonical.com/~michaeln/menu_3-0/2-DistroSourcepkg-with-disabled-tabs.png

edge/staging warning message
----------------------------
As I've only implemented the location header/nav, I have not moved it so
it will still appear under the current 'breadcrumb'-like structure at
the top of the page.

== Implementation details ==

== Tests ==

bin/test -vvt tests/base-layout.txt

== Demo and Q/A ==

= Launchpad lint =

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

Linting changed files:
  lib/canonical/launchpad/icing/style.css
  lib/lp/app/browser/tests/testfiles/main-side.pt
  lib/lp/app/browser/tests/testfiles/searchless.pt
  lib/lp/app/browser/tests/base-layout.txt
  lib/lp/app/templates/base-layout.pt
  lib/lp/app/templates/base-layout-macros.pt
  lib/lp/app/browser/tests/testfiles/main-only.pt
  lib/canonical/launchpad/icing/style-3-0.css
  lib/lp/registry/templates/distribution-search.pt

--
Michael

« Back to merge proposal