Code review comment for lp:~wallyworld/launchpad/recipe-find-related-branches

Revision history for this message
Ian Booth (wallyworld) wrote :

> I think it's good to have the links to the SourcePackage, but we should also
> have them for the ProductSeries. Maybe that could replace the owner column?
> Or is the owner really important for some reason?
>

I've added links to the product series instead of owner. I've also made the ui look better by including <h2> headings for each table. These headings and the table headers etc are all styled with the "code" domain css. Hence the headings appear in gold etc. New screenshot: http://people.canonical.com/~ianb/recipe-related-branches2.png

The ordering for the product series branches is based on the sorted_active_series_list property provided by the SortSeriesMixin. This provides only active series with the most recent first and the development focus at the top of the list.

I realised when doing these changes that I was exposing private branches and others the users had no right to see. So I fixed the implementation and added a couple of extra tests.

> I thought the text area widgets were constrained by the width of the
> surrounding elements, but I was wrong; zope's TextAreaWidget uses, by default,
> a 60-cols width. I'd also prefer if the tables had the same width as the
> other form elements, but I'm not sure using a fixed width on the table is a
> good idea because the widget's width might change but most importantly because
> I don't think the fixed-width table will behave the same way as the textare
> widget when the window is resized to a smaller size than what's needed to fit
> the textarea.

Before doing the fixed width tables, I did check the behaviour of the widgets' layouts when the browser window was resized and it all behaved as expected - the text area widget and tables retained their width and a simple horizontal scroll bar was added to the browser window.

What I've done now is to make the <fieldset> element which contains everything for the related branches widget a fixed size and remove the width styling from the tables. The resize behaviour is the same as described above but I think it's a cleaner implementation.

« Back to merge proposal