launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #07499
[Merge] lp:~sinzui/launchpad/null-export into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/null-export into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #992660 in Launchpad itself: "null object on /<lang_code>/+export"
https://bugs.launchpad.net/launchpad/+bug/992660
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/null-export/+merge/104290
Pre-implementation: no one
https://translations.qastaging.launchpad.net/gdp/0.5.x/+pots/gedit-developer-plugins/es/+export
is usable, but browsers report different js errors.
IE states that 'changedtext' is null or not an object.
Chromium reports that it cannot call removeClass on null.
Both issues are probably the same issue where TAL is conditionally
rendering a block that the script assumes will be there.
--------------------------------------------------------------------
RULES
* Move the inline JavaScript to a module and add tests for what it does.
* Update the script to return early if there is no subordinate checkbox.
QA
* https://translations.qastaging.launchpad.net/gdp/0.5.x/+pots/gedit-devel
oper-plugins/es/+export
* Verify there is no checkbox below the format select widget.
* Verify there is no javascript error
LINT
lib/lp/translations/javascript/poexport.js
lib/lp/translations/javascript/tests/test_poexport.html
lib/lp/translations/javascript/tests/test_poexport.js
lib/lp/translations/templates/pofile-export.pt
TEST
./bin/test -vv --layer=YUITest lp.translations
IMPLEMENTATION
Moved script into a tested module.
lib/lp/translations/javascript/poexport.js
lib/lp/translations/javascript/tests/test_poexport.html
lib/lp/translations/javascript/tests/test_poexport.js
lib/lp/translations/templates/pofile-export.pt
--
https://code.launchpad.net/~sinzui/launchpad/null-export/+merge/104290
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/null-export into lp:launchpad.
=== added file 'lib/lp/translations/javascript/poexport.js'
--- lib/lp/translations/javascript/poexport.js 1970-01-01 00:00:00 +0000
+++ lib/lp/translations/javascript/poexport.js 2012-05-01 20:18:20 +0000
@@ -0,0 +1,34 @@
+YUI.add('lp.translations.poexport', function(Y) {
+
+var namespace = Y.namespace('lp.translations.poexport');
+
+/*
+ * Initialize Javascript code for a POT/<lang_code> +export page.
+ */
+namespace.initialize_pofile_export_page = function() {
+ var pochanged_block = Y.one('#div_pochanged');
+ if (Y.Lang.isNull(pochanged_block)) {
+ return false;
+ }
+
+ var formatlist = Y.one('#div_format select');
+ var checkbox = Y.one('#div_pochanged input');
+ var changedtext = Y.one('#div_pochanged span');
+ function toggle_pochanged() {
+ if (formatlist.get('value') === 'PO') {
+ changedtext.removeClass('disabledpochanged');
+ checkbox.set('disabled', false);
+ }
+ else {
+ changedtext.addClass('disabledpochanged');
+ checkbox.set('disabled', true);
+ }
+ }
+ formatlist.on('change', toggle_pochanged);
+ // Initialize the state of the controls.
+ toggle_pochanged();
+ Y.one('#po-format-only').addClass('unseen');
+ return true;
+};
+
+}, "0.1", {"requires": ["node"]});
=== added file 'lib/lp/translations/javascript/tests/test_poexport.html'
--- lib/lp/translations/javascript/tests/test_poexport.html 1970-01-01 00:00:00 +0000
+++ lib/lp/translations/javascript/tests/test_poexport.html 2012-05-01 20:18:20 +0000
@@ -0,0 +1,64 @@
+<!DOCTYPE html>
+<!--
+Copyright 2012 Canonical Ltd. This software is licensed under the
+GNU Affero General Public License version 3 (see the file LICENSE).
+-->
+
+<html>
+ <head>
+ <title>Test lp.translations.poexport</title>
+
+ <!-- YUI and test setup -->
+ <script type="text/javascript"
+ src="../../../../../build/js/yui/yui/yui.js">
+ </script>
+ <link rel="stylesheet"
+ href="../../../../../build/js/yui/console/assets/console-core.css" />
+ <link rel="stylesheet"
+ href="../../../../../build/js/yui/console/assets/skins/sam/console.css" />
+ <link rel="stylesheet"
+ href="../../../../../build/js/yui/test/assets/skins/sam/test.css" />
+
+ <script type="text/javascript"
+ src="../../../../../build/js/lp/app/testing/testrunner.js"></script>
+
+ <link rel="stylesheet" href="../../../app/javascript/testing/test.css" />
+
+ <!-- Dependencies -->
+
+ <!-- The module under test. -->
+ <script type="text/javascript" src="../poexport.js"></script>
+
+ <!-- The test suite. -->
+ <script type="text/javascript" src="test_poexport.js"></script>
+ </head>
+ <body class="yui3-skin-sam">
+ <ul id="suites">
+ <li>lp.translations.poexport.test</li>
+ </ul>
+
+ <div id="fixture"></div>
+
+ <script type="text/x-template" id="pofile-export">
+ <form>
+ <div id="div_format">
+ <label for="sel_format">Format:</label>
+ <select name="format" id="sel_format">
+ <option value="PO">PO format</option>
+ <option value="MO">MO format</option>
+ </select>
+ </div>
+ <div id="div_pochanged">
+ <input type="checkbox" name="pochanged" id="cb_pochanged"
+ value="POCHANGED" />
+ <label for="cb_pochanged">
+ <span>
+ <em id="po-format-only">PO format only:</em>
+ Only strings that differ from imported versions
+ </span>
+ </label>
+ </div>
+ </form>
+ </script>
+ </body>
+</html>
=== added file 'lib/lp/translations/javascript/tests/test_poexport.js'
--- lib/lp/translations/javascript/tests/test_poexport.js 1970-01-01 00:00:00 +0000
+++ lib/lp/translations/javascript/tests/test_poexport.js 2012-05-01 20:18:20 +0000
@@ -0,0 +1,61 @@
+/* Copyright 2012 Canonical Ltd. This software is licensed under the
+ * GNU Affero General Public License version 3 (see the file LICENSE).
+ */
+
+YUI().use('lp.testing.runner', 'test', 'console', 'node-event-simulate',
+ 'lp.translations.poexport', function(Y) {
+
+var suite = new Y.Test.Suite("export Tests");
+var namespace = Y.lp.translations.poexport;
+
+
+suite.add(new Y.Test.Case({
+ name: 'PO export',
+
+ setUp: function() {
+ fixture = Y.one("#fixture");
+ var template = Y.one('#pofile-export').getContent();
+ var test_node = Y.Node.create(template);
+ fixture.append(test_node);
+ },
+
+ tearDown: function() {
+ Y.one("#fixture").empty();
+ },
+
+ test_initialize_pofile_export_page_without_pochanged: function() {
+ // The change handler was not added if the checbox does not exist.
+ var pochanged = Y.one('#div_pochanged');
+ pochanged.get('parentNode').removeChild(pochanged);
+ handler_added = namespace.initialize_pofile_export_page(Y);
+ Y.Assert.isFalse(handler_added);
+ },
+
+ test_initialize_pofile_export_page_with_pochanged_default_po: function() {
+ // The checkbox is enabled when PO is selected.
+ handler_added = namespace.initialize_pofile_export_page(Y);
+ Y.Assert.isTrue(handler_added);
+ Y.Assert.isTrue(
+ Y.one('#po-format-only').hasClass('unseen'));
+ Y.Assert.isFalse(
+ Y.one('#div_pochanged span').hasClass('disabledpochanged'));
+ Y.Assert.isFalse(
+ Y.one('#div_pochanged input').get('disabled'));
+ },
+
+ test_initialize_pofile_export_page_with_pochanged_mo_selected: function() {
+ // The checkbox is disabled when MO is selected.
+ handler_added = namespace.initialize_pofile_export_page(Y);
+ Y.Assert.isTrue(handler_added);
+ var formatlist = Y.one('#div_format select');
+ formatlist.set('selectedIndex', 1);
+ formatlist.simulate('change');
+ Y.Assert.isTrue(
+ Y.one('#div_pochanged span').hasClass('disabledpochanged'));
+ Y.Assert.isTrue(
+ Y.one('#div_pochanged input').get('disabled'));
+ }
+}));
+
+Y.lp.testing.Runner.run(suite);
+});
=== modified file 'lib/lp/translations/templates/pofile-export.pt'
--- lib/lp/translations/templates/pofile-export.pt 2012-04-28 00:03:54 +0000
+++ lib/lp/translations/templates/pofile-export.pt 2012-05-01 20:18:20 +0000
@@ -13,26 +13,9 @@
}
</style>
<script type="text/javascript">
- LPJS.use('node', 'event', function(Y){
+ LPJS.use('lp.translations.poexport', function(Y){
Y.on('domready', function(){
- // The pochanged option is only available for the PO format.
- var formatlist = Y.one('#div_format select');
- var checkbox = Y.one('#div_pochanged input');
- var changedtext = Y.one('#div_pochanged span');
- function toggle_pochanged() {
- if (formatlist.get('value') == 'PO') {
- changedtext.removeClass('disabledpochanged');
- checkbox.set('disabled', false);
- }
- else {
- changedtext.addClass('disabledpochanged');
- checkbox.set('disabled', true);
- }
- }
- formatlist.on('change', toggle_pochanged);
- // Initialize the state of the controls.
- toggle_pochanged();
- Y.one('#po-format-only').addClass('unseen');
+ Y.lp.translations.poexport.initialize_pofile_export_page(Y);
});
});
</script>
Follow ups
