Code review comment for lp:~henninge/launchpad/bug-425583-languages

Revision history for this message
Henning Eggers (henninge) wrote :

= Bug 425593 =

This branch adds the UI elements for the user's preferred languages and the full list of languages in Launchpad as described in the bug report. Adding the breadcrumb (as mentioned in the bug) was already done by some other branch.

As discussed in the bug report, the page changes the search form to a filter form and filters the dispĺayed languages on the client side.

== Implementation notes ==

Although the preferred languages display is copied from what is on the person index page (person-portlet-contact-details.pt) it does not share code. I wanted the languages to be links, too. Also, constructing a comma-speparated list is not easily done in TAL and can get ugly, so I am generating the HTML snippet in the view code. I *could* use the TAL formatter here but I think the icons might be confusing when used in such a list.

Displaying the language listing was a breeze, as there is a TAL link formatter for ILanguage. Also I had found out about the "two-column-list" css class earlier and could easily construct a "three-column-list".

For the JS code I created a new file in the lib/lp hierarchy and linked to it from lib/canonical/launchpad. I assumed that this is the new way of doing things and that l/c/l will go away eventually, won't it?

The diff for the template got quite long as I put everything into the top-portlet and had to adapt the indention.

== Tests ==

bin/test -vvt language-views.txt

Windmill tests to follow.

== Demo/QA ==

Got to https://translations.launchpad.dev/+languages as user carlos and see the three preferred languages he has. Also have fun filtering languages. ;)

== Launchpad Lint ==

I merged db-devel to make use of yui3final, so "make lint" is confused now, trying to diff against devel. So, no useful lint output available, sorry.

« Back to merge proposal