Code review comment for lp:~allenap/launchpad/ui-convert-filebug-3.0-bug-415263-search

Revision history for this message
Gavin Panella (allenap) wrote :

> Hi Gavin,
>
> This looks so much better without that initial list. Good call.
> I do have, however, comments:

Thanks for looking at it :)

>
> - The second step of the bug report shows me the old UI, even though it
> appears to be the same URL. Could you make sure that this only lands with both
> steps using the same UI?

Sure. I already have a follow-on branch that does this, so I'll land
them together.

> - How about moving the "continue" button right next to the input, and maybe
> rename it to "next", like in our 3.0 mockup?
> https://wiki.canonical.com/Launchpad/UI/Bugs/ThreeDotO/FileBug

The form is being rendered by launchpad-form, which does not allow
more than one widget per row. To get the button to display on the same
row means either doing big work on launchpad-form, or bypassing it,
which would lose us a lot and adds a big maintenance overhead.

Also, as Bjorn has just mentioned in our stand-up call, we very much
need to pick up the pace of template conversions, so I'd like to leave
these kinds of changes for the second round.

Can we leave this for now, and perhaps schedule some work on
launchpad-form?

> - You have 2 h1's on the page. There should only be 1, in this case, the
> "Report a bug". The projects' name should be an h2. Talk to Curtis if you're
> not sure how to fix that

This seems to be a rule in the location_heading formatter:

        if not IHasLogo.providedBy(context):
            context = nearest(context, IHasLogo)
            heading = 'h2'
        else:
            heading = 'h1'

On the +filebug page, the context is the project (or distribution,
etc), so this is always going to end up as an h1. I'll talk to Curtis
about it.

For now, I've changed the "Report a bug" to an h2, which noodles
suggested.

Gavin.

« Back to merge proposal