> Hi Ursula, > > This looks good, but there are a few stylistic issues that need fixing, > so I'm just giving you some quick feedback, so that you can do these > changes with any other changes that come up during the UI review of > this. > > review needs-fixing > > On Fri, 2009-08-28 at 13:15 +0000, Ursula Junque wrote: > > = Summary = > > > > This branch migrates the template of language-index page to the new > > format. > > The bug that's tracking this change is bug 418345. > > It also has a fix for test_translationlinksaggregator, that was > > relying on the database data order and started to fail. > > I've added a 3-line fix for bug 404898, because it wasn't worthy > > creating a separated branch for it. Besides, the fix is related to the > > main change here, that's bug 418345. > > > > > > == Pre-implementation notes == > > > > It was discussed with rockstar about how to deal with the context menu > > now, and he said we should now use NavigationMenu instead of > > ContextMenu. So, considering nothing else in translations was using > > LanguageContextMenu, I've changed this to inherit NavigationMenu, and > > renamed it to LanguageNavigationMenu. > > > > > > == Tests == > > > > This should test all changes, including a fix for > > test_translationlinksaggregator: > > bin/test -vvt lp.translations.* > > > > To test specifically the template migration changes: > > bin/test -vvt xx-language > > > > > > == Demo and Q/A == > > > > To check the changed pages, go to: > > - https://translations.launchpad.dev/+languages/us > > As a regular user you should see the changed layout. > > As an admin user, you should also be able to see the 'Administer' > > link as well, in the new format. > > > === modified file 'lib/lp/translations/stories/standalone/xx-language.txt' > > --- lib/lp/translations/stories/standalone/xx-language.txt 2009-07-01 > 20:45:39 +0000 > > +++ lib/lp/translations/stories/standalone/xx-language.txt 2009-08-28 > 09:10:18 +0000 > > @@ -89,15 +89,16 @@ > > >>> print browser.url > > http://translations.launchpad.dev/+languages/es > > > > - >>> pluralforms_portlet = find_portlet(browser.contents, 'Plural > Forms') > > + >>> pluralforms_portlet = find_portlet(browser.contents, 'Plural > forms') > > >>> print pluralforms_portlet > > - <... > > - The amount of plural forms for Spanish is: 2 > > +
> ... > > - And its default plural form expression is: > > + There are 2 > > + plural forms for Spanish. > > Our test suite runs with the NORMALIZE_WHITESPACE [1] option turned on, > so you can join the two lines above and trim it to make the test a bit > more readable. > > [1] http://www.python.org/doc/2.5.2/lib/doctest-options.html Thanks for the information. Fixed. > > > ... > > 
n != 1
> > ... > > +
> > Why do you want to show that this is inside a
now? I didn't, fixed. > > > > > >>> translationteams_portlet = find_portlet( > > ... browser.contents, 'Translation teams') > > > > === modified file 'lib/lp/translations/templates/language-index.pt' > > --- lib/lp/translations/templates/language-index.pt 2009-07-17 17:59:07 > +0000 > > +++ lib/lp/translations/templates/language-index.pt 2009-08-28 10:29:18 > +0000 > > @@ -3,99 +3,75 @@ > > xmlns:tal="http://xml.zope.org/namespaces/tal" > > xmlns:metal="http://xml.zope.org/namespaces/metal" > > xmlns:i18n="http://xml.zope.org/namespaces/i18n" > > - xml:lang="en" > > - lang="en" > > - dir="ltr" > > - metal:use-macro="context/@@main_template/master" > > i18n:domain="launchpad" > > + metal:use-macro="view/macro:page/main_side" > > > > > - > > - > > -
> > - > > - > > -
> > -

> > - Here you see the information we have available about this language. > > -

> > - > > -

> > - If you see anything wrong with this information, please see whether > it's > > - already been brought up in the > > - Answers application. > > - If not, please file a question there. > > -

> > - > > - > > -
> > - > > -
> > -

