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

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

> To walk through the UI, try:

Thanks for providing these walk-throughs... much easier to review! I'll start doing that too.
And sorry for the two 'needs fixing' reviews! The pages really do look great (we initially tried to re-use our soyuz context portlets in the same way), but afaik, it's left-over from 1.0 (or maybe I'm confused at that point - I wasn't around then), but it's not meant to be in the 3.0 side-bar.

>
> * Go to the bug trackers page (for IBugTrackerSet) at
> <https://bugs.launchpad.dev/bugs/bugtrackers>,
>
> * There's nothing new in this page for this branch, so just click on
> "This Mozilla.prg Bug Tracker" to see its overview page,
>
> * Look at the portlets. The top portlet replicates some of the info
> that's in the page, but it is used elsewhere in Launchpad so it's
> useful to keep.

See comments on the first MP. I think it's really 1.0 templates trying to be forced into the 3.0 mold (I tried the same thing with some soyuz pages and got rejected ;) ).

>
> * Click "Change details",
>
> * This is one place where the details portlet is useful.

Well, not really - the only extra information presented in the portlet is not really relevant to the person editing the watch? Otherwise it's just duplicated info, and as mentioned in the previous MP, afaik, isn't the type of content that is meant to go in the side portlet. I'd recommend using main_only for this template, you might even be able to get rid of the template and use generic-edit.pt?

>
> * Use the new cancel link to return to the previous page,

Great!

>
> * Notice the new h1 title and h2 "Summary" heading,

Great! Note that both of these above two points will happen automatically with generic-edit.

>
> * Append /42 to the URL and see the remote bug index page, including
> the h2 "Is watched by..." and the new text. I think the text is
> good, but the heading was a tough one. I'm simply still not sure
> exactly how to use the heading slot.

Yeah, it's been confusing and could change again - but afaik, you've done the right thing for the current rules (ie. this is an +index page, so putting an <h1>context.title</h1> is correct).

I'm not so sure about "Is watched by...". I think this h2 shouldn't be a continuation of a sentance. In fact, I'm not even sure that the h2 is necessary. I know I'm looking at the Remote bug #42 for the Mozilla.org bug tracker, and your overview para tells me exactly what I need to know.

And again, I don't think we can put the context details portlet in the side-bar (it was requested for 1.0 and afaik meant to be removed for 2.0(?) and doesn't belong in the side-bar for 3.0).

review: Needs Fixing (ui)

« Back to merge proposal