Code review comment for lp://staging/~jtv/launchpad/bug-418430-stuff-to-review-3.0

Revision history for this message
Deryck Hodge (deryck) wrote :

Hi, Jeroen.

This looks nice!

One note about the UI, when I looked at the page itself, the two
headings -- the person name heading in an h2 and the page title in an
h1 -- were in the same purple color used for translations currently.
As I understand it, these should be light gray and green respectively.
I assume there's some current 2.0 LP style rule making all headings
this color for translations. The same is true of the bugs app, I
know. So I just call your attention to it, if you're not already
aware. Nothing to fix now, but at some point it has to be changed.
Maybe you guys have already talked about this, but just in case not, I
mention it.

I see only one minor fix to recommend:

> + <div class="top-portlet">
> + <h1>Translations to review</h1>
> + <tal:work-to-do condition="view/num_projects_and_packages_to_review">
> + <p>
> + These translations are available to be reviewed by
> + <tal:person replace="structure context/fmt:link">R. Viewer</tal:person>:
> + </p>

The h1 of the page should match the page title. In the case of the
page I sampled, the page title was "Translations for review by
Launchpad German translators" so I would expect the main h1 to be that
as well.

Otherwise, this looks good. Marking this as approved, with the
assumption that this one change will be made.

Cheers,
deryck

review: Approve (code)

« Back to merge proposal