Code review comment for lp:~adiroiban/launchpad/bug-359180

Thanks for the review!

Here is the latest diff.

=== modified file 'lib/canonical/launchpad/javascript/translations/pofile.js'
--- lib/canonical/launchpad/javascript/translations/pofile.js 2009-12-21 11:36:04 +0000
+++ lib/canonical/launchpad/javascript/translations/pofile.js 2009-12-22 07:44:31 +0000
@@ -86,7 +86,7 @@
 };

-var setFocus = function(e, field) {
+self.setFocus = function(field) {
     //Y.log(e.type + ":" + e.keyCode + ': ' + field);
     // if there is nofield, do nothing
     if (Y.one('#' + field)) {
@@ -96,7 +96,7 @@

 var setNextFocus = function(e, field) {
- setFocus(e,field);
+ self.setFocus(field);
     // stopPropagation() and preventDefault()
     e.halt();
 };
@@ -106,8 +106,8 @@

     // Original singular test is focused first to make sure
     // it is visible when scrolling up
- setFocus(e, original);
- setFocus(e, field);
+ self.setFocus(original);
+ self.setFocus(field);
     // stopPropagation() and preventDefault()
     e.halt();
 };
@@ -116,7 +116,6 @@
 var copyOriginalTextOne = function(from_id, to_id, select_id) {
     var from = Y.one('#' + from_id);
     var to = Y.one('#' + to_id);
-
     // The replacement regex strips all tags from the html.
     to.set('value', unescapeHTML(
         from.get('innerHTML').replace(/<\/?[^>]+>/gi, "")));
@@ -182,7 +181,13 @@
  */
 self.initializeKeyBindings = function(e) {

- var fields = tabindex_chain.split(' ');
+ if (translations_order.length < 1) {
+ // If no translations fiels are displayed on the page
+ // don't initialize the translations order
+ return;
+ }
+
+ var fields = translations_order.split(' ');
     // The last field is Save & Continue button
     fields.push('save_and_continue_button');

@@ -192,7 +197,7 @@

         var html_parts = fields[key].split('_');
         var original_stem = html_parts[0] + '_' + html_parts[1];
- var translation_stem = original_stem + '_' + html_parts[2];
+ var translation_stem = fields[key].replace(/_translation_(\d)+_new/,"");
         var select_widget = (
             translation_stem + '_' + html_parts[3] + '_' +
             html_parts[4] + '_new_select');

=== modified file 'lib/lp/translations/browser/pofile.py'
--- lib/lp/translations/browser/pofile.py 2009-12-20 11:12:16 +0000
+++ lib/lp/translations/browser/pofile.py 2009-12-22 06:16:13 +0000
@@ -935,17 +935,17 @@
             first_field = first_message.translation_dictionaries[0]
             return first_field['html_id_translation']
         except IndexError:
- return False
+ return ""

     @property
- def tabindex_chain(self):
+ def translations_order(self):
         try:
- tabindex = []
+ order = []
             for message in self.translationmessage_views:
                 for dictionary in message.translation_dictionaries:
- tabindex.append(
+ order.append(
                         dictionary['html_id_translation'] + '_new')
- return ' '.join(tabindex)
+ return ' '.join(order)

         except IndexError:
             return ""

=== modified file 'lib/lp/translations/browser/translationmessage.py'
--- lib/lp/translations/browser/translationmessage.py 2009-12-20 11:12:16 +0000
+++ lib/lp/translations/browser/translationmessage.py 2009-12-22 06:17:04 +0000
@@ -836,16 +836,16 @@
             first_field = first_message.translation_dictionaries[0]
             return first_field['html_id_translation']
         except IndexError:
- return False
+ return ""

     @property
- def tabindex_chain(self):
+ def translations_order(self):
         try:
- tabindex = []
+ order = []
             message = self.translationmessage_view
             for dictionary in message.translation_dictionaries:
- tabindex.append(dictionary['html_id_translation'] + '_new')
- return ' '.join(tabindex)
+ order.append(dictionary['html_id_translation'] + '_new')
+ return ' '.join(order)

         except IndexError:
             return ""

=== modified file 'lib/lp/translations/templates/currenttranslationmessage-translate-one.pt'
--- lib/lp/translations/templates/currenttranslationmessage-translate-one.pt 2009-12-21 10:50:26 +0000
+++ lib/lp/translations/templates/currenttranslationmessage-translate-one.pt 2009-12-22 06:19:00 +0000
@@ -48,7 +48,7 @@
                '${view/html_id}_singular',
                '${view/html_id_singular}_new');;
              selectWidget('${view/html_id_singular}_new_select', event);;
- return false;;
+ return false;;
          "><img alt="Copy text" title="Copy text" src="/@@/copy" /></a>
     </tal:form-writeable>
   </td>

=== modified file 'lib/lp/translations/templates/pofile-translate.pt'
--- lib/lp/translations/templates/pofile-translate.pt 2009-12-20 10:57:20 +0000
+++ lib/lp/translations/templates/pofile-translate.pt 2009-12-22 07:30:06 +0000
@@ -25,7 +25,7 @@
           Y.on('domready', Y.lp.pofile.initializeKeyBindings);
           Y.on('domready', Y.lp.pofile.updateNotificationBox);
           Y.on('domready', function(e) {
- Y.one('#' + autofocus_field).focus();
+ Y.lp.pofile.setFocus(autofocus_field);
           });
       });
     </script>

=== modified file 'lib/lp/translations/templates/translations-macros.pt'
--- lib/lp/translations/templates/translations-macros.pt 2009-12-20 10:57:20 +0000
+++ lib/lp/translations/templates/translations-macros.pt 2009-12-22 06:17:44 +0000
@@ -131,11 +131,10 @@

 <metal:pofile-js-footer define-macro="pofile-js-footer">
     <script type="text/javascript"
- tal:condition="view/autofocus_html_id"
         tal:content="
         structure string:<!--
         var autofocus_field = '${view/autofocus_html_id}_new';
- var tabindex_chain = '${view/tabindex_chain}';
+ var translations_order = '${view/translations_order}';
         var plural_forms = ${context/plural_forms};
         // -->" />
 </metal:pofile-js-footer>