Merge lp:~gmb/launchpad/fix-subscribe-form-preloading-bug-722450 into lp:launchpad

Proposed by Graham Binns
Status: Merged
Approved by: Graham Binns
Approved revision: no longer in the source branch.
Merged at revision: 12496
Proposed branch: lp:~gmb/launchpad/fix-subscribe-form-preloading-bug-722450
Merge into: lp:launchpad
Diff against target: 69 lines (+20/-5)
1 file modified
lib/lp/bugs/javascript/bugtask_index_portlets.js (+20/-5)
To merge this branch: bzr merge lp:~gmb/launchpad/fix-subscribe-form-preloading-bug-722450
Reviewer Review Type Date Requested Status
Benji York (community) code Approve
Review via email: mp+51510@code.launchpad.net

Commit message

[r=benji][ui=none] [r=benji][bug=722450] The advanced direct bug subscription overlay's content is now loaded in the onClick handler for the Subscribe link rather than being loaded with every bug page load, which added unnecessary load to the server.

Description of the change

This branch is a minor JS refactoring to prevent the advanced subscription overlay from being loaded with every page load, which as lifeless pointed out increases server load unnecessarily.

The fix for this was to move the code that fetches the form content for the overlay into a function that is called in the onClick handler for the "Subscribe" link. This also means that we can do away with reloading the form content after the user has made a change to their subscription, saving us another unnecessary page load.

To post a comment you must log in.
Revision history for this message
Benji York (benji) wrote :

This branch looks good. The only suggestion I have is that I suspect
that users would have a slightly better experience if we hypnotize them
with a spinner so they don't notice the passage of time while we do the
HTTP request to populate the form overlay.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/bugs/javascript/bugtask_index_portlets.js'
--- lib/lp/bugs/javascript/bugtask_index_portlets.js 2011-02-24 00:23:04 +0000
+++ lib/lp/bugs/javascript/bugtask_index_portlets.js 2011-02-28 16:12:21 +0000
@@ -287,7 +287,8 @@
287 subscription.set('is_team', false);287 subscription.set('is_team', false);
288 var parent = e.target.get('parentNode');288 var parent = e.target.get('parentNode');
289 if (namespace.use_advanced_subscriptions) {289 if (namespace.use_advanced_subscriptions) {
290 subscription_overlay.show();290 load_and_show_advanced_subscription_overlay(
291 subscription, subscription_overlay);
291 } else {292 } else {
292 // Look for the false conditions of subscription, which293 // Look for the false conditions of subscription, which
293 // is_direct_subscription, etc. don't report correctly,294 // is_direct_subscription, etc. don't report correctly,
@@ -495,7 +496,6 @@
495 * @param subscription {Object} A Y.lp.bugs.subscriber.Subscription object.496 * @param subscription {Object} A Y.lp.bugs.subscriber.Subscription object.
496 */497 */
497function setup_advanced_subscription_overlay(subscription) {498function setup_advanced_subscription_overlay(subscription) {
498 subscription.enable_spinner("Loading...");
499 var subscription_overlay = new Y.lazr.FormOverlay({499 var subscription_overlay = new Y.lazr.FormOverlay({
500 headerContent: '<h2>Subscribe to bug</h2>',500 headerContent: '<h2>Subscribe to bug</h2>',
501 form_submit_button:501 form_submit_button:
@@ -505,7 +505,23 @@
505 centered: true,505 centered: true,
506 visible: false506 visible: false
507 });507 });
508 subscription_overlay.render('#privacy-form-container');
509 return subscription_overlay;
510}
508511
512/*
513 * Load the content for and display the advanced subscription overlay.
514 * The call to show() the overlay happens in the success handler for
515 * loading the form content. That way the overlay won't appear empty.
516 *
517 * @method load_and_show_advanced_subscription_overlay
518 * @param subscription {Object} A Y.lp.bugs.subscriber.Subscription object.
519 * @param subscription_overlay {Object} A Y.lazr.FormOverlay to load
520 * content into.
521 */
522function load_and_show_advanced_subscription_overlay(subscription,
523 subscription_overlay) {
524 subscription.enable_spinner('Loading...');
509 var subscription_link_url = subscription.get(525 var subscription_link_url = subscription.get(
510 'link').get('href') + '/++form++';526 'link').get('href') + '/++form++';
511 subscription_overlay.set(527 subscription_overlay.set(
@@ -525,6 +541,7 @@
525 subscription_overlay.renderUI();541 subscription_overlay.renderUI();
526 subscription_overlay.bindUI();542 subscription_overlay.bindUI();
527 subscription.disable_spinner();543 subscription.disable_spinner();
544 subscription_overlay.show();
528 }545 }
529 function on_failure(id, response, subscription_overlay) {546 function on_failure(id, response, subscription_overlay) {
530 subscription_overlay.set(547 subscription_overlay.set(
@@ -532,14 +549,12 @@
532 "Sorry, an error occurred while loading the form.");549 "Sorry, an error occurred while loading the form.");
533 subscription_overlay.renderUI();550 subscription_overlay.renderUI();
534 subscription.disable_spinner();551 subscription.disable_spinner();
552 subscription_overlay.show();
535 }553 }
536 var config = {554 var config = {
537 on: {success: on_success, failure: on_failure},555 on: {success: on_success, failure: on_failure},
538 }556 }
539 Y.io(subscription_link_url, config);557 Y.io(subscription_link_url, config);
540 subscription_overlay.render('#privacy-form-container');
541
542 return subscription_overlay;
543}558}
544559
545/*560/*