launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03343
[Merge] lp:~abentley/launchpad/sharing-spinners into lp:launchpad
Aaron Bentley has proposed merging lp:~abentley/launchpad/sharing-spinners into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #758922 in Launchpad itself: "Missing spinners on translations +sharing-details js actions"
https://bugs.launchpad.net/launchpad/+bug/758922
For more details, see:
https://code.launchpad.net/~abentley/launchpad/sharing-spinners/+merge/57888
= Summary =
Fix bug #758922: Missing spinners on translations +sharing-details js actions
== Proposed fix ==
Add hidden spinners to web page, show them as needed.
== Pre-implementation notes ==
None
== Implementation details ==
Added a new "pending" boolean to checklist items. When this is true, the TranslationSharingController will show the spinner and hide the picker. When this is false, the TranslationSharingController will hide the spinner and may show the picker.
== Tests ==
firefox lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.html
== Demo and Q/A ==
Go to the +sharing-details page for a SourcePackage. Ensure that when each setting is changed, a spinner is shown. This should happen both when the list item was formlerly uncheckd and when it was formerly checked.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/translations/templates/sourcepackage-sharing-details.pt
lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.js
lib/lp/translations/javascript/sourcepackage_sharing_details.js
lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.html
--
https://code.launchpad.net/~abentley/launchpad/sharing-spinners/+merge/57888
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/sharing-spinners into lp:launchpad.
=== modified file 'lib/lp/translations/javascript/sourcepackage_sharing_details.js'
--- lib/lp/translations/javascript/sourcepackage_sharing_details.js 2011-04-13 17:39:32 +0000
+++ lib/lp/translations/javascript/sourcepackage_sharing_details.js 2011-04-15 15:09:33 +0000
@@ -26,7 +26,8 @@
}
},
// The HTML identifier of the item.
- identifier: null
+ identifier: null,
+ pending: {value: false}
};
Y.extend(CheckItem, Y.Base, {
});
@@ -195,9 +196,8 @@
}
-function IOHandler(flash_target, error_handler) {
+function IOHandler(controller, check, error_handler) {
that = this;
- this.flash_target = flash_target;
if (!Y.Lang.isValue(error_handler)){
this.error_handler = new Y.lp.client.ErrorHandler();
}
@@ -205,7 +205,10 @@
this.error_handler = error_handler;
}
this.error_handler.showError = function(error_msg) {
- Y.lp.app.errors.display_error(Y.one(that.flash_target), error_msg);
+ check.set('pending', false);
+ controller.update_check(check);
+ var flash_target = Y.one(controller.visible_check_selector(check));
+ Y.lp.app.errors.display_error(Y.one(flash_target), error_msg);
};
}
@@ -344,6 +347,8 @@
var productseries_check = that.get('tsconfig').get('product_series');
var lp_client = new Y.lp.client.Launchpad();
function save_productseries(config) {
+ productseries_check.set('pending', true);
+ that.update_check(productseries_check);
var source_package = that.get('source_package');
config.parameters = {
productseries: productseries_summary.api_uri};
@@ -369,11 +374,11 @@
}
function set_usage(product) {
that.replace_product(product);
+ productseries_check.set('pending', false);
that.update();
that.flash_check_green(productseries_check);
}
- var css_selector = this.visible_check_selector(productseries_check);
- var io_handler = new IOHandler(css_selector);
+ var io_handler = new IOHandler(this, productseries_check);
save_productseries(io_handler.chain_config(
get_productseries, cache_productseries, cache_branch, set_usage));
},
@@ -391,6 +396,8 @@
* closure, such as "that" and "branch_summary".
*/
function save_branch(config) {
+ branch_check.set('pending', true);
+ that.update_check(branch_check);
var productseries = that.get('productseries');
productseries.set('branch_link', branch_summary.api_uri);
productseries.lp_save(config);
@@ -400,11 +407,11 @@
}
function set_link(branch){
that.set_branch(branch);
+ branch_check.set('pending', false);
that.update();
that.flash_check_green(branch_check);
}
- css_selector = that.visible_check_selector(branch_check);
- var io_handler = new IOHandler(css_selector);
+ var io_handler = new IOHandler(this, branch_check);
save_branch(io_handler.chain_config(get_branch, set_link));
},
/**
@@ -442,6 +449,7 @@
*/
update_check: function(check){
var complete = Y.one(this.check_selector(check, true));
+ var hide_picker = !check.get('enabled') || check.get('pending');
var link = complete.one('.link a');
if (link !== null){
link.set('href', check.get('url'));
@@ -451,14 +459,19 @@
complete.toggleClass('lowlight', !check.get('enabled'));
var complete_picker = Y.one(this.picker_selector(check, true));
if (complete_picker !== null) {
- complete_picker.toggleClass('unseen', !check.get('enabled'));
+ complete_picker.toggleClass('unseen', hide_picker);
}
var incomplete = Y.one(this.check_selector(check, false));
incomplete.toggleClass('unseen', check.get('complete'));
incomplete.toggleClass('lowlight', !check.get('enabled'));
var incomplete_picker = Y.one(this.picker_selector(check, false));
if (incomplete_picker !== null) {
- incomplete_picker.toggleClass('unseen', !check.get('enabled'));
+ incomplete_picker.toggleClass('unseen', hide_picker);
+ }
+ var selector = this.visible_check_selector(check) + '-spinner';
+ var spinner = Y.one(selector);
+ if (Y.Lang.isValue(spinner)){
+ spinner.toggleClass('unseen', !check.get('pending'));
}
},
flash_check_green: function(check) {
@@ -514,14 +527,15 @@
product_series.set('translations_autoimport_mode', mode);
var autoimport_check = sharing_controller.get(
'tsconfig').get('autoimport');
- var css_selector = sharing_controller.visible_check_selector(
- autoimport_check);
- handler = new IOHandler(css_selector);
+ handler = new IOHandler(sharing_controller, autoimport_check);
function update_controller() {
sharing_controller.set_autoimport_mode(mode);
+ autoimport_check.set('pending', false);
sharing_controller.update();
sharing_controller.flash_check_green(autoimport_check);
}
+ autoimport_check.set('pending', true);
+ sharing_controller.update_check(autoimport_check);
/* XXX: AaronBentley 2011-04-04 bug=369293: Avoid 412 on repeated
* changes. This does not increase the risk of changing from a
* stale value, because the staleness check is not reasonable.
@@ -558,15 +572,22 @@
}
function replace_product(new_product) {
sharing_controller.replace_product(new_product);
+ usage.set('pending', false);
sharing_controller.update();
sharing_controller.flash_check_green(usage);
}
- css_selector = sharing_controller.visible_check_selector(usage);
var io_handler = new IOHandler(
+<<<<<<< TREE
css_selector, new Y.lp.client.FormErrorHandler());
var config = io_handler.chain_config(
get_productseries, replace_productseries, get_product,
replace_product);
+=======
+ sharing_controller, usage, new Y.lp.client.FormErrorHandler());
+ usage.set('pending', true);
+ sharing_controller.update_check(usage);
+ var config = io_handler.chain_config(get_product, replace_product);
+>>>>>>> MERGE-SOURCE
submit_form(
config, form_data, product, '+configure-translations', 'change');
});
=== modified file 'lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.html'
--- lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.html 2011-04-04 19:36:27 +0000
+++ lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.html 2011-04-15 15:09:33 +0000
@@ -46,8 +46,13 @@
<div id="branch-complete">Branch selected: <span class="link"><a
href="#" class="link"></a></span>
<span id="branch-complete-picker"><a href="#"></a></span>
- </div>
- <div id="branch-incomplete">No branch selected.</div>
+ <img src="../../../../canonical/launchpad/images/spinner.gif"
+ id="branch-complete-spinner"/>
+ </div>
+ <div id="branch-incomplete">No branch selected.
+ <img src="../../../../canonical/launchpad/images/spinner.gif"
+ id="branch-incomplete-spinner"/>
+ </div>
<span id="branch-incomplete-picker"><a href="#"></a></span>
<div id="translation-incomplete">
Translations are not enabled on the upstream project.
@@ -58,12 +63,16 @@
<a href="#enable-translations"></a>
</div>
<div id="upstream-sync-incomplete">
- Automatic synchronization of translations is not enabled.
- <a href="#enable-sync"></a>
+ Automatic synchronization of translations is not enabled.
+ <img src="../../../../canonical/launchpad/images/spinner.gif"
+ id="upstream-sync-incomplete-spinner"/>
+ <span id="upstream-sync-incomplete-picker"><a href="#enable-sync"></a></span>
</div>
<div id="upstream-sync-complete">
Automatic synchronization of translations is enabled.
- <a href="#enable-sync"></a>
+ <span id="upstream-sync-complete-picker"><a href="#enable-sync"></a></span>
+ <img src="../../../../canonical/launchpad/images/spinner.gif"
+ id="upstream-sync-complete-spinner"/>
</div>
</div>
<!-- The test output -->
=== modified file 'lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.js'
--- lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.js 2011-04-07 15:39:20 +0000
+++ lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.js 2011-04-15 15:09:33 +0000
@@ -198,6 +198,39 @@
ctrl.update_check(branch);
Y.Assert.isFalse(incomplete.hasClass('lowlight'));
},
+ test_update_check_pending: function(){
+ var incomplete_spinner = Y.one(
+ '#upstream-sync-incomplete-spinner');
+ var ctrl = new TranslationSharingController();
+ var autoimport = ctrl.get('tsconfig').get('autoimport');
+ var branch = ctrl.get('tsconfig').get('branch');
+ branch.set_link('a', 'b');
+ var incomplete_picker = Y.one(
+ ctrl.picker_selector(autoimport, false));
+ ctrl.update_check(autoimport);
+ Y.Assert.isTrue(
+ incomplete_spinner.hasClass('unseen'), 'spinner unseen');
+ Y.Assert.isFalse(
+ incomplete_picker.hasClass('unseen'), 'picker seen');
+ autoimport.set('pending', true);
+ ctrl.update_check(autoimport);
+ Y.Assert.isFalse(
+ incomplete_spinner.hasClass('unseen'), 'spinner seen');
+ Y.Assert.isTrue(
+ incomplete_picker.hasClass('unseen'), 'picker unseen');
+ autoimport.set('complete', true);
+ var complete_spinner = Y.one(
+ '#upstream-sync-complete-spinner');
+ var complete_picker = Y.one(
+ ctrl.picker_selector(autoimport, true));
+ ctrl.update_check(autoimport);
+ Y.Assert.isFalse(complete_spinner.hasClass('unseen'));
+ Y.Assert.isTrue(complete_picker.hasClass('unseen'));
+ autoimport.set('pending', false);
+ ctrl.update_check(autoimport);
+ Y.Assert.isTrue(complete_spinner.hasClass('unseen'));
+ Y.Assert.isFalse(complete_picker.hasClass('unseen'));
+ },
test_set_autoimport_mode: function() {
var ctrl = new TranslationSharingController();
var check = ctrl.get('tsconfig').get('autoimport');
=== modified file 'lib/lp/translations/templates/sourcepackage-sharing-details.pt'
--- lib/lp/translations/templates/sourcepackage-sharing-details.pt 2011-04-11 17:16:28 +0000
+++ lib/lp/translations/templates/sourcepackage-sharing-details.pt 2011-04-15 15:09:33 +0000
@@ -39,6 +39,8 @@
<span id="packaging-incomplete-picker">
<a tal:replace="structure view/set_packaging_link/escapedtext" />
</span>
+ <img src="/@@/spinner" class="unseen"
+ id="packaging-incomplete-spinner"/>
</li>
<li tal:attributes="class view/packaging_complete_class"
id="packaging-complete">
@@ -51,6 +53,8 @@
<span id="packaging-complete-picker">
<a tal:replace="structure view/change_packaging_link/escapedtext" />
</span>
+ <img src="/@@/spinner" class="unseen"
+ id="packaging-complete-spinner"/>
<a tal:replace="structure view/remove_packaging_link/escapedtext" />
</li>
<li tal:attributes="class view/branch_incomplete_class"
@@ -59,6 +63,8 @@
<span id="branch-incomplete-picker">
<a tal:replace="structure view/new_branch_link/escapedtext" />
</span>
+ <img src="/@@/spinner" class="unseen"
+ id="branch-incomplete-spinner"/>
</li>
<li tal:attributes="class view/branch_complete_class"
id="branch-complete">
@@ -69,6 +75,8 @@
<span id="branch-complete-picker">
<a tal:replace="structure view/change_branch_link/escapedtext" />
</span>
+ <img src="/@@/spinner" class="unseen"
+ id="branch-complete-spinner"/>
</li>
<li tal:attributes="class view/translations_disabled_class"
id="translation-incomplete">
@@ -76,6 +84,8 @@
<span id="translation-incomplete-picker">
<a tal:replace="structure view/configure_translations_link_unconfigured/escapedtext" />
</span>
+ <img src="/@@/spinner" class="unseen"
+ id="translation-incomplete-spinner"/>
</li>
<li tal:attributes="class view/translations_enabled_class"
id="translation-complete">
@@ -83,6 +93,8 @@
<span id="translation-complete-picker">
<a tal:replace="structure view/configure_translations_link_configured/escapedtext" />
</span>
+ <img src="/@@/spinner" class="unseen"
+ id="translation-complete-spinner"/>
</li>
<li tal:attributes="class view/upstream_sync_disabled_class"
id="upstream-sync-incomplete">
@@ -90,6 +102,8 @@
<span id="upstream-sync-incomplete-picker">
<a tal:replace="structure view/translation_sync_link_unconfigured/escapedtext" />
</span>
+ <img src="/@@/spinner" class="unseen"
+ id="upstream-sync-incomplete-spinner"/>
</li>
<li tal:attributes="class view/upstream_sync_enabled_class"
id="upstream-sync-complete">
@@ -97,6 +111,8 @@
<span id="upstream-sync-complete-picker">
<a tal:replace="structure view/translation_sync_link_configured/escapedtext" />
</span>
+ <img src="/@@/spinner" class="unseen"
+ id="upstream-sync-complete-spinner"/>
</li>
</ul>
</dd>
