launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/sharing-overlay-buttons into lp:launchpad with lp:~abentley/launchpad/sharing-language-bugs as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #758929 in Launchpad itself: "On translate sharing details page, "Submit" button on AJAX overlays doesn't say what it's submitting"
  https://bugs.launchpad.net/launchpad/+bug/758929

For more details, see:
https://code.launchpad.net/~abentley/launchpad/sharing-overlay-buttons/+merge/57938

= Summary =
Fix bug #758929: On translate sharing details page, "Submit" button on AJAX overlays doesn't say what it's submitting

== Proposed fix ==
Use the checkmark or X submit/cancel buttons used on other AJAX forms.

== Pre-implementation notes ==
Discussed with jml.

== Implementation details ==
Extracted create_form_overlay from prepare so that setting submit/cancel buttons could be done in a single place.

== Tests ==
None

== Demo and Q/A ==
Go to +sharing-details for a sourcepackage.  Click on the bottom two edit icons.  The displayed form should show checkbox and X for submit and cancel, and they should work as expected.


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.js
  lib/lp/translations/browser/tests/test_sharing_details.py
  lib/lp/translations/javascript/sourcepackage_sharing_details.js
  lib/lp/translations/browser/sourcepackage.py
-- 
https://code.launchpad.net/~abentley/launchpad/sharing-overlay-buttons/+merge/57938
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/sharing-overlay-buttons into lp:launchpad.

=== modified file 'lib/lp/translations/javascript/sourcepackage_sharing_details.js'
--- lib/lp/translations/javascript/sourcepackage_sharing_details.js	2011-04-15 18:58:28 +0000
+++ lib/lp/translations/javascript/sourcepackage_sharing_details.js	2011-04-15 18:58:30 +0000
@@ -140,6 +140,35 @@
 namespace.TranslationSharingConfig = TranslationSharingConfig;
 
 
+/**
+ * Create a form overlay for a submittable form.
+ */
+function create_form_overlay(title_html, submit_callback){
+    var submit_button = Y.Node.create(
+        '<button type="submit" name="field.actions.create" ' +
+        'value="Save Changes" class="lazr-pos lazr-btn" ' +
+        '>OK</button>'
+        );
+    var cancel_button = Y.Node.create(
+        '<button type="button" name="field.actions.cancel" ' +
+        'class="lazr-neg lazr-btn" >Cancel</button>'
+        );
+    function hide_and_callback(){
+        form_overlay.hide();
+        submit_callback.apply(this, arguments);
+    }
+    var form_overlay = new Y.lazr.FormOverlay({
+        centered: true,
+        headerContent: title_html,
+        form_cancel_button: cancel_button,
+        form_submit_button: submit_button,
+        form_submit_callback: hide_and_callback,
+        visible: false
+    });
+    return form_overlay;
+}
+
+
 function form_url(entry, view_name) {
     entry_url = Y.lp.get_url_path(entry.get('web_link'));
     return entry_url + '/' + view_name + '/++form++';
@@ -499,17 +528,12 @@
         'ProductSeries', productseries_picker_config);
     /* Picker can't normally be activated by two different elements. */
     add_activator(productseries_picker, '#packaging-incomplete-picker a');
-    var import_overlay = new Y.lazr.FormOverlay({
-        headerContent: '<h2>Import settings<h2>',
-        centered: true,
-        visible: false
-    });
-    import_overlay.set('form_submit_callback', function(form_data) {
+    var import_overlay = create_form_overlay(
+        '<h2>Import settings<h2>', function(form_data) {
         Y.log(form_data['field.translations_autoimport_mode']);
         mode = enum_title(
             form_data, 'field.translations_autoimport_mode',
             namespace.autoimport_modes);
-        import_overlay.hide();
         var product_series = sharing_controller.get('productseries');
         product_series.set('translations_autoimport_mode', mode);
         var autoimport_check = sharing_controller.get(
@@ -533,14 +557,9 @@
     });
     var autoimport = sharing_controller.get('tsconfig').get('autoimport');
     sharing_controller.set_check_picker(autoimport, import_overlay);
-    var usage_overlay = new Y.lazr.FormOverlay({
-        headerContent: '<h2>Configure translations<h2>',
-        centered: true,
-        visible: false
-    });
     var usage = sharing_controller.get('tsconfig').get('translations_usage');
-    usage_overlay.set('form_submit_callback', function(form_data) {
-        usage_overlay.hide();
+    var usage_overlay = create_form_overlay(
+        '<h2>Configure translations<h2>', function(form_data) {
         var product = sharing_controller.get('product');
         function reload_entry(config, entry) {
             lp_client.get(entry.get('self_link'), config);