Merge lp:~adiroiban/launchpad/bug-359180-take-2 into lp:launchpad
- bug-359180-take-2
- Merge into devel
Status: | Merged | ||||||||
---|---|---|---|---|---|---|---|---|---|
Approved by: | Graham Binns | ||||||||
Approved revision: | no longer in the source branch. | ||||||||
Merged at revision: | not available | ||||||||
Proposed branch: | lp:~adiroiban/launchpad/bug-359180-take-2 | ||||||||
Merge into: | lp:launchpad | ||||||||
Diff against target: |
802 lines (+446/-68) 11 files modified
lib/canonical/launchpad/javascript/translations/pofile.js (+306/-2) lib/canonical/launchpad/templates/batchnavigator-navigation-links.pt (+4/-0) lib/lp/translations/browser/pofile.py (+20/-2) lib/lp/translations/browser/tests/pofile-views.txt (+20/-2) lib/lp/translations/browser/translationmessage.py (+24/-0) lib/lp/translations/stories/standalone/xx-pofile-translate-newlines-check.txt (+2/-2) lib/lp/translations/stories/standalone/xx-pofile-translate.txt (+33/-5) lib/lp/translations/templates/currenttranslationmessage-translate-one.pt (+8/-11) lib/lp/translations/templates/pofile-translate.pt (+10/-43) lib/lp/translations/templates/translationmessage-translate.pt (+7/-0) lib/lp/translations/templates/translations-macros.pt (+12/-1) |
||||||||
To merge this branch: | bzr merge lp:~adiroiban/launchpad/bug-359180-take-2 | ||||||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Graham Binns (community) | code, js | Approve | |
Review via email: mp+20122@code.launchpad.net |
This proposal supersedes a proposal from 2010-01-20.
Commit message
Add keybindings on translating pofiles or translation messages.
Description of the change
= Bug 35180 =
Would be useful if the First - Previous - Next - Last link in the translation interface had a keyboard shortcut for fast accessing them. Don't need to be visible, but at least documented somewhere.
Maybe even the Save & Continue button.
== Proposed Fix ==
Add the following keybindings on +translate page for pofile and translaitonmessage
First field is autofocused.
Shift+Alt+b - Focus first translation field
Shift+Alt+a - First page
Shift+Alt+n - Next page
Shift+Alt+p - Previous page
Shift+Alt+l - Last page
Shift+Alt+s - Save and continue
Shift+Alt+Down - Next field
Shift+Alt+Up - Previous field
Shift+Alt+j - Next field
Shift+Alt+k - Previous field
Shift+Alt+C - Copy original text (both singular and plural)
Shift+Alt+0 - Mark current translation
Shift+Alt+NUMBER - Mark suggestion NUMBER
Shift+Alt+d - Dismiss all suggestions
Shift+Alt+r - Tick "Someone should review this translation"
== Pre-implementation notes ==
I talked with Danilo about keybindng implementation using comments for bug 35180
Previous MPs:
https:/
https:/
Part of this branch was already approved in previous MPs, but I have refactored most of those functions.
== Implementation notes ==
The legacy JS code from canonical/
Those functions from lp.js was rewritten using YUI3. Maybe we can no delete them from lp.js.
I have added a test for Bug #513625, since that bug was produces by some changes in this branch.
== Tests ==
I was not able to produce a reasonable windmill test since it is not trivial to find the current focused node or to see if a node is focused.
This requires adding onFocus and onBlur trigger for all DOM nodes.
./bin/test -t pofile-views
== Demo and QA ==
Log in as admin or as a person with rights on adding translations to a pofile.
Go to a pofile translate page:
https:/
The first field should have focus and you can start translating right away.
Adding a new translation should automatically select the radio button in front of it.
Try the keybindings described in the "Proposed fix" section.
= Launchpad lint =
Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.
Linting changed files:
lib/canonical
== JSLint notices ==
No handlers could be found for logger "bzr"
jslint: No problem found in '/home/
jslint: 1 file to lint.
Adi Roiban (adiroiban) wrote : Posted in a previous version of this proposal | # |
Paul Hummer (rockstar) wrote : Posted in a previous version of this proposal | # |
Please change line 140 of this diff so that the page tests conform with the 80 character line rule. Other than that, thanks for looking into this.
Adi Roiban (adiroiban) wrote : | # |
Due to bug 513625, the original branch was removed from lp/devel.
I have added a testcase for bug 513625 and I'm resubmitting this branch for review.
Graham Binns (gmb) wrote : | # |
After talking with Adi, it's clear that the branch is actually an update to an older branch which was landed on devel and then pulled before 10.01 was released due to issue with the keybindings. However, I'm unable to get a sane diff for the changes since then.
Adi, in order to be able to review this branch I need a sane diff for it. At the moment I can't get one because I get a bunch of conflicts when trying to merge it into devel. Here's how you can produce one for me:
1. Branch devel.
2. Merge your original branch into your branch of devel.
3. Merge this branch and paste the diff into the merge proposal.
That should give us just the changes that you've made since your last branch was approved; I'll be happy to review the new diff.
Adi Roiban (adiroiban) wrote : | # |
Hi,
$ bzr branch devel merge
$ bzr branch lp:~adiroiban/launchpad/bug-359180/ merge-1
$ cd merge
$ bzr merge ../merge-1/
Nothing to do.
The original branch is part of devel... just that a new commit is reverting those changes. There was no "hard rebase" to revert the branch.
I can not get a clean diff.
Is there an option for a filter to diff to only show lines changed be me?
Maybe it will be easier to have a new full review.
Adi Roiban (adiroiban) wrote : | # |
Hi,
This is the diff.
It is still quite big as the original branch was submitted 2 months ago and was one of my first patch for LP.
Meanwhile I discovered the JavaScript guidelines and looking again at the code I found many places where the previous code could be improved.
Please let me know if there are any other actions I need to do to make this branch ready for review?
=== modified file 'lib/canonical/
--- lib/canonical/
+++ lib/canonical/
@@ -1,7 +1,7 @@
/** Copyright (c) 2009, Canonical Ltd. All rights reserved.
*
* @module lp.pofile
- * @requires event, node
+ * @requires anim, cookie, event-key, event, node
*/
YUI.add(
@@ -42,8 +42,48 @@
};
+var hide_notification = function(node) {
+ var hide_anim = new Y.Anim({
+ node: node,
+ to: { height: 0,
+ marginTop: 0, marginBottom: 0,
+ paddingTop: 0, paddingBottom: 0 }
+ });
+ node.setStyle(
+ hide_anim.
+ hide_anim.on('end', function(e) {
+ node.setStyle(
+ });
+ hide_anim.run();
+}
+
+self.updateNot
+ var notice = Y.one('
+ var balloon = notice.
+ var dismiss_
+ documentation_
+
+ // Check the cookie to see if the user has already dismissed
+ // the notification box for this session.
+ var already_seen = Y.Cookie.
+ if (already_seen) {
+ notice.
+ }
+
+ var cancel_button = notice.one(
+ '.important-
+ // Cancel button starts out hidden. If user has JavaScript,
+ // then we want to show it.
+ cancel_
+ cancel_
+ e.halt();
+ hide_notificati
+ Y.Cookie.
+ });
+};
+
+
self.setFocus = function(field) {
- //Y.log(e.type + ":" + e.keyCode + ': ' + field);
// if there is nofield, do nothing
if (Y.one('#' + field)) {
Y.one('#' + field).focus();
@@ -75,7 +115,7 @@
// The replacement regex strips all tags from the html.
to.
- selectWidget(
+ selectWidgetByI
};
@@ -97,7 +137,6 @@
var singular_select = translation_stem + '_translation_
var translation_
var translation_plural = translation_stem + '_translation_';
- //Y.log(e.type + ":" + e.keyCode + ': ' + singular_select);
// Copy singular text
copyOrigin
@@ -112,43 +151,102 @@
};
-var selectWidget = function(widget) {
- if (Y.one('#' + widget)) {
- Y.one('#' + widget)
Graham Binns (gmb) wrote : | # |
Hi Adi,
I really like this branch; good job! I'm not entirely happy about the lack of Windmill tests, but I understand what you mean about it being non-trivial to write one for this case. Please file a bug about adding some Windmill tests for this, detailing the difficulties you had. It might be worth mailing the launchpad-dev list to see if anyone has any suggestions.
Preview Diff
1 | === modified file 'lib/canonical/launchpad/javascript/translations/pofile.js' |
2 | --- lib/canonical/launchpad/javascript/translations/pofile.js 2010-01-27 07:38:02 +0000 |
3 | +++ lib/canonical/launchpad/javascript/translations/pofile.js 2010-02-25 13:03:21 +0000 |
4 | @@ -1,7 +1,7 @@ |
5 | /** Copyright (c) 2009, Canonical Ltd. All rights reserved. |
6 | * |
7 | * @module lp.pofile |
8 | - * @requires event, node |
9 | + * @requires anim, cookie, event-key, event, node |
10 | */ |
11 | |
12 | YUI.add('lp.pofile', function(Y) { |
13 | @@ -41,4 +41,308 @@ |
14 | } |
15 | }; |
16 | |
17 | -}, "0.1", {"requires": ["event", "node"]}); |
18 | + |
19 | +var hide_notification = function(node) { |
20 | + var hide_anim = new Y.Anim({ |
21 | + node: node, |
22 | + to: { height: 0, |
23 | + marginTop: 0, marginBottom: 0, |
24 | + paddingTop: 0, paddingBottom: 0 } |
25 | + }); |
26 | + node.setStyle('border', 'none'); |
27 | + hide_anim.set('duration', 0.4); |
28 | + hide_anim.on('end', function(e) { |
29 | + node.setStyle('display', 'none'); |
30 | + }); |
31 | + hide_anim.run(); |
32 | +} |
33 | + |
34 | +self.updateNotificationBox = function(e) { |
35 | + var notice = Y.one('.important-notice-container'); |
36 | + var balloon = notice.one('.important-notice-balloon'); |
37 | + var dismiss_notice_cookie = ('translation-docs-for-' + |
38 | + documentation_cookie); |
39 | + |
40 | + // Check the cookie to see if the user has already dismissed |
41 | + // the notification box for this session. |
42 | + var already_seen = Y.Cookie.get(dismiss_notice_cookie, Boolean); |
43 | + if (already_seen) { |
44 | + notice.setStyle('display', 'none'); |
45 | + } |
46 | + |
47 | + var cancel_button = notice.one( |
48 | + '.important-notice-cancel-button'); |
49 | + // Cancel button starts out hidden. If user has JavaScript, |
50 | + // then we want to show it. |
51 | + cancel_button.setStyle('visibility', 'visible'); |
52 | + cancel_button.on('click', function(e) { |
53 | + e.halt(); |
54 | + hide_notification(balloon); |
55 | + Y.Cookie.set(dismiss_notice_cookie, true); |
56 | + }); |
57 | +}; |
58 | + |
59 | + |
60 | +self.setFocus = function(field) { |
61 | + // if there is nofield, do nothing |
62 | + if (Y.one('#' + field)) { |
63 | + Y.one('#' + field).focus(); |
64 | + } |
65 | +}; |
66 | + |
67 | + |
68 | +var setNextFocus = function(e, field) { |
69 | + self.setFocus(field); |
70 | + // stopPropagation() and preventDefault() |
71 | + e.halt(); |
72 | +}; |
73 | + |
74 | + |
75 | +var setPreviousFocus = function(e, field, original) { |
76 | + |
77 | + // Original singular test is focused first to make sure |
78 | + // it is visible when scrolling up |
79 | + self.setFocus(original); |
80 | + self.setFocus(field); |
81 | + // stopPropagation() and preventDefault() |
82 | + e.halt(); |
83 | +}; |
84 | + |
85 | + |
86 | +var copyOriginalTextOne = function(from_id, to_id, select_id) { |
87 | + var from = Y.one('#' + from_id); |
88 | + var to = Y.one('#' + to_id); |
89 | + // The replacement regex strips all tags from the html. |
90 | + to.set('value', unescapeHTML( |
91 | + from.get('innerHTML').replace(/<\/?[^>]+>/gi, ""))); |
92 | + selectWidgetByID(select_id); |
93 | +}; |
94 | + |
95 | + |
96 | +var copyOriginalTextPlural = function (from_id, |
97 | + to_id_pattern, nplurals) { |
98 | + // skip when x is 0, as that is the singular |
99 | + for (var x = 1; x < nplurals; x++) { |
100 | + var to_id = to_id_pattern + x + "_new"; |
101 | + var to_select = to_id_pattern + x + "_new_select"; |
102 | + copyOriginalTextOne(from_id, to_id, to_select); |
103 | + } |
104 | +}; |
105 | + |
106 | + |
107 | +var copyOriginalTextAll = function(e, original_stem, translation_stem) { |
108 | + |
109 | + var original_singular = original_stem + '_singular'; |
110 | + var original_plural = original_stem + '_plural'; |
111 | + var singular_select = translation_stem + '_translation_0_new_select'; |
112 | + var translation_singular = translation_stem + '_translation_0_new'; |
113 | + var translation_plural = translation_stem + '_translation_'; |
114 | + // Copy singular text |
115 | + copyOriginalTextOne( |
116 | + original_singular, translation_singular, singular_select); |
117 | + |
118 | + // Copy plural text if needed |
119 | + if (Y.one('#' + translation_plural + '1')) { |
120 | + copyOriginalTextPlural( |
121 | + original_plural, translation_plural, plural_forms); |
122 | + } |
123 | + // stopPropagation() and preventDefault() |
124 | + e.halt(); |
125 | +}; |
126 | + |
127 | + |
128 | +var selectWidgetByID = function(widget) { |
129 | + var node = Y.one('#' + widget); |
130 | + if (node) { |
131 | + node.set('checked', true); |
132 | + } |
133 | +}; |
134 | + |
135 | + |
136 | +var toggleWidget = function(widget) { |
137 | + var node = Y.one('#' + widget); |
138 | + if (node) { |
139 | + if (node.get('checked')) { |
140 | + node.set('checked', false); |
141 | + } else { |
142 | + node.set('checked', true); |
143 | + } |
144 | + } |
145 | +}; |
146 | + |
147 | + |
148 | +var selectTranslation = function(e, widget) { |
149 | + // Don't select when tabbing, navigating and simply pressing |
150 | + // enter to submit the form. |
151 | + // Looks like this is not needed for Epiphany and Chromium |
152 | + if (e.keyCode == 9 || e.keyCode == 13 || |
153 | + e.keyCode == 38 || e.keyCode == 40 || |
154 | + (e.shiftKey && e.altKey && e.keyCode == 74) || |
155 | + (e.shiftKey && e.altKey && e.keyCode == 75)) { |
156 | + return; |
157 | + } |
158 | + selectWidgetByID(widget); |
159 | +}; |
160 | + |
161 | + |
162 | +var initializeGlobalKeyBindings = function(fields) { |
163 | + |
164 | + Y.get('document').on("keyup", function(e) { |
165 | + // Shift+Alt+s - Save form |
166 | + if (e.shiftKey && e.altKey && e.keyCode == 83) { |
167 | + Y.one('#save_and_continue_button').invoke('click'); |
168 | + } |
169 | + // Shift+Alt+f - Go to search field |
170 | + if (e.shiftKey && e.altKey && e.keyCode == 70) { |
171 | + self.setFocus('search_box'); |
172 | + } |
173 | + // Shift+Alt+b - Go to first translation field |
174 | + if (e.shiftKey && e.altKey && e.keyCode == 66) { |
175 | + self.setFocus(fields[0]); |
176 | + } |
177 | + // Shift+Alt+n - Go to next page in batch |
178 | + if (e.shiftKey && e.altKey && e.keyCode == 78) { |
179 | + if (link = Y.one('#batchnav_next')){ |
180 | + window.location.assign(link.get('href')) |
181 | + } |
182 | + } |
183 | + // Shift+Alt+p - Go to previous page in batch |
184 | + if (e.shiftKey && e.altKey && e.keyCode == 80) { |
185 | + if (link = Y.one('#batchnav_previous')){ |
186 | + window.location.assign(link.get('href')) |
187 | + } |
188 | + } |
189 | + // Shift+Alt+a - Go to first page in batch |
190 | + if (e.shiftKey && e.altKey && e.keyCode == 65) { |
191 | + if (link = Y.one('#batchnav_first')){ |
192 | + window.location.assign(link.get('href')) |
193 | + } |
194 | + } |
195 | + // Shift+Alt+l - Go to last page in batch |
196 | + if (e.shiftKey && e.altKey && e.keyCode == 76) { |
197 | + if (link = Y.one('#batchnav_last')){ |
198 | + window.location.assign(link.get('href')) |
199 | + } |
200 | + } |
201 | + }); |
202 | +} |
203 | + |
204 | + |
205 | +var initializeSuggestionsKeyBindings = function(stem) { |
206 | + |
207 | + suggestions = Y.all('.' + stem.replace(/_new/,"") + ' input'); |
208 | + suggestions.each(function(node) { |
209 | + // Only add keybinding for the first 9 suggestions |
210 | + var index = suggestions.indexOf(node); |
211 | + if (index < 10) { |
212 | + // Shift+Alt+NUMBER - Mark suggestion NUMBER |
213 | + Y.on('key', function(e, id) { |
214 | + selectWidgetByID(id); |
215 | + }, |
216 | + '#' + stem, 'down:' + Number(index+49) + '+shift+alt', |
217 | + Y, node.get('id')); |
218 | + } |
219 | + }); |
220 | +} |
221 | + |
222 | + |
223 | +var initializeFieldsKeyBindings = function (fields) { |
224 | + for (var key = 0; key < fields.length; key++) { |
225 | + var next = key + 1; |
226 | + var previous = key - 1; |
227 | + |
228 | + var html_parts = fields[key].split('_'); |
229 | + var original_stem = html_parts[0] + '_' + html_parts[1]; |
230 | + var translation_stem = fields[key].replace(/_translation_(\d)+_new/,""); |
231 | + var select_widget = ( |
232 | + translation_stem + '_' + html_parts[3] + '_' + |
233 | + html_parts[4] + '_new_select'); |
234 | + |
235 | + Y.on( |
236 | + 'change', selectTranslation, |
237 | + '#' + fields[key], Y, select_widget); |
238 | + Y.on( |
239 | + 'keypress', selectTranslation, |
240 | + '#' + fields[key], Y, select_widget); |
241 | + |
242 | + // Set next field and copy text for all but last field |
243 | + // (last is Save & Continue button) |
244 | + if (key < fields.length - 1) { |
245 | + // Shift+Alt+j - Go to next translation |
246 | + Y.on( |
247 | + 'key', setNextFocus, '#' + fields[key], |
248 | + 'down:74+shift+alt', Y, fields[next]); |
249 | + // Shift+Alt+KEY_DOWN - Go to next translation |
250 | + Y.on( |
251 | + 'key', setNextFocus, '#' + fields[key], |
252 | + 'down:40+shift+alt', Y, fields[next]); |
253 | + // Shift+Alt+c - Copy original text |
254 | + Y.on( |
255 | + 'key', copyOriginalTextAll, '#' + fields[key], |
256 | + 'down:67+shift+alt', Y, original_stem, translation_stem); |
257 | + |
258 | + // Shift+Alt+r - Toggle someone should review |
259 | + Y.on( |
260 | + 'key', |
261 | + function(e, stem) { |
262 | + toggleWidget(stem + '_force_suggestion'); |
263 | + }, |
264 | + '#' + fields[key], 'down:82+shift+alt', Y, original_stem); |
265 | + |
266 | + // Shift+Alt+d - Toggle dismiss all translations |
267 | + Y.on( |
268 | + 'key', function(e, stem) { |
269 | + toggleWidget(stem + '_dismiss'); |
270 | + }, '#' + fields[key], 'down:68+shift+alt', Y, original_stem); |
271 | + |
272 | + // Shift+Alt+0 - Mark current translation |
273 | + Y.on( |
274 | + 'key', function(e, key) { |
275 | + selectWidgetByID(key.replace(/_new/, "_radiobutton")); |
276 | + }, '#' + fields[key], 'down:48+shift+alt', Y, fields[key]); |
277 | + |
278 | + initializeSuggestionsKeyBindings(fields[key]); |
279 | + } |
280 | + |
281 | + // Set previous field for all but first field |
282 | + if (key > 0) { |
283 | + var parts = fields[previous].split('_'); |
284 | + var singular_copy_text = ( |
285 | + parts[0] + '_' + parts[1] + '_singular_copy_text'); |
286 | + // Shift+Alt+k - Go to previous translation |
287 | + Y.on( |
288 | + 'key', setPreviousFocus, '#' + fields[key], |
289 | + 'down:75+shift+alt', Y, fields[previous], |
290 | + singular_copy_text); |
291 | + // Shift+Alt+KEY_UP - Go to previous translation |
292 | + Y.on( |
293 | + 'key', setPreviousFocus, '#' + fields[key], |
294 | + 'down:38+shift+alt', Y, fields[previous], |
295 | + singular_copy_text); |
296 | + } |
297 | + } |
298 | +} |
299 | + |
300 | + |
301 | +/** |
302 | + * Initialize event-key bindings such as moving to the next or previous |
303 | + * field, or copying original text |
304 | + */ |
305 | +self.initializeKeyBindings = function(e) { |
306 | + |
307 | + if (translations_order.length < 1) { |
308 | + // If no translations fiels are displayed on the page |
309 | + // don't initialize the translations order |
310 | + return; |
311 | + } |
312 | + |
313 | + var fields = translations_order.split(' '); |
314 | + // The last field is Save & Continue button |
315 | + fields.push('save_and_continue_button'); |
316 | + |
317 | + initializeGlobalKeyBindings(fields); |
318 | + initializeFieldsKeyBindings(fields); |
319 | +}; |
320 | + |
321 | +}, "0.1", {"requires": ["event", "event-key", "node", "cookie", "anim"]}); |
322 | + |
323 | |
324 | === modified file 'lib/canonical/launchpad/templates/batchnavigator-navigation-links.pt' |
325 | --- lib/canonical/launchpad/templates/batchnavigator-navigation-links.pt 2010-01-27 07:38:02 +0000 |
326 | +++ lib/canonical/launchpad/templates/batchnavigator-navigation-links.pt 2010-02-25 13:03:21 +0000 |
327 | @@ -29,6 +29,7 @@ |
328 | tal:condition="first_page_url" |
329 | tal:attributes="href first_page_url" |
330 | class="first" |
331 | + id="batchnav_first" |
332 | rel="first" |
333 | >First</a> |
334 | <span tal:condition="not:first_page_url" class="first inactive" |
335 | @@ -38,6 +39,7 @@ |
336 | tal:condition="prev_page_url" |
337 | tal:attributes="href prev_page_url" |
338 | class="previous" |
339 | + id="batchnav_previous" |
340 | rel="previous" |
341 | >Previous</a> |
342 | <span tal:condition="not:prev_page_url" class="previous inactive" |
343 | @@ -46,6 +48,7 @@ |
344 | <a |
345 | tal:condition="next_page_url" |
346 | tal:attributes="href next_page_url" |
347 | + id="batchnav_next" |
348 | class="next" |
349 | rel="next" |
350 | ><strong>Next</strong></a> |
351 | @@ -57,6 +60,7 @@ |
352 | tal:condition="last_page_url" |
353 | tal:attributes="href last_page_url" |
354 | class="last" |
355 | + id="batchnav_last" |
356 | rel="last" |
357 | >Last</a> |
358 | <span tal:condition="not:last_page_url" class="last inactive" |
359 | |
360 | === modified file 'lib/lp/translations/browser/pofile.py' |
361 | --- lib/lp/translations/browser/pofile.py 2010-01-27 07:38:02 +0000 |
362 | +++ lib/lp/translations/browser/pofile.py 2010-02-25 13:03:21 +0000 |
363 | @@ -503,8 +503,6 @@ |
364 | |
365 | DEFAULT_BATCH_SIZE = 50 |
366 | |
367 | - page_title = "Contributions" |
368 | - |
369 | @property |
370 | def _person_name(self): |
371 | """Person's display name. Graceful about unknown persons.""" |
372 | @@ -930,6 +928,26 @@ |
373 | def completeness(self): |
374 | return '%.0f%%' % self.context.translatedPercentage() |
375 | |
376 | + def _messages_html_id(self): |
377 | + order = [] |
378 | + for message in self.translationmessage_views: |
379 | + if (message.form_is_writeable): |
380 | + for dictionary in message.translation_dictionaries: |
381 | + order.append( |
382 | + dictionary['html_id_translation'] + '_new') |
383 | + return order |
384 | + |
385 | + @property |
386 | + def autofocus_html_id(self): |
387 | + if (len(self._messages_html_id()) > 0): |
388 | + return self._messages_html_id()[0] |
389 | + else: |
390 | + return "" |
391 | + |
392 | + @property |
393 | + def translations_order(self): |
394 | + return ' '.join(self._messages_html_id()) |
395 | + |
396 | |
397 | class POExportView(BaseExportView): |
398 | |
399 | |
400 | === modified file 'lib/lp/translations/browser/tests/pofile-views.txt' |
401 | --- lib/lp/translations/browser/tests/pofile-views.txt 2009-09-17 16:22:32 +0000 |
402 | +++ lib/lp/translations/browser/tests/pofile-views.txt 2010-02-25 13:03:21 +0000 |
403 | @@ -145,6 +145,23 @@ |
404 | >>> translationmessage_view.context.translations |
405 | [u'libreta de direcciones de Evolution'] |
406 | |
407 | +To help the JavaScript key navigation the view is exposing the autofocus |
408 | +field and a list of all translation fields ordered by the way they are |
409 | +listed in the page. |
410 | + |
411 | + >>> for translationmessage_view in ( |
412 | + ... pofile_view.translationmessage_views): |
413 | + ... translationmessage_view.initialize() |
414 | + >>> print pofile_view.autofocus_html_id |
415 | + msgset_130_es_translation_0_new |
416 | + >>> print pofile_view.translations_order |
417 | + msgset_130_es_translation_0_new msgset_131_es_translation_0_new |
418 | + msgset_132_es_translation_0_new msgset_133_es_translation_0_new |
419 | + msgset_134_es_translation_0_new msgset_135_es_translation_0_new |
420 | + msgset_136_es_translation_0_new msgset_137_es_translation_0_new |
421 | + msgset_138_es_translation_0_new msgset_139_es_translation_0_new |
422 | + msgset_130_es_translation_0_new |
423 | + |
424 | It's time to check the submission of translations and the IPOFile statistics |
425 | update. |
426 | |
427 | @@ -403,7 +420,8 @@ |
428 | |
429 | = POFileNavigation = |
430 | |
431 | -This class is used to traverse from IPOFile objects to ITranslationMessage ones. |
432 | +This class is used to traverse from IPOFile objects to ITranslationMessage |
433 | +ones. |
434 | |
435 | >>> from zope.security.proxy import isinstance |
436 | >>> from lp.translations.browser.pofile import POFileNavigation |
437 | @@ -483,4 +501,4 @@ |
438 | And we are redirected to the index page, as expected: |
439 | |
440 | >>> print pofile_view.request.response.getHeader('Location') |
441 | - http://translations.../ubuntu/hoary/+source/evolution/+pots/evolution-2.2/es |
442 | + http://trans.../ubuntu/hoary/+source/evolution/+pots/evolution-2.2/es |
443 | |
444 | === modified file 'lib/lp/translations/browser/translationmessage.py' |
445 | --- lib/lp/translations/browser/translationmessage.py 2010-01-27 07:38:02 +0000 |
446 | +++ lib/lp/translations/browser/translationmessage.py 2010-02-25 13:03:21 +0000 |
447 | @@ -829,6 +829,30 @@ |
448 | self._redirectToNextPage() |
449 | return True |
450 | |
451 | + def _messages_html_id(self): |
452 | + order = [] |
453 | + message = self.translationmessage_view |
454 | + # If we don't know about plural forms, or there are some other |
455 | + # reason that prevent translations, translationmessage_view is |
456 | + # not created |
457 | + if ((message is not None) and (message.form_is_writeable)): |
458 | + for dictionary in message.translation_dictionaries: |
459 | + order.append( |
460 | + dictionary['html_id_translation'] + '_new') |
461 | + return order |
462 | + |
463 | + @property |
464 | + def autofocus_html_id(self): |
465 | + if (len(self._messages_html_id()) > 0): |
466 | + return self._messages_html_id()[0] |
467 | + else: |
468 | + return "" |
469 | + |
470 | + @property |
471 | + def translations_order(self): |
472 | + return ' '.join(self._messages_html_id()) |
473 | + |
474 | + |
475 | class CurrentTranslationMessageView(LaunchpadView): |
476 | """Holds all data needed to show an ITranslationMessage. |
477 | |
478 | |
479 | === modified file 'lib/lp/translations/stories/standalone/xx-pofile-translate-newlines-check.txt' |
480 | --- lib/lp/translations/stories/standalone/xx-pofile-translate-newlines-check.txt 2010-01-27 07:38:02 +0000 |
481 | +++ lib/lp/translations/stories/standalone/xx-pofile-translate-newlines-check.txt 2010-02-25 13:03:21 +0000 |
482 | @@ -43,7 +43,7 @@ |
483 | >>> print browser.url |
484 | http://translations.launchpad.dev/ubuntu/hoary/+source/evolution/+pots/evolution-2.2/es/+translate?start=19&batch=1 |
485 | >>> print find_tag_by_id(browser.contents, 'msgset_149_es_translation_0_new') |
486 | - <textarea ... name="msgset_149_es_translation_0_new" ...> |
487 | + <textarea ... name="msgset_149_es_translation_0_new"...> |
488 | |
489 | foo |
490 | |
491 | @@ -63,7 +63,7 @@ |
492 | >>> print find_tag_by_id( |
493 | ... browser.contents, |
494 | ... 'msgset_149_es_translation_0_new') #doctest: -NORMALIZE_WHITESPACE |
495 | - <textarea ... name="msgset_149_es_translation_0_new" ...> |
496 | + <textarea ... name="msgset_149_es_translation_0_new"...> |
497 | foo</textarea> |
498 | |
499 | Now, Check that even though the user forgot the trailing new line char, |
500 | |
501 | === modified file 'lib/lp/translations/stories/standalone/xx-pofile-translate.txt' |
502 | --- lib/lp/translations/stories/standalone/xx-pofile-translate.txt 2010-01-27 07:38:02 +0000 |
503 | +++ lib/lp/translations/stories/standalone/xx-pofile-translate.txt 2010-02-25 13:03:21 +0000 |
504 | @@ -30,7 +30,8 @@ |
505 | |
506 | The page is rendered in read-only mode, without any textareas for input. |
507 | |
508 | - >>> main_content = find_tag_by_id(browser.contents, 'messages_to_translate') |
509 | + >>> main_content = find_tag_by_id( |
510 | + ... browser.contents, 'messages_to_translate') |
511 | >>> for textarea in main_content.findAll('textarea'): |
512 | ... print 'Found textarea:\n%s' % textarea |
513 | |
514 | @@ -145,8 +146,8 @@ |
515 | |
516 | >>> browser = setupBrowser(auth='Basic carlos@canonical.com:test') |
517 | >>> browser.open("http://translations.launchpad.dev/" |
518 | - ... "ubuntu/hoary/+source/evolution/+pots/evolution-2.2/en_AU/" |
519 | - ... "+translate?field.alternative_language=es") |
520 | + ... "ubuntu/hoary/+source/evolution/+pots/evolution-2.2" |
521 | + ... "/en_AU/+translate?field.alternative_language=es") |
522 | |
523 | Elements related 1:1 to a translatable message on this form have names and |
524 | identifiers constructed as "msgset_<id>," where <id> is the unpadded decimal |
525 | @@ -158,7 +159,7 @@ |
526 | ... print id |
527 | msgset_130 |
528 | ... |
529 | - msgset_130_singular |
530 | + msgset_130_singular... |
531 | |
532 | HTML element identifiers for suggestions and translations on this form are |
533 | constructed as an underscore-separated sequence of: |
534 | @@ -180,7 +181,9 @@ |
535 | msgset_130_es_suggestion_562_0 |
536 | msgset_130_es_suggestion_562_0_origin |
537 | msgset_130_es_suggestion_562_0_radiobutton |
538 | + msgset_130_force_suggestion |
539 | msgset_130_singular |
540 | + msgset_130_singular_copy_text |
541 | |
542 | Radio buttons are grouped by their name attribute. The translate page shows |
543 | each translatable message with one radiobutton to select the existing |
544 | @@ -190,7 +193,9 @@ |
545 | Here we see an example where three suggestions are offered, making for five |
546 | identically-named radio buttons and sundry other HTML tags. |
547 | |
548 | - >>> browser.open('http://translations.launchpad.dev/alsa-utils/trunk/+pots/alsa-utils/es/+translate') |
549 | + >>> browser.open( |
550 | + ... 'http://translations.launchpad.dev/alsa-utils/trunk/' |
551 | + ... '+pots/alsa-utils/es/+translate') |
552 | >>> msgset_198 = get_tags(browser, 'name', 'msgset_198') |
553 | >>> for name in msgset_198: |
554 | ... print name |
555 | @@ -213,3 +218,26 @@ |
556 | ... browser.contents, 'msgset_134_es_suggestion_694_0')) |
557 | tarjetas |
558 | |
559 | + |
560 | +Missing plural forms information |
561 | +-------------------------------- |
562 | + |
563 | +If the plural forms are not known for a language, users can not add |
564 | +new translations and are asked to help Launchpad Translations by providing |
565 | +the plural form informations. |
566 | + |
567 | +This notice is display when doing batch translations or translating a |
568 | +single message. |
569 | + |
570 | + >>> browser.open('http://translations.launchpad.dev/ubuntu/hoary/' |
571 | + ... '+source/evolution/+pots/evolution-2.2/ab/+translate') |
572 | + >>> print extract_text(find_tag_by_id( |
573 | + ... browser.contents, 'maincontent')) |
574 | + Launchpad can’t handle the plural items ... |
575 | + |
576 | + >>> browser.open('http://translations.launchpad.dev/ubuntu/hoary/' |
577 | + ... '+source/evolution/+pots/evolution-2.2/ab/5/+translate') |
578 | + >>> print extract_text(find_tag_by_id( |
579 | + ... browser.contents, 'maincontent')) |
580 | + Launchpad can’t handle the plural items ... |
581 | + |
582 | |
583 | === modified file 'lib/lp/translations/templates/currenttranslationmessage-translate-one.pt' |
584 | --- lib/lp/translations/templates/currenttranslationmessage-translate-one.pt 2010-01-27 07:38:02 +0000 |
585 | +++ lib/lp/translations/templates/currenttranslationmessage-translate-one.pt 2010-02-25 13:03:21 +0000 |
586 | @@ -42,6 +42,7 @@ |
587 | <a href="" |
588 | tal:condition="not:context/potmsgset/is_translation_credit" |
589 | tal:attributes=" |
590 | + id string:${view/html_id}_singular_copy_text; |
591 | onClick string: |
592 | javascript:copyInnerHTMLById( |
593 | '${view/html_id}_singular', |
594 | @@ -172,7 +173,7 @@ |
595 | <tal:translation-dictionaries |
596 | repeat="translation_dictionary view/translation_dictionaries"> |
597 | <tal:plural-form define="plural_index translation_dictionary/plural_index"> |
598 | - <tr class="secondary translation"> |
599 | + <tr tal:attributes="class string:secondary translation ${view/html_id}"> |
600 | <th colspan="3"> |
601 | <label class="language-code">Current |
602 | <span tal:replace="context/pofile/language/englishname"> |
603 | @@ -321,9 +322,10 @@ |
604 | </td> |
605 | </tal:locked> |
606 | </tr> |
607 | - <tr class="secondary confirm_and_dismiss" |
608 | - tal:define="name_id string:${view/html_id}_dismiss" |
609 | - tal:condition="view/can_confirm_and_dismiss"> |
610 | + <tr tal:define="name_id string:${view/html_id}_dismiss" |
611 | + tal:condition="view/can_confirm_and_dismiss" |
612 | + tal:attributes=" |
613 | + class string:secondary confirm_and_dismiss ${view/html_id}"> |
614 | <td colspan="4"></td> |
615 | <td> |
616 | <label class="fuzzy-checkbox" tal:attributes="for name_id"> |
617 | @@ -504,8 +506,6 @@ |
618 | lang context/pofile/language/dashedcode; |
619 | dir context/pofile/language/abbreviated_text_dir; |
620 | value translation_dictionary/submitted_translation; |
621 | - onKeyPress string:javascript:selectWidget('${translation_dictionary/html_id_translation}_new_select', event); |
622 | - onChange string:javascript:selectWidget('${translation_dictionary/html_id_translation}_new_select', event); |
623 | " |
624 | class="translate expandable" |
625 | /> |
626 | @@ -528,8 +528,6 @@ |
627 | name string:${translation_dictionary/html_id_translation}_new; |
628 | lang context/pofile/language/dashedcode; |
629 | dir context/pofile/language/abbreviated_text_dir; |
630 | - onKeyPress string:javascript:selectWidget('${translation_dictionary/html_id_translation}_new_select', event); |
631 | - onChange string:javascript:selectWidget('${translation_dictionary/html_id_translation}_new_select', event); |
632 | "> |
633 | <tal:content replace="translation_dictionary/submitted_translation" /></textarea> |
634 | </tal:with-content> |
635 | @@ -548,8 +546,6 @@ |
636 | name string:${translation_dictionary/html_id_translation}_new; |
637 | lang context/pofile/language/dashedcode; |
638 | dir context/pofile/language/abbreviated_text_dir; |
639 | - onKeyPress string:javascript:selectWidget('${translation_dictionary/html_id_translation}_new_select', event); |
640 | - onChange string:javascript:selectWidget('${translation_dictionary/html_id_translation}_new_select', event); |
641 | "></textarea> |
642 | </tal:without-content> |
643 | </tal:multi-line> |
644 | @@ -595,7 +591,8 @@ |
645 | value="force_suggestion" |
646 | tal:attributes=" |
647 | checked view/force_suggestion; |
648 | - name string:${view/html_id}_${language_code}_needsreview" |
649 | + name string:${view/html_id}_${language_code}_needsreview; |
650 | + id string:${view/html_id}_force_suggestion;" |
651 | /> |
652 | <tal:block condition="not:view/is_plural"> |
653 | Someone should review this translation |
654 | |
655 | === modified file 'lib/lp/translations/templates/pofile-translate.pt' |
656 | --- lib/lp/translations/templates/pofile-translate.pt 2010-01-27 07:38:02 +0000 |
657 | +++ lib/lp/translations/templates/pofile-translate.pt 2010-02-25 13:03:21 +0000 |
658 | @@ -13,6 +13,7 @@ |
659 | color: lightgray; |
660 | } |
661 | </style> |
662 | + |
663 | <script type="text/javascript" |
664 | tal:condition="devmode" |
665 | tal:define="lp_js string:${icingroot}/build" |
666 | @@ -20,50 +21,13 @@ |
667 | <script type="text/javascript"> |
668 | registerLaunchpadFunction(insertAllExpansionButtons); |
669 | |
670 | - LPS.use('node', 'cookie', 'anim', 'lp.pofile', function(Y) { |
671 | - |
672 | - var hide_notification = function(node) { |
673 | - var hide_anim = new Y.Anim({ |
674 | - node: node, |
675 | - to: { height: 0, |
676 | - marginTop: 0, marginBottom: 0, |
677 | - paddingTop: 0, paddingBottom: 0 } |
678 | - }); |
679 | - node.setStyle('border', 'none'); |
680 | - hide_anim.set('duration', 0.4); |
681 | - hide_anim.on('end', function(e) { |
682 | - node.setStyle('display', 'none'); |
683 | - }); |
684 | - hide_anim.run(); |
685 | - } |
686 | - |
687 | - var updateNotificationBox = function(e) { |
688 | - var notice = Y.one('.important-notice-container'); |
689 | - var balloon = notice.one('.important-notice-balloon'); |
690 | - var dismiss_notice_cookie = ('translation-docs-for-' + |
691 | - documentation_cookie); |
692 | - |
693 | - // Check the cookie to see if the user has already dismissed |
694 | - // the notification box for this session. |
695 | - var already_seen = Y.Cookie.get(dismiss_notice_cookie, Boolean); |
696 | - if (already_seen) { |
697 | - notice.setStyle('display', 'none'); |
698 | - } |
699 | - |
700 | - var cancel_button = notice.one( |
701 | - '.important-notice-cancel-button'); |
702 | - // Cancel button starts out hidden. If user has JavaScript, |
703 | - // then we want to show it. |
704 | - cancel_button.setStyle('visibility', 'visible'); |
705 | - cancel_button.on('click', function(e) { |
706 | - e.halt(); |
707 | - hide_notification(balloon); |
708 | - Y.Cookie.set(dismiss_notice_cookie, true); |
709 | - }); |
710 | - }; |
711 | - |
712 | + LPS.use('lp.pofile', function(Y) { |
713 | Y.on('domready', Y.lp.pofile.setupSuggestionDismissal); |
714 | - Y.on('domready', updateNotificationBox); |
715 | + Y.on('domready', Y.lp.pofile.updateNotificationBox); |
716 | + Y.on('domready', Y.lp.pofile.initializeKeyBindings); |
717 | + Y.on('domready', function(e) { |
718 | + Y.lp.pofile.setFocus(autofocus_field); |
719 | + }); |
720 | }); |
721 | </script> |
722 | </div> |
723 | @@ -262,6 +226,7 @@ |
724 | <td style="text-align: right;"> |
725 | <input type="submit" |
726 | name="submit_translations" |
727 | + id="save_and_continue_button" |
728 | value="Save & Continue" |
729 | /> |
730 | </td> |
731 | @@ -277,6 +242,8 @@ |
732 | <tal:status replace="structure context/@@+access" /> |
733 | <tal:contributors replace="structure context/@@+contributors" /> |
734 | </tal:havepluralforms> |
735 | + <metal:pofile-js-footer |
736 | + use-macro="context/@@+translations-macros/pofile-js-footer" /> |
737 | </div> |
738 | </body> |
739 | </html> |
740 | |
741 | === modified file 'lib/lp/translations/templates/translationmessage-translate.pt' |
742 | --- lib/lp/translations/templates/translationmessage-translate.pt 2010-01-27 07:38:02 +0000 |
743 | +++ lib/lp/translations/templates/translationmessage-translate.pt 2010-02-25 13:03:21 +0000 |
744 | @@ -20,6 +20,10 @@ |
745 | <script type="text/javascript"> |
746 | LPS.use('node', 'lp.pofile', function(Y) { |
747 | Y.on('domready', Y.lp.pofile.setupSuggestionDismissal); |
748 | + Y.on('domready', Y.lp.pofile.initializeKeyBindings); |
749 | + Y.on('domready', function(e) { |
750 | + Y.lp.pofile.setFocus(autofocus_field); |
751 | + }); |
752 | }); |
753 | </script> |
754 | </div> |
755 | @@ -76,6 +80,7 @@ |
756 | <th colspan="5" style="text-align: right;"> |
757 | <input type="submit" |
758 | name="submit_translations" |
759 | + id="save_and_continue_button" |
760 | value="Save & Continue" /> |
761 | </th> |
762 | </tr> |
763 | @@ -89,6 +94,8 @@ |
764 | replace="structure view/batchnav/@@+navigation-links-lower" /> |
765 | <tal:status replace="structure context/pofile/@@+access" /> |
766 | </tal:havepluralforms> |
767 | + <metal:pofile-js-footer |
768 | + use-macro="context/@@+translations-macros/pofile-js-footer" /> |
769 | </div> |
770 | |
771 | </body> |
772 | |
773 | === modified file 'lib/lp/translations/templates/translations-macros.pt' |
774 | --- lib/lp/translations/templates/translations-macros.pt 2010-01-27 07:38:02 +0000 |
775 | +++ lib/lp/translations/templates/translations-macros.pt 2010-02-25 13:03:21 +0000 |
776 | @@ -6,7 +6,7 @@ |
777 | <metal:render-suggestion define-macro="render-suggestion"> |
778 | <tal:submission condition="submission"> |
779 | <tal:not-empty condition="not:submission/is_empty"> |
780 | - <tr tal:attributes="class string:secondary ${dismissable}; |
781 | + <tr tal:attributes="class string:secondary ${dismissable} ${submission/translation_html_id}; |
782 | id submission/row_html_id"> |
783 | <th colspan="3" tal:content="section_title"> |
784 | Packaged: |
785 | @@ -130,6 +130,17 @@ |
786 | </metal:nav-pofile-subpages> |
787 | |
788 | |
789 | +<metal:pofile-js-footer define-macro="pofile-js-footer"> |
790 | + <script type="text/javascript" |
791 | + tal:content=" |
792 | + structure string:<!-- |
793 | + var autofocus_field = '${view/autofocus_html_id}'; |
794 | + var translations_order = '${view/translations_order}'; |
795 | + var plural_forms = ${context/plural_forms}; |
796 | + // -->" /> |
797 | +</metal:pofile-js-footer> |
798 | + |
799 | + |
800 | <metal:translations-js define-macro="translations-js"> |
801 | <script |
802 | type="text/javascript" |
= Bug 359180 =
This is the follow up for the current commited branch for bug 359180 as it was discovered that Shift+Alt+up and Shift+Alt+Down are used by.
== Proposed fix ==
Use Shift+Alt+j and Shift+Alt+k for navigation.
Add fields in the translations_order only if the user can edit(add, change or suggest) the translation.
== Pre-implementation notes == /code.edge. launchpad. net/~adiroiban/ launchpad/ bug-359180/ +merge/ 16422
Notes can be found on the previous MP:
https:/
== Implementation details ==
There was a small refactorization for getting translations_order and autofocus_html_id.
== Tests ==
I was not able to produce a reasonable windmill test since it is not trivial to find the current focused node or to see if a node is focused.
This requires adding onFocus and onBlur trigger for all DOM nodes.
The test for the view is here:
./bin/test -t pofile-views
== Demo and Q/A == /code.edge. launchpad. net/~adiroiban/ launchpad/ bug-359180/ +merge/ 16422
Demo and Q/A can be found on the previous MP
https:/
Instead of UP and DOWN, use j and k
= Launchpad lint =
Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.
Linting changed files: /launchpad/ javascript/ translations/ pofile. js
lib/canonical
== JSLint notices == adi/launchpad/ lp-branches/bug-359180-take-2/ lib/canonical/ launchpad/ javascript/ translations/ pofile. js'.
No handlers could be found for logger "bzr"
jslint: No problem found in '/home/
jslint: 1 file to lint.