Launchpad itself

Merge lp:~adiroiban/launchpad/bug-570899 into lp:launchpad

  1. bug-570899
  2. Merge into devel
Proposed by Adi Roiban
Status: Merged
Approved by: Eleanor Berger
Approved revision: no longer in the source branch.
Merged at revision: 10912
Proposed branch: lp:~adiroiban/launchpad/bug-570899
Merge into: lp:launchpad
Diff against target: 327 lines (+114/-82)
1 file modified
lib/canonical/launchpad/javascript/translations/pofile.js (+114/-82)
To merge this branch: bzr merge lp:~adiroiban/launchpad/bug-570899
Reviewer Review Type Date Requested Status
Eleanor Berger (community) Approve
Review via email: mp+25443@code.launchpad.net

Commit message

Don't autoselect new translations when pressing the navigation keys.

Description of the change

= Bug 570899 =

When I click on suggested translation, then click to text field to move cursor as I read original string (or whatever reason), horizontal movement (left and right cursor keys) in text field will mark suggested translation as New translation. Even though I did not made any changes.
There is a risk that I overlook it, hit Save & Continue and translation I intended to only approve is hereby translated and reviewed by me. This isn't fair to original translator regarding Karma and morally.

== Proposed fix ==

Add an exception so that key navigation will not autoselect the translations.

== Pre-implementation notes ==

No pre-implementation chat yet.

== Implementation details ==

After a first feedback from intellectronica, I have added key code constants and refactored the existent/previous code to avoid jslint warnings.

== Tests ==

Unfortunately I was not able to simulate/trigger LEFT/RIGHT/UP/DOWN and other key events.
I have also reported this problem and talked with the Windmill developer but we could not find a way for triggering such events in Firefox.

== Demo and Q/A ==
Login as admin using Firefox.
Go to https://translations.launchpad.dev/evolution/trunk/+pots/evolution-2.2/es/5/+translate

Click on "bang bang in evo hoary" suggestion.

Now click on the "New translation:" text input and press the LEFT and RIGHT, UP and DOWN keys.
The new translation should not be selected and the "bang bang in evo hoary" suggestion should be still selected.

The same result should be observed with SHIFT+ALT+J and SHIFT+ALT+K

= 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/launchpad/javascript/translations/pofile.js

== JSLint notices ==
jslint: No problem found in '/home/adi/launchpad/lp-branches/bug-570899/lib/canonical/launchpad/javascript/translations/pofile.js'.

jslint: 1 file to lint.

To post a comment you must log in.
Revision history for this message
Eleanor Berger (intellectronica) wrote :

