Launchpad itself

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

  1. bug-548999
  2. Merge into devel
Proposed by Adi Roiban
Status: Merged
Approved by: Aaron Bentley
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~adiroiban/launchpad/bug-548999
Merge into: lp:launchpad
Diff against target: 192 lines (+74/-35)
2 files modified
lib/canonical/launchpad/javascript/translations/pofile.js (+22/-14)
lib/lp/translations/windmill/tests/test_pofile_translate.py (+52/-21)
To merge this branch: bzr merge lp:~adiroiban/launchpad/bug-548999
Reviewer Review Type Date Requested Status
Aaron Bentley (community) Approve
Review via email: mp+22271@code.launchpad.net

Commit message

Handle new translation autoselect for languages containing country code.

Description of the change

= Bug 548999 =
Then typing a new translation the radio button that selects the translation is not automatically selected for languages codes like pt_BR or en_GB.

== Proposed fix ==
Improve pofile.js initializeFieldsKeyBindings() so that it will handle such language codes for the autoselect functionality.

== Pre-implementation notes ==
There are no pre-implementation notes.

== Implementation details ==
I have renamed some of the variabled and add more comment to document the naming conventions for POFile translate form fields.

== Tests ==
./bin/test -t pofile_new_translation_autoselect

== Demo and Q/A ==
Login as admin or a normal translator and go to an Brazilian translation page.
ie. http://translations.launchpad.dev/evolution/trunk/+pots/evolution-2.2/pt_BR/1/+translate

Type something in the `new translation` field.

The radio button before the new translation text input should be now selected

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/translations/windmill/tests/test_pofile_translate.py

To post a comment you must log in.
Revision history for this message
Aaron Bentley (abentley) wrote :

