launchpad-reviewers team mailing list archive

[Guilherme Salgado <guilherme.salgado@xxxxxxxxxx>]

Guilherme Salgado has proposed merging lp:~salgado/launchpad/bug-415365 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #415365 in Launchpad itself: "Errors are not handled properly in inline multi-line text editing"
  https://bugs.launchpad.net/launchpad/+bug/415365

For more details, see:
https://code.launchpad.net/~salgado/launchpad/bug-415365/+merge/95447

This uses an overlay to show errors on multi-line text editing (i.e. Y.EditableText) as the error is currently inserted in a node which is behind the text area so never seen by the user. I'm sure there's lots of room for improvement still, but this is certainly better than not showing the error to users at all, so I'd appreciate if that was taken into consideration.
-- 
https://code.launchpad.net/~salgado/launchpad/bug-415365/+merge/95447
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~salgado/launchpad/bug-415365 into lp:launchpad.

=== modified file 'lib/lp/app/javascript/inlineedit/editor.js'
--- lib/lp/app/javascript/inlineedit/editor.js	2011-12-21 07:54:40 +0000
+++ lib/lp/app/javascript/inlineedit/editor.js	2012-03-01 20:02:06 +0000
@@ -405,7 +405,11 @@
      */
     showError: function(msg) {
         this.hideLoadingSpinner();
-        this.get(ERROR_MSG).set('innerHTML', msg);
+        if (this.get(MULTILINE)) {
+            Y.lp.app.errors.display_error(this.get(BOUNDING_BOX), msg);
+        } else {
+            this.get(ERROR_MSG).set('innerHTML', msg);
+        }
         this.set(IN_ERROR, true);
         this.get(INPUT_EL).focus();
     },