Merge lp://staging/~jtv/launchpad/translations-person-3.0-mechanical into lp://staging/launchpad
- translations-person-3.0-mechanical
- Merge into devel
Status: | Merged | ||||
---|---|---|---|---|---|
Merged at revision: | not available | ||||
Proposed branch: | lp://staging/~jtv/launchpad/translations-person-3.0-mechanical | ||||
Merge into: | lp://staging/launchpad | ||||
Diff against target: | None lines | ||||
To merge this branch: | bzr merge lp://staging/~jtv/launchpad/translations-person-3.0-mechanical | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Martin Albisetti (community) | ui | Needs Fixing | |
Jeroen T. Vermeulen (community) | Abstain | ||
Brad Crittenden (community) | code | Approve | |
Review via email: mp+10984@code.staging.launchpad.net |
Commit message
Description of the change
Jeroen T. Vermeulen (jtv) wrote : | # |
Martin Albisetti (beuno) wrote : | # |
Unfortunately, my rf is borked, so I'll need screenshots to review
Brad Crittenden (bac) wrote : | # |
Hi Jeroen,
Here are some random thoughts as I review your branch:
* In your merge proposal "Demo & QA" could've been a little more fleshed out. I'm not all that familiar with testing translations so I don't know who is well represented in sample data.
* On +langauges/es there are a lot of countries listed. I think the space can be better used if you have that section use a two-column list.
* At https:/
* This page https:/
.../canonical/
Looking for [licence] in files like .*p[ty] under .:
./lib/lp/
40: BSD = Item("licence all my translations in Launchpad "
41: "under the BSD licence.")
./lib/lp/
20: <a href="https:/
25: If you do not agree to the BSD licence, you will not be able
./lib/lp/
31: * Reflect updated licence.
./lib/lp/
98: <dl id="licences">
* The links at the bottom of https:/
I'm marking it approved, with the suggested changes. I'm sure the UI review will generate some changes.
--bac
> === modified file 'lib/canonical/
> --- lib/canonical/
> +++ lib/canonical/
> @@ -3127,3 +3127,28 @@
> return self.base
> else:
> return self.master
> +
> +
> +class TranslationGrou
> + """Adapter for `ITranslationGroup` objects to a formatted string."""
Thanks for adding these formatters.
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> @@ -50,13 +50,21 @@
> '../templates/
>
> @property
> + def page_title(self):
> + """See `LaunchpadFormV
> + return "Translation import queue for %s" % self.context.
> +
> + @property
> + def label(self):
> + """See `LaunchpadFormV
> + return self.page_title
The bugs dudes are doing simply the following, which is lighter:
label = page_title
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> @@ -24,6 +24,8 @@
> from zope.component import getUtility
> from...
Jeroen T. Vermeulen (jtv) wrote : | # |
> Unfortunately, my rf is borked, so I'll need screenshots to review
...And of course my system had to freeze up while I was refreshing pages
for my screenshots. Taking my detailed guide with it. Something in
Launchpad must tickle the system the wrong way; I had a freeze while
running tests yesterday and Ursula had one the day before.
See screenshots here:
http://
Read on for my _new_ detailed description of same. :-)
= 01: Main Person page =
This page is in a state of flux. That's why it's still a jumble of old
features and new ones for the personal translations dashboard as
designed at our BA sprint in March. The plan is to throw in dashboard
features as we implement them, and then go over the composition of the
page to make it look decent.
One thing that's painfully obvious here is that there's not enough
padding above and below the h2 headings and the listing tables. I
didn't hack around that by inserting my own spacing, since it'd conflict
with any attempt to fix the padding globally.
I'm given to understand that the tables should be no wider than 800px
but our CSS does not limit them. I used style attributes instead.
One icon was spritified on this page. There's a related-pages list at
the bottom, which is the direct descendant of the old pages' navigation
menu. This is not pretty either, but again needs global fixing rather
than ad-hoc combovers.
= 02: Translations licensing =
There are two ways to get to this page. One is from the navigation menu
but the other is to try and access a translation page without having
registered agreement with the licensing terms. Hence the title; in this
case the <h1> is radically different from the <title>, but I think that
is appropriate for the situation. It'd be nice if we could change this
depdending on how you arrived here, but that's not particularly worth
fixing now.
I changed the wording here a bit, and (as Brad suggested) spelled
licen[cs]e consistently as "license."
There is no navigation menu here; this is basically a settings page so
it gained a Cancel link instead.
= 03: Import queue =
Here you see a person's translations import queue. The title and label
are more consistent and no longer try to explain the purpose of the
page.
I would dearly love to replace the status dropdowns and "Change status"
button with status-picker widgets, but figured that's best left to a
separate branch.
On the right, next to the first queue entry, you see the info and
foldout icons indicating warning output attached to the entry. Those
two icons are not spritefied yet; nor is the spinner that shows up when
you click them. Another thing I'd much like to fix, but since it's all
in JavaScript, it seems more appropriate to do that separately.
= 04: POFile filter =
Ursula will be giving this page the love it really needs. Here you see
just the initial 3.0 template conversion.
Note that this page is actually in the POFile context, not that of the
Person you access it from. In a way it's a junction of those two
context. In this case I ended up dropping the navigation menu (in a
very late state) and just linking to...
Jeroen T. Vermeulen (jtv) wrote : | # |
Brad,
Thanks for the review. I made some more small changes, such as using
sprites for images--though nothing particularly exciting.
> * On +langauges/es there are a lot of countries listed. I think the space can
> be better used if you have that section use a two-column list.
That's outside the scope of this branch, showing that my instructions
were indeed not clear enough. Spanish is fairly exceptional though, and
you're not missing out on anything because of the extra vertical stretch
so I don't think it's going to be an issue in practice.
> * At https:/
> important" has to be redirected. Do you want to make it point to the actual
> location since you're editing the page template?
Appearances notwithstanding, it's still the right page for this link.
> * This page https:/
> spellings of 'license'. Please standardize on the en_US spelling. (Have I
> not filed a bug on this a long time ago?)
Fixed.
> > === modified file 'lib/lp/
> > --- lib/lp/
> 00:26:05 +0000
> > +++ lib/lp/
> 09:51:12 +0000
> > @@ -50,13 +50,21 @@
> > '../templates/
> >
> > @property
> > + def page_title(self):
> > + """See `LaunchpadFormV
> > + return "Translation import queue for %s" % self.context.
> > +
> > + @property
> > + def label(self):
> > + """See `LaunchpadFormV
> > + return self.page_title
>
> The bugs dudes are doing simply the following, which is lighter:
>
> label = page_title
That's a nice trick! Unfortunately it oopses the page for me. I'll be
lazy and leave this as it is for now.
> > === modified file 'lib/lp/
> > --- lib/lp/
> > +++ lib/lp/
> > @@ -24,6 +24,8 @@
> > from zope.component import getUtility
> > from zope.publisher.
> >
> > +from canonical.
> > +
> > from canonical.
> > from canonical.config import config
> > from lp.translations
> > @@ -246,6 +248,24 @@
> >
> > DEFAULT_BATCH_SIZE = 50
> >
> > + def page_title(self):
> > + """See `LaunchpadView`."""
> > + if self.person is None:
> > + person_name = "unknown person"
> > + else:
> > + person_name = self.person.
> > +
> > + return smartquote(
> > + person_name, self.context.title)
> > +
> > + def label(self):
> > + """See `LaunchpadView`."""
> > + if self.person:
> > + person_name = "unknown person"
> > + else:
> > + person_name = self.person.
> > + return "Translations by %s" % person_name
>
> Looks like a good spot fo...
Jeroen T. Vermeulen (jtv) wrote : | # |
Incremental diff (also reflected in the screenshots):
=== modified file 'lib/lp/
--- lib/lp/
+++ lib/lp/
@@ -248,23 +248,22 @@
DEFAULT_
- def page_title(self):
- """See `LaunchpadView`."""
+ @property
+ def _person_name(self):
+ """Person's display name. Graceful about unknown persons."""
if self.person is None:
- person_name = "unknown person"
+ return "unknown person"
else:
- person_name = self.person.
+ return self.person.
+ def page_title(self):
+ """See `LaunchpadView`."""
return smartquote(
- person_name, self.context.title)
+ self._person_name, self.context.title)
def label(self):
"""See `LaunchpadView`."""
- if self.person:
- person_name = "unknown person"
- else:
- person_name = self.person.
- return "Translations by %s" % person_name
+ return "Translations by %s" % self._person_name
def initialize(self):
"""See `LaunchpadView`."""
=== modified file 'lib/lp/
--- lib/lp/
+++ lib/lp/
@@ -37,9 +37,9 @@
class TranslationReli
- BSD = Item("licence all my translations in Launchpad "
- "under the BSD licence.")
- REMOVE = Item("not make translations in Launchpad.")
+ BSD = Item("License all my translations in Launchpad "
+ "under the BSD license.")
+ REMOVE = Item("Not make translations in Launchpad.")
class ITranslationRel
=== modified file 'lib/lp/
--- lib/lp/
+++ lib/lp/
@@ -17,12 +17,12 @@
<p>
To let the maximum number of projects benefit from this, we ask
that you agree to <strong>license all your translations under the
- <a href="https:/
+ <a href="https:/
If you change your mind later, you can return to this page
</p>
<p>
- If you do not agree to the BSD licence, you will not be able
+ If you do not agree to the BSD license, you will not be able
to make translations in Launchpad.
</p>
<p>
=== modified file 'lib/lp/
--- lib/lp/
Martin Albisetti (beuno) wrote : | # |
01, 03 and 04: Even though it's a mechanical change, it looks incredibly messy. Even more so in the new design. Some of it, as you say, is general layout issues that need to be addressed, but a lot of it is that this layout is just confusing.
I can't approve it in this state, but talk to the other UI reviewers to help you through it.
Also, moving the menu to the bottom seems wrong, and unlike the rest of the pages. Is there any other way people can navigate through these options?
02: Nice improvement for a mechanical change
05: Nice improvement for a mechanical change, although that stray info icon is hard to guess what it will do
Данило Шеган (danilo) wrote : | # |
У сре, 02. 09 2009. у 17:33 +0000, Martin Albisetti пише:
> Review: Needs Fixing ui
> 01, 03 and 04: Even though it's a mechanical
> change, it looks incredibly messy. Even more so in the new design.
> Some of it, as you say, is general layout issues that need to be
> addressed, but a lot of it is that this layout is just confusing. I
> can't approve it in this state, but talk to the other UI reviewers to
> help you through it. Also, moving the menu to the bottom seems wrong,
> and unlike the rest of the pages. Is there any other way people can
> navigate through these options?
Re 03: is it not exactly the same as 05, plus the related pages links.
Re 04: this is why I say mechanical changes need no UI review. The way
developers work is that they incrementally develop code. Plan is to
have this template migrated first, and then to have it reworked.
It's not about cheating in statistics. It's about dealing with work
planning. We do have a separate task for reworking this page before
3.0, but it's not Jeroen's plate.
There is more to this though: translations pages don't provide any
breadcrumbs. This is something not reflected in the conversions.html,
but is as important. Our plan is to go through all pages again and make
sure page title, label and breadcrumbs are up to satisfaction for 3.0.
To be honest, I don't understand why he went for the UI review with this
branch in the first place.
Though, one thing I vehemently agree with: let's get rid of the related
pages links on 03 and 04 mockups: once breadcrumbs are in, we won't need
them.
As far as 01 goes, it's crap today (and if Jeroen had better sampledata
for his mockups, it would not have been that bad). I think the right
way forward for it is:
* split it out into separate bug/branch
* move full history to a separate page (+history)
* fix up all the other details to match our desires from the UI sprint
* get it through UI review and review and land it
For all the rest (02, 03, 04, 05):
* get rid of related pages links, land (it's either approved for
mechanical changes, or we already planned to do more work on them: I am
actually surprised by the opinion of 03, because it's just like 05 +
related pages links)
* set up separate tasks for any remaining fixes (pofile-filter mainly,
which we already have planned for)
Preview Diff
1 | === modified file 'lib/canonical/launchpad/pagetitles.py' |
2 | --- lib/canonical/launchpad/pagetitles.py 2009-09-01 07:52:07 +0000 |
3 | +++ lib/canonical/launchpad/pagetitles.py 2009-09-01 10:46:26 +0000 |
4 | @@ -76,17 +76,6 @@ |
5 | return self.text % context.displayname |
6 | |
7 | |
8 | -class FilteredTranslationsTitle(SubstitutionHelper): |
9 | - """Return the formatted string with context's title and view's person.""" |
10 | - def __call__(self, context, view): |
11 | - if view.person is not None: |
12 | - person = view.person.displayname |
13 | - else: |
14 | - person = 'unknown' |
15 | - return self.text % {'title' : context.title, |
16 | - 'person' : person } |
17 | - |
18 | - |
19 | class ContextId(SubstitutionHelper): |
20 | """Return the formatted string with context's id.""" |
21 | def __call__(self, context, view): |
22 | @@ -503,8 +492,6 @@ |
23 | |
24 | hassprints_sprints = ContextTitle("Events related to %s") |
25 | |
26 | -hastranslationimports_index = 'Translation import queue' |
27 | - |
28 | hwdb_fingerprint_submissions = ( |
29 | "Hardware Database submissions for a fingerprint") |
30 | |
31 | @@ -829,8 +816,6 @@ |
32 | |
33 | person_translations = ContextDisplayName('Translations related to %s') |
34 | |
35 | -person_translations_relicensing = "Translations licensing" |
36 | - |
37 | person_translations_to_review = ContextDisplayName( |
38 | 'Translations for review by %s') |
39 | |
40 | @@ -841,9 +826,6 @@ |
41 | person_vouchers = ContextDisplayName( |
42 | 'Commercial subscription vouchers for %s') |
43 | |
44 | -pofile_filter = FilteredTranslationsTitle( |
45 | - smartquote('Translations by %(person)s in "%(title)s"')) |
46 | - |
47 | pofile_index = ContextTitle(smartquote('Translation overview for "%s"')) |
48 | |
49 | def pofile_translate(context, view): |
50 | @@ -1259,10 +1241,6 @@ |
51 | |
52 | translationimportqueueentry_index = 'Translation import queue entry' |
53 | |
54 | -translationimportqueue_index = 'Translation import queue' |
55 | - |
56 | -translationimportqueue_blocked = 'Translation import queue - Blocked' |
57 | - |
58 | def translationmessage_translate(context, view): |
59 | """Return the page to translate a template into a language per message.""" |
60 | return 'Translating %s into %s' % ( |
61 | |
62 | === modified file 'lib/canonical/launchpad/webapp/configure.zcml' |
63 | --- lib/canonical/launchpad/webapp/configure.zcml 2009-08-27 09:00:17 +0000 |
64 | +++ lib/canonical/launchpad/webapp/configure.zcml 2009-09-01 08:37:31 +0000 |
65 | @@ -507,6 +507,12 @@ |
66 | name="fmt" |
67 | /> |
68 | <adapter |
69 | + for="lp.translations.interfaces.translationgroup.ITranslationGroup" |
70 | + provides="zope.traversing.interfaces.IPathAdapter" |
71 | + factory="canonical.launchpad.webapp.tales.TranslationGroupFormatterAPI" |
72 | + name="fmt" |
73 | + /> |
74 | + <adapter |
75 | for="*" |
76 | provides="zope.traversing.interfaces.IPathAdapter" |
77 | factory="canonical.launchpad.webapp.tales.PermissionRequiredQuery" |
78 | |
79 | === modified file 'lib/canonical/launchpad/webapp/tales.py' |
80 | --- lib/canonical/launchpad/webapp/tales.py 2009-08-25 19:31:41 +0000 |
81 | +++ lib/canonical/launchpad/webapp/tales.py 2009-09-01 08:37:31 +0000 |
82 | @@ -3127,3 +3127,28 @@ |
83 | return self.base |
84 | else: |
85 | return self.master |
86 | + |
87 | + |
88 | +class TranslationGroupFormatterAPI(ObjectFormatterAPI): |
89 | + """Adapter for `ITranslationGroup` objects to a formatted string.""" |
90 | + |
91 | + traversable_names = { |
92 | + 'link': 'link', |
93 | + 'url': 'url', |
94 | + 'displayname': 'displayname', |
95 | + } |
96 | + |
97 | + def url(self, view_name=None, rootsite='translations'): |
98 | + """See `ObjectFormatterAPI`.""" |
99 | + return super(TranslationGroupFormatterAPI, self).url( |
100 | + view_name, rootsite) |
101 | + |
102 | + def link(self, view_name, rootsite='translations'): |
103 | + """See `ObjectFormatterAPI`.""" |
104 | + group = self._context |
105 | + url = self.url(view_name, rootsite) |
106 | + return u'<a href="%s">%s</a>' % (url, cgi.escape(group.title)) |
107 | + |
108 | + def displayname(self, view_name, rootsite=None): |
109 | + """Return the displayname as a string.""" |
110 | + return self._context.title |
111 | |
112 | === modified file 'lib/lp/translations/browser/hastranslationimports.py' |
113 | --- lib/lp/translations/browser/hastranslationimports.py 2009-07-17 00:26:05 +0000 |
114 | +++ lib/lp/translations/browser/hastranslationimports.py 2009-09-01 09:51:12 +0000 |
115 | @@ -50,13 +50,21 @@ |
116 | '../templates/translation-import-queue-macros.pt') |
117 | |
118 | @property |
119 | + def page_title(self): |
120 | + """See `LaunchpadFormView`.""" |
121 | + return "Translation import queue for %s" % self.context.displayname |
122 | + |
123 | + @property |
124 | + def label(self): |
125 | + """See `LaunchpadFormView`.""" |
126 | + return self.page_title |
127 | + |
128 | + @property |
129 | def initial_values(self): |
130 | return self._initial_values |
131 | |
132 | def initialize(self): |
133 | - """Set form label depending on the context.""" |
134 | - self.label = 'Translation files waiting to be imported for %s' % ( |
135 | - self.context.displayname) |
136 | + """See `LaunchpadFormView`.""" |
137 | self._initial_values = {} |
138 | LaunchpadFormView.initialize(self) |
139 | |
140 | |
141 | === modified file 'lib/lp/translations/browser/person.py' |
142 | --- lib/lp/translations/browser/person.py 2009-08-31 16:55:39 +0000 |
143 | +++ lib/lp/translations/browser/person.py 2009-09-01 14:28:06 +0000 |
144 | @@ -361,6 +361,10 @@ |
145 | custom_widget('back_to', TextWidget, visible=False) |
146 | |
147 | @property |
148 | + def page_title(self): |
149 | + return "Translations licensing by %s" % self.context.displayname |
150 | + |
151 | + @property |
152 | def initial_values(self): |
153 | """Set the default value for the relicensing radio buttons.""" |
154 | translations_person = ITranslationsPerson(self.context) |
155 | @@ -380,6 +384,11 @@ |
156 | """Return an URL for this view.""" |
157 | return canonical_url(self.context, view_name='+licensing') |
158 | |
159 | + @property |
160 | + def cancel_url(self): |
161 | + """Escape to the person's main Translations page.""" |
162 | + return canonical_url(self.context) |
163 | + |
164 | def getSafeRedirectURL(self, url): |
165 | """Successful form submission should send to this URL.""" |
166 | if url and url.startswith(self.request.getApplicationURL()): |
167 | @@ -391,9 +400,9 @@ |
168 | def submit_action(self, action, data): |
169 | """Store person's decision about translations relicensing. |
170 | |
171 | - Decision is stored through |
172 | + The user's decision is stored through |
173 | `ITranslationsPerson.translations_relicensing_agreement` |
174 | - which uses TranslationRelicensingAgreement table. |
175 | + which is backed by the TranslationRelicensingAgreement table. |
176 | """ |
177 | translations_person = ITranslationsPerson(self.context) |
178 | allow_relicensing = data['allow_relicensing'] |
179 | @@ -406,8 +415,6 @@ |
180 | translations_person.translations_relicensing_agreement = False |
181 | self.request.response.addInfoNotification(_( |
182 | "We respect your choice. " |
183 | - "Your translations will be removed once we complete the " |
184 | - "switch to the BSD license. " |
185 | "Thanks for trying out Launchpad Translations.")) |
186 | else: |
187 | raise AssertionError( |
188 | |
189 | === modified file 'lib/lp/translations/browser/pofile.py' |
190 | --- lib/lp/translations/browser/pofile.py 2009-08-31 13:06:42 +0000 |
191 | +++ lib/lp/translations/browser/pofile.py 2009-09-01 11:41:42 +0000 |
192 | @@ -24,6 +24,8 @@ |
193 | from zope.component import getUtility |
194 | from zope.publisher.browser import FileUpload |
195 | |
196 | +from canonical.lazr.utils import smartquote |
197 | + |
198 | from canonical.cachedproperty import cachedproperty |
199 | from canonical.config import config |
200 | from lp.translations.browser.translationmessage import ( |
201 | @@ -246,6 +248,24 @@ |
202 | |
203 | DEFAULT_BATCH_SIZE = 50 |
204 | |
205 | + def page_title(self): |
206 | + """See `LaunchpadView`.""" |
207 | + if self.person is None: |
208 | + person_name = "unknown person" |
209 | + else: |
210 | + person_name = self.person.displayname |
211 | + |
212 | + return smartquote('Translations by %s in "%s"') % ( |
213 | + person_name, self.context.title) |
214 | + |
215 | + def label(self): |
216 | + """See `LaunchpadView`.""" |
217 | + if self.person: |
218 | + person_name = "unknown person" |
219 | + else: |
220 | + person_name = self.person.displayname |
221 | + return "Translations by %s" % person_name |
222 | + |
223 | def initialize(self): |
224 | """See `LaunchpadView`.""" |
225 | self.person = None |
226 | |
227 | === modified file 'lib/lp/translations/browser/tests/test_translationlinksaggregator.py' |
228 | --- lib/lp/translations/browser/tests/test_translationlinksaggregator.py 2009-08-29 19:37:54 +0000 |
229 | +++ lib/lp/translations/browser/tests/test_translationlinksaggregator.py 2009-09-01 10:11:08 +0000 |
230 | @@ -224,7 +224,7 @@ |
231 | (product, canonical_url(product_pofile), [product_pofile]), |
232 | (package, canonical_url(package_pofile), [package_pofile]), |
233 | ] |
234 | - self.assertEqual(expected, descriptions) |
235 | + self.assertContentEqual(expected, descriptions) |
236 | |
237 | def test_aggregate_bundles_productseries(self): |
238 | # _aggregateTranslationTargets describes POFiles for the same |
239 | |
240 | === modified file 'lib/lp/translations/browser/translationimportqueue.py' |
241 | --- lib/lp/translations/browser/translationimportqueue.py 2009-07-17 00:26:05 +0000 |
242 | +++ lib/lp/translations/browser/translationimportqueue.py 2009-09-01 09:51:12 +0000 |
243 | @@ -397,13 +397,16 @@ |
244 | |
245 | |
246 | class TranslationImportQueueView(HasTranslationImportsView): |
247 | - """View class used for Translation Import Queue management.""" |
248 | - label = 'Translation files waiting to be imported.' |
249 | + """The global Translation Import Queue.""" |
250 | + @property |
251 | + def page_title(self): |
252 | + """See `HasTranslationImportsView`.""" |
253 | + # There's no useful context here, so dumb down the title. |
254 | + return "Translation import queue" |
255 | |
256 | def initialize(self): |
257 | """Useful initialization for this view class.""" |
258 | - self._initial_values = {} |
259 | - LaunchpadFormView.initialize(self) |
260 | + super(TranslationImportQueueView, self).initialize() |
261 | target_filter = self.widgets['filter_target'] |
262 | if target_filter.hasInput() and not target_filter.hasValidInput(): |
263 | raise UnexpectedFormData("Unknown target.") |
264 | |
265 | === modified file 'lib/lp/translations/interfaces/translationrelicensingagreement.py' |
266 | --- lib/lp/translations/interfaces/translationrelicensingagreement.py 2009-07-17 00:26:05 +0000 |
267 | +++ lib/lp/translations/interfaces/translationrelicensingagreement.py 2009-09-01 14:59:04 +0000 |
268 | @@ -37,10 +37,9 @@ |
269 | |
270 | |
271 | class TranslationRelicensingAgreementOptions(EnumeratedType): |
272 | - BSD = Item("I agree to licence all my translations in Launchpad " |
273 | - "using the BSD licence.") |
274 | - REMOVE = Item("I do not want to use the BSD licence and understand this " |
275 | - "means I can't make translations in Launchpad.") |
276 | + BSD = Item("licence all my translations in Launchpad " |
277 | + "under the BSD licence.") |
278 | + REMOVE = Item("not make translations in Launchpad.") |
279 | |
280 | |
281 | class ITranslationRelicensingAgreementEdit(ITranslationRelicensingAgreement): |
282 | |
283 | === modified file 'lib/lp/translations/stories/standalone/xx-licensing.txt' |
284 | --- lib/lp/translations/stories/standalone/xx-licensing.txt 2009-08-25 06:39:38 +0000 |
285 | +++ lib/lp/translations/stories/standalone/xx-licensing.txt 2009-09-01 14:28:06 +0000 |
286 | @@ -14,9 +14,9 @@ |
287 | that this page has information about relicensing. |
288 | |
289 | >>> print browser.title |
290 | - Translations licensing |
291 | + Translations licensing by Karl Tilbury |
292 | >>> print extract_text(find_main_content(browser.contents)) |
293 | - Overview / Translations licensing / ... |
294 | + Karl Tilbury |
295 | Sorry to interrupt... |
296 | |
297 | Karl is asked whether he wants to license his translations. |
298 | @@ -33,11 +33,10 @@ |
299 | >>> radiobuttons.value = ['REMOVE'] |
300 | >>> browser.getControl("Confirm").click() |
301 | |
302 | -Karl sees a notice that his translations will be removed in the near |
303 | -future. |
304 | +Karl sees a notice acknowledging his choice. |
305 | |
306 | >>> get_feedback_messages(browser.contents) |
307 | - [u'We respect your choice. Your translations will be removed...'] |
308 | + [u'We respect your choice...'] |
309 | |
310 | He's also forwarded back to the Spanish alsa-utils translations page. |
311 | |
312 | @@ -72,7 +71,7 @@ |
313 | >>> browser.url |
314 | 'http://translations.launchpad.dev/~karl/+licensing' |
315 | >>> print browser.title |
316 | - Translations licensing |
317 | + Translations licensing by Karl Tilbury |
318 | |
319 | Karl sees that the current value is 'no', which he set before. |
320 | |
321 | |
322 | === modified file 'lib/lp/translations/stories/standalone/xx-person-translations.txt' |
323 | --- lib/lp/translations/stories/standalone/xx-person-translations.txt 2009-07-06 11:57:21 +0000 |
324 | +++ lib/lp/translations/stories/standalone/xx-person-translations.txt 2009-09-01 09:51:12 +0000 |
325 | @@ -44,7 +44,7 @@ |
326 | |
327 | >>> listing = find_tags_by_class(anon_browser.contents, 'listing')[0] |
328 | >>> print_list_of_contributions(listing) |
329 | - Translation file Untra... Needs... Latest translation |
330 | + Translation file Untra... Need... Latest translation |
331 | English (en) translat... 6 0 2007-07-12 auto, esddsp... |
332 | Spanish (es) translat... 3 0 2007-04-07 _: EMAIL OF ... |
333 | Spanish (es) translat... 0 0 2007-01-24 test man pag... |
334 | |
335 | === modified file 'lib/lp/translations/stories/translationgroups/05-add-translation-group.txt' |
336 | --- lib/lp/translations/stories/translationgroups/05-add-translation-group.txt 2009-08-25 16:42:56 +0000 |
337 | +++ lib/lp/translations/stories/translationgroups/05-add-translation-group.txt 2009-09-01 16:00:23 +0000 |
338 | @@ -154,7 +154,6 @@ |
339 | >>> for group_row in groups: |
340 | ... group = group_row.findNext('td') |
341 | ... print '%s: %s' % (group.a.string, group.a['href']) |
342 | - Just a testing team: testing-translation-team |
343 | - Single-language Translators: monolingua |
344 | - The PolyGlot Translation Group: polyglot |
345 | - |
346 | + Just a testing team: ...testing-translation-team |
347 | + Single-language Translators: ...monolingua |
348 | + The PolyGlot Translation Group: ...polyglot |
349 | |
350 | === modified file 'lib/lp/translations/templates/hastranslationimports-index.pt' |
351 | --- lib/lp/translations/templates/hastranslationimports-index.pt 2009-07-17 17:59:07 +0000 |
352 | +++ lib/lp/translations/templates/hastranslationimports-index.pt 2009-09-01 15:16:28 +0000 |
353 | @@ -3,10 +3,7 @@ |
354 | xmlns:tal="http://xml.zope.org/namespaces/tal" |
355 | xmlns:metal="http://xml.zope.org/namespaces/metal" |
356 | xmlns:i18n="http://xml.zope.org/namespaces/i18n" |
357 | - xml:lang="en" |
358 | - lang="en" |
359 | - dir="ltr" |
360 | - metal:use-macro="view/macro:page/onecolumn" |
361 | + metal:use-macro="view/macro:page/main_only" |
362 | i18n:domain="launchpad" |
363 | > |
364 | <body> |
365 | @@ -39,6 +36,8 @@ |
366 | |
367 | <metal:translation-import-queue |
368 | use-macro="view/translation_import_queue_content" /> |
369 | + |
370 | + <tal:menu replace="structure context/@@+related-pages" /> |
371 | </div> |
372 | </body> |
373 | </html> |
374 | |
375 | === modified file 'lib/lp/translations/templates/person-translations-relicensing.pt' |
376 | --- lib/lp/translations/templates/person-translations-relicensing.pt 2009-07-17 17:59:07 +0000 |
377 | +++ lib/lp/translations/templates/person-translations-relicensing.pt 2009-09-01 09:51:12 +0000 |
378 | @@ -3,37 +3,35 @@ |
379 | xmlns:tal="http://xml.zope.org/namespaces/tal" |
380 | xmlns:metal="http://xml.zope.org/namespaces/metal" |
381 | xmlns:i18n="http://xml.zope.org/namespaces/i18n" |
382 | - xml:lang="en" |
383 | - lang="en" |
384 | - dir="ltr" |
385 | - metal:use-macro="view/macro:page/onecolumn" |
386 | + metal:use-macro="view/macro:page/main_only" |
387 | i18n:domain="launchpad" |
388 | > |
389 | <body> |
390 | <div metal:fill-slot="main"> |
391 | - <h1>Sorry to interrupt…</h1> |
392 | - <p> |
393 | - When you make a translation in Launchpad, it's available for use |
394 | - by all projects that use Launchpad. |
395 | - </p> |
396 | - <p> |
397 | - So that the maximum number of projects can benefit from this, |
398 | - we ask that you agree to <strong>license all your translations |
399 | - using the |
400 | - <a href="https://help.launchpad.net/Legal/BSDLicense">BSD licence</a></strong>. |
401 | - If you change your mind later, you can return to this page |
402 | - (“Translations licensing”) at any time. |
403 | - </p> |
404 | - <p> |
405 | - If you do not agree to the BSD licence, you will not be able |
406 | - to make translations in Launchpad. |
407 | - </p> |
408 | - <p> |
409 | - For more about why we ask this from you, see our |
410 | - <a href="http://help.launchpad.net/Translations/LicensingFAQ" |
411 | - >translations licensing FAQ</a>. |
412 | - </p> |
413 | - <div metal:use-macro="context/@@launchpad_form/form" /> |
414 | + <div> |
415 | + <h1>Sorry to interrupt…</h1> |
416 | + <p> |
417 | + When you make a translation in Launchpad, it's available for use |
418 | + by all projects that use Launchpad. |
419 | + </p> |
420 | + <p> |
421 | + To let the maximum number of projects benefit from this, we ask |
422 | + that you agree to <strong>license all your translations under the |
423 | + <a href="https://help.launchpad.net/Legal/BSDLicense">BSD licence</a></strong>. |
424 | + If you change your mind later, you can return to this page |
425 | + (“Translations licensing”) at any time. |
426 | + </p> |
427 | + <p> |
428 | + If you do not agree to the BSD licence, you will not be able |
429 | + to make translations in Launchpad. |
430 | + </p> |
431 | + <p> |
432 | + For more about why we ask this from you, see our |
433 | + <a href="http://help.launchpad.net/Translations/LicensingFAQ" |
434 | + >translations licensing FAQ</a>. |
435 | + </p> |
436 | + <div metal:use-macro="context/@@launchpad_form/form" /> |
437 | + </div> |
438 | </div> |
439 | </body> |
440 | |
441 | |
442 | === modified file 'lib/lp/translations/templates/person-translations.pt' |
443 | --- lib/lp/translations/templates/person-translations.pt 2009-08-28 08:23:04 +0000 |
444 | +++ lib/lp/translations/templates/person-translations.pt 2009-09-01 08:37:31 +0000 |
445 | @@ -3,17 +3,17 @@ |
446 | xmlns:tal="http://xml.zope.org/namespaces/tal" |
447 | xmlns:metal="http://xml.zope.org/namespaces/metal" |
448 | xmlns:i18n="http://xml.zope.org/namespaces/i18n" |
449 | - xml:lang="en" |
450 | - lang="en" |
451 | - dir="ltr" |
452 | - metal:use-macro="view/macro:page/onecolumn" |
453 | + metal:use-macro="view/macro:page/main_only" |
454 | i18n:domain="launchpad" |
455 | > |
456 | |
457 | <body> |
458 | |
459 | +<div metal:fill-slot="heading"> |
460 | + <h1>Translations related to <tal:name content="context/displayname" /></h1> |
461 | +</div> |
462 | + |
463 | <div metal:fill-slot="main"> |
464 | - <h1>Translations related to <tal:name content="context/displayname" /></h1> |
465 | |
466 | <tal:me condition="view/person_includes_me"> |
467 | <tal:reviewer condition="view/person_is_reviewer"> |
468 | @@ -58,12 +58,12 @@ |
469 | <tal:navigation content="structure view/batchnav/@@+navigation-links-upper" /> |
470 | </div> |
471 | |
472 | - <table class="listing"> |
473 | + <table class="listing" style="max-width:800px"> |
474 | <thead> |
475 | <tr> |
476 | <th>Translation file</th> |
477 | <th>Untranslated</th> |
478 | - <th>Needs review</th> |
479 | + <th>Need review</th> |
480 | <th>Latest translation</th> |
481 | </tr> |
482 | </thead> |
483 | @@ -125,33 +125,38 @@ |
484 | <span tal:replace="context/displayname">Mark Shuttleworth</span> is |
485 | a member of the following translation groups: |
486 | |
487 | - <table class="listing" id="translation-group-memberships"> |
488 | - <thead> |
489 | - <tr> |
490 | - <th>Translation group</th> |
491 | - <th>Language</th> |
492 | - <th>Translation guidelines</th> |
493 | - </tr> |
494 | - </thead> |
495 | - <tbody> |
496 | - <tr tal:repeat="translator view/translators"> |
497 | - <td><a tal:attributes="href translator/translationgroup/fmt:url" |
498 | - tal:content="translator/translationgroup/title"></a></td> |
499 | - <td tal:content="translator/language/englishname">Esperanto</td> |
500 | - <td><a tal:condition="translator/style_guide_url" |
501 | - tal:content="translator/style_guide_url" |
502 | - tal:attributes="href translator/style_guide_url" |
503 | - >Documentation URL</a> |
504 | - <a tal:attributes="href string:${translator/translationgroup/fmt:url}/${translator/language/code}/+edit" |
505 | - ><img src="/@@/edit" /></a></td> |
506 | - </tr> |
507 | + <table class="listing" |
508 | + id="translation-group-memberships" |
509 | + style="max-width:800px"> |
510 | + <thead> |
511 | + <tr> |
512 | + <th>Translation group</th> |
513 | + <th>Language</th> |
514 | + <th>Translation guidelines</th> |
515 | + </tr> |
516 | + </thead> |
517 | + <tbody> |
518 | + <tr tal:repeat="translator view/translators"> |
519 | + <td> |
520 | + <a tal:replace="structure translator/translationgroup/fmt:link"> |
521 | + Launchpad Translators |
522 | + </a> |
523 | + </td> |
524 | + <td tal:content="translator/language/englishname">Esperanto</td> |
525 | + <td><a tal:condition="translator/style_guide_url" |
526 | + tal:content="translator/style_guide_url" |
527 | + tal:attributes="href translator/style_guide_url" |
528 | + >Documentation URL</a> |
529 | + <a tal:attributes="href string:${translator/translationgroup/fmt:url}/${translator/language/code}/+edit" |
530 | + ><img src="/@@/edit" /></a></td> |
531 | + </tr> |
532 | </tbody> |
533 | </table> |
534 | |
535 | </tal:block> |
536 | |
537 | + <tal:menu replace="structure context/@@+related-pages" /> |
538 | + |
539 | </div> |
540 | - |
541 | - |
542 | </body> |
543 | </html> |
544 | |
545 | === modified file 'lib/lp/translations/templates/pofile-filter.pt' |
546 | --- lib/lp/translations/templates/pofile-filter.pt 2009-07-17 17:59:07 +0000 |
547 | +++ lib/lp/translations/templates/pofile-filter.pt 2009-09-01 10:56:28 +0000 |
548 | @@ -3,10 +3,7 @@ |
549 | xmlns:tal="http://xml.zope.org/namespaces/tal" |
550 | xmlns:metal="http://xml.zope.org/namespaces/metal" |
551 | xmlns:i18n="http://xml.zope.org/namespaces/i18n" |
552 | - xml:lang="en" |
553 | - lang="en" |
554 | - dir="ltr" |
555 | - metal:use-macro="context/@@main_template/master" |
556 | + metal:use-macro="view/macro:page/main_only" |
557 | i18n:domain="launchpad" |
558 | > |
559 | <metal:heading fill-slot="head_epilogue"> |
560 | @@ -22,40 +19,7 @@ |
561 | </metal:heading> |
562 | |
563 | <body> |
564 | - |
565 | -<metal:portlets fill-slot="portlets"> |
566 | - |
567 | - <div tal:replace="structure context/@@+portlet-details" /> |
568 | - <div tal:replace="structure context/@@+portlet-stats" /> |
569 | - |
570 | -</metal:portlets> |
571 | - |
572 | - <div metal:fill-slot="help"> |
573 | - |
574 | - <p>This page shows a list of all translations a user has submitted |
575 | - for a certain PO file. It includes both currently used and |
576 | - unused suggestions, including those which are now hidden.</p> |
577 | - |
578 | - <dl> |
579 | - <dt class="usedtranslation">Bold:</dt> |
580 | - <dd>Indicates currently used submissions.</dd> |
581 | - <dt>Regular:</dt> |
582 | - <dd>Indicates unused, recent suggestion.</dd> |
583 | - <dt class="hiddentranslation">Gray</dt> |
584 | - <dd>Indicates hidden, unused suggestion (hidden are those |
585 | - which have been reviewed but rejected).</dd> |
586 | - </dl> |
587 | - </div> |
588 | - |
589 | <div metal:fill-slot="main"> |
590 | - |
591 | - <div> |
592 | - <a tal:attributes="href context/fmt:url" |
593 | - tal:content="context/title"> |
594 | - evolution 2.10 translation |
595 | - </a> |
596 | - </div> |
597 | - |
598 | <h1 tal:condition="view/person">Contributions by |
599 | <tal:contributor content="view/person/displayname"/> |
600 | </h1> |
601 | @@ -68,12 +32,15 @@ |
602 | |
603 | <tal:translations condition="messages"> |
604 | <p tal:condition="view/person"> |
605 | - <tal:contributor content="view/person/displayname"/> has |
606 | - submitted following translations. |
607 | + <a tal:replace="structure view/person/fmt:link"/> has |
608 | + submitted the following strings to this translation. |
609 | + Contributions are visually coded: |
610 | + <span class="usedtranslation">currently used translations</span>, |
611 | + <span>unreviewed suggestions</span>, |
612 | + <span class="hiddentranslation">rejected suggestions</span>. |
613 | </p> |
614 | |
615 | - |
616 | - <div class="lesser"> |
617 | + <div> |
618 | <tal:navigation content=" |
619 | structure view/batchnav/@@+navigation-links-upper" /> |
620 | </div> |
621 | @@ -114,6 +81,7 @@ |
622 | |
623 | </tal:translations> |
624 | </tal:block> |
625 | + <tal:menu replace="structure context/@@+related-pages" /> |
626 | </div> |
627 | </body> |
628 | </html> |
629 | |
630 | === modified file 'lib/lp/translations/templates/translation-import-queue-macros.pt' |
631 | --- lib/lp/translations/templates/translation-import-queue-macros.pt 2009-07-17 17:59:07 +0000 |
632 | +++ lib/lp/translations/templates/translation-import-queue-macros.pt 2009-09-01 09:51:12 +0000 |
633 | @@ -90,13 +90,11 @@ |
634 | <a tal:attributes="href entry/content/http_url" |
635 | tal:content="entry/path">po/foo.pot</a> in |
636 | <a tal:condition="entry/sourcepackage" |
637 | - tal:content="entry/sourcepackage/title" |
638 | - tal:attributes="href entry/sourcepackage/fmt:url" |
639 | - >evolution in Ubuntu Hoary</a> |
640 | + tal:replace="structure entry/sourcepackage/fmt:link"> |
641 | + evolution in Ubuntu Hoary</a> |
642 | <a tal:condition="entry/productseries" |
643 | - tal:content="entry/productseries/title" |
644 | - tal:attributes="href entry/productseries/fmt:url" |
645 | - >Evolution Series: MAIN</a> |
646 | + tal:replace="structure entry/productseries/fmt:link"> |
647 | + Evolution Series: MAIN</a> |
648 | </td> |
649 | <td tal:condition="entry/required:launchpad.Edit"> |
650 | <tal:status define="name string:status_${entry/id}" |
651 | @@ -128,9 +126,9 @@ |
652 | <tr class="discreet secondary"> |
653 | <td colspan="3"> |
654 | Uploaded by |
655 | - <a tal:content="entry/importer/displayname" |
656 | - tal:attributes="href entry/importer/fmt:url" |
657 | - >Mark Shuttleworth</a> |
658 | + <a tal:replace="structure entry/importer/fmt:link"> |
659 | + Mark Shuttleworth |
660 | + </a> |
661 | on <span tal:content="entry/dateimported/fmt:datetime">date</span> |
662 | </td> |
663 | </tr> |
664 | |
665 | === modified file 'lib/lp/translations/templates/translationgroups-index.pt' |
666 | --- lib/lp/translations/templates/translationgroups-index.pt 2009-08-25 12:03:26 +0000 |
667 | +++ lib/lp/translations/templates/translationgroups-index.pt 2009-09-01 15:08:42 +0000 |
668 | @@ -45,10 +45,7 @@ |
669 | <tbody> |
670 | <tr tal:repeat="group context"> |
671 | <td> |
672 | - <a |
673 | - tal:content="group/title/fmt:shorten/57" |
674 | - tal:attributes="href group/name" |
675 | - >Ubuntu Translators</a> |
676 | + <a tal:replace="structure group/fmt:link">Ubuntu Translators</a> |
677 | </td> |
678 | <td> |
679 | <tal:projects condition="group/top_projects"> |
680 | |
681 | === modified file 'lib/lp/translations/templates/translationimportqueue-index.pt' |
682 | --- lib/lp/translations/templates/translationimportqueue-index.pt 2009-07-17 17:59:07 +0000 |
683 | +++ lib/lp/translations/templates/translationimportqueue-index.pt 2009-09-01 09:51:12 +0000 |
684 | @@ -3,10 +3,7 @@ |
685 | xmlns:tal="http://xml.zope.org/namespaces/tal" |
686 | xmlns:metal="http://xml.zope.org/namespaces/metal" |
687 | xmlns:i18n="http://xml.zope.org/namespaces/i18n" |
688 | - xml:lang="en" |
689 | - lang="en" |
690 | - dir="ltr" |
691 | - metal:use-macro="view/macro:page/onecolumn" |
692 | + metal:use-macro="view/macro:page/main_only" |
693 | i18n:domain="launchpad"> |
694 | <body> |
695 | <metal:css fill-slot="head_epilogue"> |
= Bug 422462 =
These are mostly basic, semi-mechanical migrations of the person-related
Translations pages to the 3.0 templates.
Most of these are very simple: there are very few actions on these pages
and the 2.0 changes left them without any portlets that might be missed.
Two navigation menus are involved (for Person and POFile), and some
groundwork having been laid already, reusing those as related-pages
menus was entirely painless in both cases.
== Details in diff order ==
Some page titles were moved into views. Saved some trouble with the translations view, which in a way has two contexts: a Person
filtered-
whose translations you're viewing and the POFile that those translations
are in.
Since we were still painstakingly composing links to translation groups,
I bit the bullet and created a formatter for them. Used on the personal
Translations page and the translation groups index. Surprisingly easy;
no idea why we never did this before.
In the relicensing view, at the bottom of browser/person.py, I removed
the sentence telling a user after declining to BSD-license their
translations that their existing translations will be removed. We were
going to do that during the BSD licensing migration, and we did. We're
not planning to do that any more now that everyone's had ample time to
choose.
In test_translatio nlinksaggregato r.py I fixed a test that I stupidly
left brittle, depending on the order of a list that isn't actually in a
guaranteed order. Others have run into this problem with the test as
well, but this instance was still present in trunk.
In TranslationImpo rtQueueView, the initialize method can now safely
re-use its parent method instead of duplicating its work. That's why
_initial_values is no longer set there: the parent does it.
In TranslationReli censingAgreemen tOptions, I shortened the option labels
after discussion with noodles, mpt, and danilo. It all started with the
labels having a "nowrap" style and so happily stretching out of a narrow
browser window.
Almost all the pages I edited got a "related pages" section at the
bottom in return for their old navigation menu. The exception is the
relicensing page, which reflects a setting and so got a Cancel link
instead. It leads back to the person's main translations page.
On the relicensing page I also took the opportunity to update more of
the wording. Some of it struck me as a bit awkward; the changes are not
very big.
Then there's pofile-filter.pt. It needed a bit more work: there was
popup help that explained how different CSS styles on the page mark
different kinds of messages. That is inherently not self-evident no
matter how much else you know about the application, and easily
forgotten unless you use that page very frequently (which most people
who use it probably don't). On the other hand there really wasn't
enough extra information to justify a separate "page." So I turned it
into a quick inline sentence.
The translation- import- queue-macros. pt template is not a page, so it
didn't strictly need updating to 3.0. But it missed some chances for an
easy fmt:link so I fixed that.
== Tests ==
I mostly just kept running the Translations stories and...