Including variable renames makes it hard to see what your branch actually does. That could have been separated into another branch. Also, the lack of a pre-implementation discussion is generally a deal-breaker, but this is an obvious-enough bug that I'll go ahead and approve it.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/javascript/translations/pofile.js'
2--- lib/canonical/launchpad/javascript/translations/pofile.js 2010-03-22 17:44:53 +0000
3+++ lib/canonical/launchpad/javascript/translations/pofile.js 2010-04-11 20:17:24 +0000
4@@ -139,10 +139,10 @@
5 };
6
7
8-var copyOriginalTextAll = function(e, original_stem, translation_stem) {
9+var copyOriginalTextAll = function(e, msgset_id, translation_stem) {
10
11- var original_singular = original_stem + '_singular';
12- var original_plural = original_stem + '_plural';
13+ var original_singular = msgset_id + '_singular';
14+ var original_plural = msgset_id + '_plural';
15 var singular_select = translation_stem + '_translation_0_new_select';
16 var translation_singular = translation_stem + '_translation_0_new';
17 var translation_plural = translation_stem + '_translation_';
18@@ -180,7 +180,7 @@
19 };
20
21
22-var selectTranslation = function(e, widget) {
23+var selectTranslation = function(e, widget_id) {
24 // Don't select when tabbing, navigating and simply pressing
25 // enter to submit the form.
26 // Looks like this is not needed for Epiphany and Chromium
27@@ -190,7 +190,7 @@
28 (e.shiftKey && e.altKey && e.keyCode == 75)) {
29 return;
30 }
31- selectWidgetByID(widget);
32+ selectWidgetByID(widget_id);
33 };
34
35
36@@ -265,19 +265,27 @@
37 var next = key + 1;
38 var previous = key - 1;
39
40+ // fields[key] has one of the following formats:
41+ // * msgset_1_es_translation_0_new
42+ // * msgset_2_pt_BR_translation_0_new
43+ // msgset_id is 'msgset_1' or 'msgset_2'
44+ // translation_stem has one of the following formats:
45+ // * msgset_1_es
46+ // * msgset_2_pt_BR
47+ // translation_select_id has one of the following formats:
48+ // * msgset_1_es_translation_0_new_select
49+ // * msgset_2_pt_BR_translation_0_new_select
50 var html_parts = fields[key].split('_');
51- var original_stem = html_parts[0] + '_' + html_parts[1];
52+ var msgset_id = html_parts[0] + '_' + html_parts[1];
53 var translation_stem = fields[key].replace(/_translation_(\d)+_new/,"");
54- var select_widget = (
55- translation_stem + '_' + html_parts[3] + '_' +
56- html_parts[4] + '_new_select');
57+ var translation_select_id = fields[key] + '_select';
58
59 Y.on(
60 'change', selectTranslation,
61- '#' + fields[key], Y, select_widget);
62+ '#' + fields[key], Y, translation_select_id);
63 Y.on(
64 'keypress', selectTranslation,
65- '#' + fields[key], Y, select_widget);
66+ '#' + fields[key], Y, translation_select_id);
67
68 // Set next field and copy text for all but last field
69 // (last is Save & Continue button)
70@@ -293,7 +301,7 @@
71 // Shift+Alt+c - Copy original text
72 Y.on(
73 'key', copyOriginalTextAll, '#' + fields[key],
74- 'down:67+shift+alt', Y, original_stem, translation_stem);
75+ 'down:67+shift+alt', Y, msgset_id, translation_stem);
76
77 // Shift+Alt+r - Toggle someone should review
78 Y.on(
79@@ -301,13 +309,13 @@
80 function(e, stem) {
81 toggleWidget(stem + '_force_suggestion');
82 },
83- '#' + fields[key], 'down:82+shift+alt', Y, original_stem);
84+ '#' + fields[key], 'down:82+shift+alt', Y, msgset_id);
85
86 // Shift+Alt+d - Toggle dismiss all translations
87 Y.on(
88 'key', function(e, stem) {
89 toggleWidget(stem + '_dismiss');
90- }, '#' + fields[key], 'down:68+shift+alt', Y, original_stem);
91+ }, '#' + fields[key], 'down:68+shift+alt', Y, msgset_id);
92
93 // Shift+Alt+0 - Mark current translation
94 Y.on(
95
96=== modified file 'lib/lp/translations/windmill/tests/test_pofile_translate.py'
97--- lib/lp/translations/windmill/tests/test_pofile_translate.py 2010-03-17 17:33:04 +0000
98+++ lib/lp/translations/windmill/tests/test_pofile_translate.py 2010-04-11 20:17:24 +0000
99@@ -12,41 +12,72 @@
100
101
102 class POFileNewTranslationFieldKeybindings(WindmillTestCase):
103- """Tests for keybinding actions associated to the translation field."""
104+ """Tests for keybinding actions associated to the translation field.
105+
106+ These tests should cover both simple (ie. pt) and composed (ie. pt_br)
107+ language codes.
108+ """
109
110 layer = TranslationsWindmillLayer
111 suite_name = 'POFile Translate'
112
113- def test_pofile_new_translation_autoselect(self):
114- """Test for automatically selecting new translation on text input.
115-
116- When new text is typed into the new translation text fields, the
117- associated radio button should be automatically selected.
118+ def _check_translation_autoselect(
119+ self, url, new_translation_id, new_translation_select_id):
120+ """Checks that the select radio button is checked when typing a new
121+ translation.
122 """
123- client = self.client
124- start_url = ('http://translations.launchpad.dev:8085/'
125- 'evolution/trunk/+pots/evolution-2.2/es/+translate')
126- user = lpuser.TRANSLATIONS_ADMIN
127- new_translation_id = u'msgset_1_es_translation_0_new'
128- radiobutton_id = u'msgset_1_es_translation_0_new_select'
129-
130 # Go to the translation page.
131- self.client.open(url=start_url)
132+ self.client.open(url=url)
133 self.client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
134- user.ensure_login(self.client)
135+ self.test_user.ensure_login(self.client)
136
137 # Wait for the new translation field and it's associated radio button.
138- client.waits.forElement(
139+ self.client.waits.forElement(
140 id=new_translation_id, timeout=constants.FOR_ELEMENT)
141- client.waits.forElement(
142- id=radiobutton_id, timeout=constants.FOR_ELEMENT)
143+ self.client.waits.forElement(
144+ id=new_translation_select_id, timeout=constants.FOR_ELEMENT)
145
146 # Check that the associated radio button is not selected.
147- client.asserts.assertNotChecked(id=radiobutton_id)
148+ self.client.asserts.assertNotChecked(id=new_translation_select_id)
149
150 # Type a new translation.
151- client.type(
152+ self.client.type(
153 id=new_translation_id, text=u'New translation')
154
155 # Check that the associated radio button is selected.
156- client.asserts.assertChecked(id=radiobutton_id)
157+ self.client.asserts.assertChecked(id=new_translation_select_id)
158+
159+ def test_pofile_new_translation_autoselect(self):
160+ """Test for automatically selecting new translation on text input.
161+
162+ When new text is typed into the new translation text fields, the
163+ associated radio button should be automatically selected.
164+ """
165+ self.test_user = lpuser.TRANSLATIONS_ADMIN
166+
167+ # Test the zoom out view for Evolution trunk Spanish (es).
168+ start_url = ('http://translations.launchpad.dev:8085/'
169+ 'evolution/trunk/+pots/evolution-2.2/es/+translate')
170+ new_translation_id = u'msgset_1_es_translation_0_new'
171+ new_translation_select_id = u'msgset_1_es_translation_0_new_select'
172+ self._check_translation_autoselect(
173+ start_url, new_translation_id, new_translation_select_id)
174+
175+ # Test the zoom in view for Evolution trunk Brazilian (pt_BR).
176+ start_url = ('http://translations.launchpad.dev:8085/'
177+ 'evolution/trunk/+pots/evolution-2.2/'
178+ 'pt_BR/1/+translate')
179+ new_translation_id = u'msgset_1_pt_BR_translation_0_new'
180+ new_translation_select_id = u'msgset_1_pt_BR_translation_0_new_select'
181+ self._check_translation_autoselect(
182+ start_url, new_translation_id, new_translation_select_id)
183+
184+ # Test the zoom out view for Ubuntu Hoary Brazilian (pt_BR).
185+ start_url = ('http://translations.launchpad.dev:8085/'
186+ 'ubuntu/hoary/+source/mozilla/+pots/pkgconf-mozilla/'
187+ 'pt_BR/1/+translate')
188+ new_translation_id = u'msgset_152_pt_BR_translation_0_new'
189+ new_translation_select_id = (u'msgset_152_pt_BR'
190+ '_translation_0_new_select')
191+ self._check_translation_autoselect(
192+ start_url, new_translation_id, new_translation_select_id)