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

> 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.

I think that we should always have a default Logo, and a plan to eventually show something meaningful there.

> 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.

I feel the third option is the best. Thank you so much for laying out my options so well :)

> 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.

I don't quite understand what you mean by "location 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.

Your instincts where right here. It's great as-is.

> 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

Again, the right thing to do.

> 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.

I see you still haven't moved the breadcrumbs to beneath the title. To be honest, I don't know what to do with the edge notice, but I feel it's no longer as important to know about in which instance you are. It could very well be something that appears next to your name "Michael Nelson (edge server) [logout].

Do you plan to land this change for only a few pages, without the breadcrumbs? This is what I see when I run it locally.

Thanks for jumping on this so quickly, you make it look easy :)