Overview of > > - > > - > > - Español > > - ( > > - > > - Spain > > - condition="context/nativename">) > > - in Launchpad. > > + > > +

> > + Overview of > > + > > + > > + Español > > + ( > > + > > + replace="context/englishname">Spain > > + condition="context/nativename">) > > + in Launchpad > >

> > - > > - > > - This language is not usually displayed in lists of languages. You > will > > - have to select it specially in your href="/+editmylanguages">language > > - preferences in order to see it. > > - > > - > > -
> > -
> > -
> > -

Plural Forms

> > -

> > - When translating into this language, you need to be able to > > - express plural forms effectively. The plural form expression > tells > > - the translation system when to use each plural form. For > example, > > - some languages express the idea of "zero objects" differently > to > > - "one object" or "more than ten objects". In these cases, the > > - plural form expression captures the information needed to know > > - which form to use based on the number in question. > > -

> > - > > -

> > - The amount of plural forms for > > - > > - Español > > - > > - is: > > - > > - 2 > > - > > -

> > -

> > - And its default plural form expression is: > > -

> > -

> > - 

> > -                n != 1
> > -
> > -

> > - > > - condition="not:context/pluralexpression"> > > -

> > - Unfortunately, Launchpad doesn't know the plural form > > - information for this language. If you know it, please open a > > - ticket with that > information, > > - so we can add it to Launchpad. > > -

> > - > > -
> > -
> > -
> > + > > + > > +
> > +
> > The indentation is not correct throughout this template. You should use > only two-spaces in page templates. And no tabs, please. ;) Actually my vimrc doesn't allow tabs in my code. :) There was only spaces, I've reindented it to have only 2 spaces spacing. > > > + > > + > > + > > + > > +
> > +
> > +
> > +

Plural forms

> > + > > +

> > + There are replace="context/pluralforms">2 > > + plural forms for replace="view/language_name">Español. > > + > > +

> > +

> > Erm, what's this for? If I remove this, the

, the formula line stays too close to the following text. Do you have any suggestions to avoid this besides using a

tag? > > > +

> > + The default plural form expression for replace="view/language_name">Español is: > > + 

> > +                        n != 1
> > +
> > +

> > +

> > Ditto I removed this one. > > > +

> > + When translating into this language, you need to be > able to > > + express plural forms effectively. The plural form > expression tells > > + the translation system when to use each plural form. > For example, > > + some languages express the idea of "zero objects" > differently to > > + "one object" or "more than ten objects". In these > cases, the > > + plural form expression captures the information needed > to know > > + which form to use based on the number in question. > > +

> > + > > + condition="not:context/pluralexpression"> > > +

> > + Unfortunately, Launchpad doesn't know the plural form > > + information for this language. If you know it, please > open a > > + ticket with that > information, > > + so we can add it to Launchpad. > > +

> > + > > +
> > +
> > + > > +
> > +
> > +
> > +
> > + > > +
> > +
> >
> >

Translation teams

> >

> > @@ -139,12 +115,11 @@ > > > > has no team or person registered as an expert. > >

> > -
> > -
> > -
> > -
> > -
> > -
> > +
> > +
> > + > > +
> > +
> >

Countries

> > > >

> > @@ -169,11 +144,13 @@ > > information, so we can add it to Launchpad. > >

> > > > -
> > -
> > -
> - tal:content="structure context/@@+portlet-top-contributors" /> > > +
> > +
> > + > >
> > -
> > - > > + > > + > > + > > + > > + > > > > > > === modified file 'lib/lp/translations/templates/language-portlet-top- > contributors.pt' > > --- lib/lp/translations/templates/language-portlet-top-contributors.pt > 2009-07-17 17:59:07 +0000 > > +++ lib/lp/translations/templates/language-portlet-top-contributors.pt > 2009-08-28 12:13:06 +0000 > > @@ -13,10 +13,9 @@ > > Español > > : > >

> > -
> > > > -- > Guilherme Salgado