Code review comment for lp:~allenap/launchpad/ui-convert-bug-tracker-3.0-bug-418155

> On Tue, 01 Sep 2009 11:26:08 -0000
> Michael Nelson <email address hidden> wrote:
>
> > Review: Needs Fixing ui
> > > To have a play, try:
> > >
> > > * Go to the bug trackers page (for IBugTrackerSet) at
> > > <https://bugs.launchpad.dev/bugs/bugtrackers>,
> >
> > Looks great! The only thing is (as bac pointed out) you've got two
> > "+ Register another bug tracker" links? I'd get rid of the one at
> > the top (assuming that people need to read through the list *before*
> > they can be sure that the one they're about to register isn't
> > already registered).
>
> Done.
>
> There are a lot of bug trackers registered in production, so this
> table gets very long. I haven't added a batch navigator yet, because
> one of the purposes of this table is for people to find if their bug
> tracker is already registered. Until we have a search function I will
> leave it as a very long table.

Great, looks good.

>
> >
> > >
> > > * Clicking on a bug tracker to see its overview page,
> >
> > It looks great, but as far as I'm aware, we have been explicitly
> > asked *not* to put the context details in the side bar. We had this
> > issue with many soyuz pages that hadn't been updated since 1.0 and
> > so had side portlets all over the place.
> >
> > So there are actually two separate-but-related issues here:
> >
> > 1. The main_side layout is meant to be used only if you have
> > notifications, a subscribers list, activity (new download releases),
> > involvement or actions that create objects etc. See
> >
> > https://dev.launchpad.net/VersionThreeDotO/UI/Conversion#Choose the layout
> >
> > 2. The context details should always be displayed in the main
> > content in portlets as you see fit (see the product page as an
> > example: https://edge.launchpad.net/firefox). I've only seen one
> > exception to this rule approved so far, and that was the builder
> > (singular) index page (builder-index.pt), because the builder status
> > and log is extremely important. To see this page, go to:
> >
> > https://edge.launchpad.net/builders
> >
> > and click on an active builder (ie. one that is not idle).
>
> I'm starting to take in all of these rules and motivations :)
>
> As we've already discussed, I've moved the summary to across the top,
> then a row with two portlets - details and related projects - and then
> a row with the watches.

Looks good.

One thing that I don't get (but could just be my lack of context), is that in the Related projects portlet, it mentions that I can link a registered bug tracker with a registered project in LP. But I was looking for an "add project link" or something similar - and checked the edit page too. I'm guessing that it's actually only possible from the project? If that's the case, how about "You can link a registered bug tracker when you edit a registered project in Launchpad:"? See what you think.

>
> >
> > >
> > > * Admire the "Change details" link on the right hand side; this is
> > > the new navigation menu,
> >
> > Note: if you do remove that extra content from the side-bar (context
> > details and related projects), you'll then be left with the same
> > issue that I had with the builder-index.pt above - the only content
> > you'll have for the side bar is an actions menu with a single change
> > details link that is only displayed to a very limited number of
> > people - otherwise it will be empty. For the builder-index.pt I
> > checked with Martin and, since the menu was only displayed to admins
> > (in my case), I decided to always use a main_only template and
> > manually put the action menu on the right-hand side where it
> > normally would be.
>
> As discussed, the side bar is always shown because the Change Details
> link can be clicked by anyone; if they're not logged-in they'll be
> asked to do so.

Yep, and we agreed that it looks a bit strange having just the "Change details" menu on it's own in the side-bar, but this is what Martin preferred in these situations.

>
> >
> > >
> > > * Use the "Bug trackers" breadcrumb to go back,
> >
> > Great!
> >
> > >
> > > * Click "Register a new bug tracker" to go to the +newbugtracker
> > > form,
> >
> > Excellent.
> >
> > >
> > > * Use the "Cancel" link to go back to the first page you were at.
> >
> > Excellent.
>
> I've actually made all of the changes in the -pt2 branch, by accident,
> so if you want to see it, merge that one.
>
> Thanks for the review Michael, it's been very helpful!

Great! (I'll be glad when we have hard-and-fast rules for the 3-0 conversions!). Thanks for the changes.

>
> Gavin.