Thanks for perfecting this branch. Very nice work.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/canonical/launchpad/javascript/translations/pofile.js'
--- lib/canonical/launchpad/javascript/translations/pofile.js 2010-04-23 16:09:20 +0000
+++ lib/canonical/launchpad/javascript/translations/pofile.js 2010-05-18 14:14:35 +0000
@@ -9,6 +9,26 @@
9Y.log('loading lp.pofile');9Y.log('loading lp.pofile');
10var self = Y.namespace('lp.pofile');10var self = Y.namespace('lp.pofile');
1111
12var KEY_CODE_TAB = 9;
13var KEY_CODE_ENTER = 13;
14var KEY_CODE_LEFT = 37;
15var KEY_CODE_UP = 38;
16var KEY_CODE_RIGHT = 39;
17var KEY_CODE_DOWN = 40;
18var KEY_CODE_0 = 48;
19var KEY_CODE_A = 65;
20var KEY_CODE_B = 66;
21var KEY_CODE_C = 67;
22var KEY_CODE_D = 68;
23var KEY_CODE_F = 70;
24var KEY_CODE_J = 74;
25var KEY_CODE_K = 75;
26var KEY_CODE_L = 76;
27var KEY_CODE_N = 78;
28var KEY_CODE_P = 80;
29var KEY_CODE_R = 82;
30var KEY_CODE_S = 83;
31
12/**32/**
13 * Function to disable/enable all suggestions as they are marked/unmarked33 * Function to disable/enable all suggestions as they are marked/unmarked
14 * for dismission.34 * for dismission.
@@ -187,11 +207,12 @@
187var selectTranslation = function(e, field) {207var selectTranslation = function(e, field) {
188 // Don't select when tabbing, navigating and simply pressing208 // Don't select when tabbing, navigating and simply pressing
189 // enter to submit the form.209 // enter to submit the form.
210 // Also, don't select when using keyboard shortcuts (ie Alt+Shift+KEY)
190 // Looks like this is not needed for Epiphany and Chromium211 // Looks like this is not needed for Epiphany and Chromium
191 if (e.keyCode == 9 || e.keyCode == 13 ||212 if (e.keyCode == KEY_CODE_TAB || e.keyCode == KEY_CODE_ENTER ||
192 e.keyCode == 38 || e.keyCode == 40 ||213 e.keyCode == KEY_CODE_LEFT || e.keyCode == KEY_CODE_UP ||
193 (e.shiftKey && e.altKey && e.keyCode == 74) ||214 e.keyCode == KEY_CODE_RIGHT || e.keyCode == KEY_CODE_DOWN ||
194 (e.shiftKey && e.altKey && e.keyCode == 75)) {215 (e.shiftKey && e.altKey)) {
195 return;216 return;
196 }217 }
197218
@@ -207,41 +228,41 @@
207228
208 Y.get('document').on("keyup", function(e) {229 Y.get('document').on("keyup", function(e) {
209 var link;230 var link;
210 // Shift+Alt+s - Save form231 // Shift+Alt+S - Save form
211 if (e.shiftKey && e.altKey && e.keyCode == 83) {232 if (e.shiftKey && e.altKey && e.keyCode == KEY_CODE_S) {
212 Y.one('#save_and_continue_button').invoke('click');233 Y.one('#save_and_continue_button').invoke('click');
213 }234 }
214 // Shift+Alt+f - Go to search field235 // Shift+Alt+F - Go to search field
215 if (e.shiftKey && e.altKey && e.keyCode == 70) {236 if (e.shiftKey && e.altKey && e.keyCode == KEY_CODE_F) {
216 setFocus('search_box');237 setFocus('search_box');
217 }238 }
218 // Shift+Alt+b - Go to first translation field239 // Shift+Alt+B - Go to first translation field
219 if (e.shiftKey && e.altKey && e.keyCode == 66) {240 if (e.shiftKey && e.altKey && e.keyCode == KEY_CODE_B) {
220 setFocus(fields[0]);241 setFocus(fields[0]);
221 }242 }
222 // Shift+Alt+n - Go to next page in batch243 // Shift+Alt+N - Go to next page in batch
223 if (e.shiftKey && e.altKey && e.keyCode == 78) {244 if (e.shiftKey && e.altKey && e.keyCode == KEY_CODE_N) {
224 link = Y.one('#batchnav_next');245 link = Y.one('#batchnav_next');
225 if (link !== null){246 if (link !== null){
226 window.location.assign(link.get('href'));247 window.location.assign(link.get('href'));
227 }248 }
228 }249 }
229 // Shift+Alt+p - Go to previous page in batch250 // Shift+Alt+P - Go to previous page in batch
230 if (e.shiftKey && e.altKey && e.keyCode == 80) {251 if (e.shiftKey && e.altKey && e.keyCode == KEY_CODE_P) {
231 link = Y.one('#batchnav_previous');252 link = Y.one('#batchnav_previous');
232 if (link !== null){253 if (link !== null){
233 window.location.assign(link.get('href'));254 window.location.assign(link.get('href'));
234 }255 }
235 }256 }
236 // Shift+Alt+a - Go to first page in batch257 // Shift+Alt+A - Go to first page in batch
237 if (e.shiftKey && e.altKey && e.keyCode == 65) {258 if (e.shiftKey && e.altKey && e.keyCode == KEY_CODE_A) {
238 link = Y.one('#batchnav_first');259 link = Y.one('#batchnav_first');
239 if (link !== null){260 if (link !== null){
240 window.location.assign(link.get('href'));261 window.location.assign(link.get('href'));
241 }262 }
242 }263 }
243 // Shift+Alt+l - Go to last page in batch264 // Shift+Alt+L - Go to last page in batch
244 if (e.shiftKey && e.altKey && e.keyCode == 76) {265 if (e.shiftKey && e.altKey && e.keyCode == KEY_CODE_L) {
245 link = Y.one('#batchnav_last');266 link = Y.one('#batchnav_last');
246 if (link !== null){267 if (link !== null){
247 window.location.assign(link.get('href'));268 window.location.assign(link.get('href'));
@@ -268,6 +289,14 @@
268 });289 });
269};290};
270291
292/*
293 * Adapter for calling functions from Y.on().
294 * It is used for ignoring the `event` parameter that is passed to all
295 * functions called by Y.on().
296 */
297var on_event_adapter = function(event, method, argument) {
298 method(argument);
299};
271300
272var initializeFieldsKeyBindings = function (fields) {301var initializeFieldsKeyBindings = function (fields) {
273 for (var key = 0; key < fields.length; key++) {302 for (var key = 0; key < fields.length; key++) {
@@ -296,38 +325,39 @@
296 // Set next field and copy text for all but last field325 // Set next field and copy text for all but last field
297 // (last is Save & Continue button)326 // (last is Save & Continue button)
298 if (key < fields.length - 1) {327 if (key < fields.length - 1) {
299 // Shift+Alt+j - Go to next translation328 // Shift+Alt+J - Go to next translation
300 Y.on(329 Y.on(
301 'key', setNextFocus, '#' + fields[key],330 'key', setNextFocus, '#' + fields[key],
302 'down:74+shift+alt', Y, fields[next]);331 'down:' + KEY_CODE_J + '+shift+alt', Y, fields[next]);
303 // Shift+Alt+KEY_DOWN - Go to next translation332 // Shift+Alt+KEY_DOWN - Go to next translation
304 Y.on(333 Y.on(
305 'key', setNextFocus, '#' + fields[key],334 'key', setNextFocus, '#' + fields[key],
306 'down:40+shift+alt', Y, fields[next]);335 'down:' + KEY_CODE_DOWN + '+shift+alt', Y, fields[next]);
307 // Shift+Alt+c - Copy original text336 // Shift+Alt+C - Copy original text
308 Y.on(337 Y.on(
309 'key', copyOriginalTextAll, '#' + fields[key],338 'key', copyOriginalTextAll, '#' + fields[key],
310 'down:67+shift+alt', Y, msgset_id, translation_stem);339 'down:' + KEY_CODE_C + '+shift+alt',
311340 Y, msgset_id, translation_stem);
312 // Shift+Alt+r - Toggle someone should review341
313 Y.on(342 // Shift+Alt+R - Toggle someone should review
314 'key',343 Y.on(
315 function(e, stem) {344 'key', on_event_adapter,
316 toggleWidget(stem + '_force_suggestion');345 '#' + fields[key], 'down:' + KEY_CODE_R + '+shift+alt', Y,
317 },346 toggleWidget, msgset_id + '_force_suggestion');
318 '#' + fields[key], 'down:82+shift+alt', Y, msgset_id);347
319348 // Shift+Alt+D - Toggle dismiss all translations
320 // Shift+Alt+d - Toggle dismiss all translations349 Y.on(
321 Y.on(350 'key', on_event_adapter,
322 'key', function(e, stem) {351 '#' + fields[key], 'down:' + KEY_CODE_D + '+shift+alt', Y,
323 toggleWidget(stem + '_dismiss');352 toggleWidget, msgset_id + '_dismiss');
324 }, '#' + fields[key], 'down:68+shift+alt', Y, msgset_id);
325353
326 // Shift+Alt+0 - Mark current translation354 // Shift+Alt+0 - Mark current translation
327 Y.on(355 Y.on(
328 'key', function(e, key) {356 'key', on_event_adapter,
329 selectWidgetByID(key.replace(/_new/, "_radiobutton"));357 '#' + fields[key], 'down:' + KEY_CODE_0 + '+shift+alt', Y,
330 }, '#' + fields[key], 'down:48+shift+alt', Y, fields[key]);358 selectWidgetByID,
359 fields[key].replace(/_new/, "_radiobutton"));
360
331361
332 initializeSuggestionsKeyBindings(fields[key]);362 initializeSuggestionsKeyBindings(fields[key]);
333 }363 }
@@ -337,15 +367,15 @@
337 var parts = fields[previous].split('_');367 var parts = fields[previous].split('_');
338 var singular_copy_text = (368 var singular_copy_text = (
339 parts[0] + '_' + parts[1] + '_singular_copy_text');369 parts[0] + '_' + parts[1] + '_singular_copy_text');
340 // Shift+Alt+k - Go to previous translation370 // Shift+Alt+K - Go to previous translation
341 Y.on(371 Y.on(
342 'key', setPreviousFocus, '#' + fields[key],372 'key', setPreviousFocus, '#' + fields[key],
343 'down:75+shift+alt', Y, fields[previous],373 'down:' + KEY_CODE_K + '+shift+alt', Y, fields[previous],
344 singular_copy_text);374 singular_copy_text);
345 // Shift+Alt+KEY_UP - Go to previous translation375 // Shift+Alt+KEY_UP - Go to previous translation
346 Y.on(376 Y.on(
347 'key', setPreviousFocus, '#' + fields[key],377 'key', setPreviousFocus, '#' + fields[key],
348 'down:38+shift+alt', Y, fields[previous],378 'down:' + KEY_CODE_UP + '+shift+alt', Y, fields[previous],
349 singular_copy_text);379 singular_copy_text);
350 }380 }
351 }381 }
@@ -426,6 +456,34 @@
426 initializeFieldsKeyBindings(fields);456 initializeFieldsKeyBindings(fields);
427};457};
428458
459/*
460 * Controls the behavior for reseting current translations
461 */
462var resetTranslation = function (event, translation_id) {
463 if (this === null) {
464 // Don't do nothing if we don't have a context object.
465 return;
466 }
467 if (this.get('checked') === true) {
468 var new_translation_select = Y.one(
469 '#' + translation_id + '_select');
470 if (new_translation_select !== null) {
471 new_translation_select.set('checked', true);
472 }
473 } else {
474 var new_translation_field = Y.one('#' + translation_id);
475 if (new_translation_field !== null &&
476 new_translation_field.get('value') === '') {
477 var current_select_id = translation_id.replace(
478 /_new$/, '_radiobutton');
479 var current_select = Y.one('#' + current_select_id);
480 if (current_select !== null) {
481 current_select.set('checked', true);
482 }
483 }
484 }
485};
486
429487
430/**488/**
431 * Translations can be reset by submitting an empty translations and ticking489 * Translations can be reset by submitting an empty translations and ticking
@@ -443,33 +501,7 @@
443 // this field, just continue to the next field.501 // this field, just continue to the next field.
444 break;502 break;
445 }503 }
446 Y.on(504 Y.on('click', resetTranslation, node , node, fields[key]
447 'click',
448 function (e, translation_id) {
449 if (this === null) {
450 // Don't do nothing if we don't have a context object.
451 return;
452 }
453 if (this.get('checked') == true) {
454 var new_translation_select = Y.one(
455 '#' + translation_id + '_select');
456 if (new_translation_select !== null) {
457 new_translation_select.set('checked', true);
458 }
459 } else {
460 var new_translation_field = Y.one('#' + translation_id);
461 if (new_translation_field !== null &&
462 new_translation_field.get('value') == '') {
463 var current_select_id = translation_id.replace(
464 /_new$/, '_radiobutton');
465 var current_select = Y.one('#' + current_select_id);
466 if (current_select !== null) {
467 current_select.set('checked', true);
468 }
469 }
470 }
471 },
472 node , node, fields[key]
473 );505 );
474 }506 }
475};507};
@@ -487,29 +519,29 @@
487var initializeBaseTranslate = function () {519var initializeBaseTranslate = function () {
488 try {520 try {
489 setupSuggestionDismissal();521 setupSuggestionDismissal();
490 } catch (e) {522 } catch (setup_suggestion_dismissal_error) {
491 Y.log(e, "error");523 Y.log(setup_suggestion_dismissal_error, "error");
492 }524 }
493525
494 try {526 try {
495 initializeKeyBindings();527 initializeKeyBindings();
496 } catch (e) {528 } catch (initialize_key_bindings_error) {
497 Y.log(e, "error");529 Y.log(initialize_key_bindings_error, "error");
498 }530 }
499531
500 try {532 try {
501 var fields = translations_order.split(' ');533 var fields = translations_order.split(' ');
502 initializeResetBehavior(fields);534 initializeResetBehavior(fields);
503 } catch (e) {535 } catch (initialize_reset_behavior_error) {
504 Y.log(e, "error");536 Y.log(initialize_reset_behavior_error, "error");
505 }537 }
506538
507 try {539 try {
508 setFocus(autofocus_field);540 setFocus(autofocus_field);
509 } catch (e) {541 } catch (set_focus_error) {
510 Y.log(e, "error");542 Y.log(set_focus_error, "error");
511 }543 }
512}544};
513545
514546
515/*547/*
@@ -518,8 +550,8 @@
518self.initializePOFile = function(e) {550self.initializePOFile = function(e) {
519 try {551 try {
520 updateNotificationBox();552 updateNotificationBox();
521 } catch (e) {553 } catch (update_notification_box_error) {
522 Y.log(e, "error");554 Y.log(update_notification_box_error, "error");
523 }555 }
524 initializeBaseTranslate();556 initializeBaseTranslate();
525};557};
@@ -533,7 +565,7 @@
533 try {565 try {
534 var fields = translations_order.split(' ');566 var fields = translations_order.split(' ');
535 initializeReviewDivergeMutualExclusion(fields);567 initializeReviewDivergeMutualExclusion(fields);
536 } catch (e) {568 } catch (initialize_review_diverge_mutual_exclusion_error) {
537 Y.log(e, "error");569 Y.log(e, "error");
538 }570 }
539571