launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/bug-993732 into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #993732 in MAAS: "Can't edit MAAS title in Chromium"
  https://bugs.launchpad.net/maas/+bug/993732

For more details, see:
https://code.launchpad.net/~jtv/maas/bug-993732/+merge/104491

The dashboard page installs and renders the widget for editing the MAAS's title.  But the handle for the widget is not on the page unless it's an admin loading the page.  The attempt to render the widget breaks.

In this branch, as brought up with Julian & Raphaël, I test for this error and then patch up the widget to stop trying to do its work when the handle is not present.  This means that it's the widget's own responsibility to see that its handle is not in the DOM, not the calling code's.  It saves the calling code having to check for this condition — the caller specifies the handle as a CSS selector, and the widget looks it up.


Jeroen
-- 
https://code.launchpad.net/~jtv/maas/bug-993732/+merge/104491
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/bug-993732 into lp:maas.

=== modified file 'src/maasserver/static/js/tests/test_utils.js'
--- src/maasserver/static/js/tests/test_utils.js	2012-04-30 13:10:04 +0000
+++ src/maasserver/static/js/tests/test_utils.js	2012-05-03 06:34:23 +0000
@@ -127,6 +127,15 @@
         return widget;
     },
 
+    test_widget_goes_away_quietly_if_not_wanted: function() {
+        // If the srcNode isn't present on the page, the widget understands
+        // that it's not wanted.  Rather than break, it simply refrains from
+        // rendering.
+        var widget = new module.TitleEditWidget({srcNode: '#no-widget-here'});
+        widget.render();
+        Y.Assert.areEqual(null, widget.get('input'));
+    },
+
     test_getInput_returns_input: function() {
         var widget = this.createWidget();
         input = widget.get('srcNode').one('input');

=== modified file 'src/maasserver/static/js/utils.js'
--- src/maasserver/static/js/utils.js	2012-04-30 13:10:04 +0000
+++ src/maasserver/static/js/utils.js	2012-05-03 06:34:23 +0000
@@ -190,7 +190,11 @@
     bindUI: function() {
         var self = this;
         var input = this.get('input');
-        // Click on the input node: start title edition.
+        if (input === null) {
+            // The widget isn't on this page.
+            return;
+        }
+        // Click on the input node: start title editing.
         input.on('click', function(e) {
             e.preventDefault();
             self.titleEditStart(e.rangeOffset);
@@ -202,8 +206,8 @@
         };
         // Blur is fired when the user clicks away.
         input.on('blur', done_editing);
-        // Change is fired when the input text as changed and the focus is now
-        // set another element.
+        // Change is fired when the input text has been changed and the focus
+        // shifts to another element.
         input.on('change', done_editing);
         // Form submitted (Enter pressed in the input Node).
         this.get('srcNode').on('submit', done_editing);