Code review comment for lp:~abentley/launchpad/request-build

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

On Fri, Apr 9, 2010 at 4:15 PM, Aaron Bentley <email address hidden> wrote:
> On 04/09/2010 06:30 AM, Michael Nelson wrote:
>> Review: Needs Information ui
>> Hi Aaron and Paul,
>>
>> Looking great.
>>
>> Regarding the table styling, did you try adding the 'listing' class?
>
> No.  Thanks for pointing it out.
>
> However, it does not work very well:
> http://people.canonical.com/~abentley/recipe-index-table-4.png
>
> The table is unnecessarily wide,

There is .narrow-listing too (which you would need to add in addition
to .listing), if that helps (as yes, the .listing class defaults to
100%)

> and the headings are not aligned with
> the cell contents.

I *think* this is because all the listing styles assume that your
table uses the thead/tbody elements, which you're not afaics. See:

lib/canonical/launchpad/icing/style.css:278 ff.

>
>> I think I forgot to mention during the review for the recipe index page, but I had a question about the breadcrumbs... shouldn't they be:
>>
>> Person-name5>>  Branches>>  generic_string23 recipe and
>> Person-name5>>  Branches>>  generic_string23 recipe>>  Request builds
>> ?

Any comment about this?

>>
>> And lastly, I was surprised to see non-ubuntu distroseries there for selection?
>
> We're aware that we may need to tweak this before going live, but it has
> been difficult to get answers about exactly what selection criteria
> should be used, so we went with simply listing active distroseries.

I see. My assumption is that it's simply active ubuntu distroseries.

Hope that helps!

>
> Aaron
> --
> https://code.launchpad.net/~abentley/launchpad/request-build/+merge/22570
> You are reviewing the proposed merge of lp:~abentley/launchpad/request-build into lp:launchpad.
>

« Back to merge proposal