On Mon, 2009-12-21 at 20:54 +0000, Edwin Grubbs wrote:
> Here is the incremental diff that addresses both Salgado's and
> Martin's comments.

Looks good to me, but there's some code that was apparently written to
test corner cases and left behind accidentally.  See below

> 
> {{{
> === modified file 'lib/canonical/launchpad/javascript/lp/errors.js'
> --- lib/canonical/launchpad/javascript/lp/errors.js	2009-11-23 19:29:02 +0000
> +++ lib/canonical/launchpad/javascript/lp/errors.js	2009-12-21 20:33:38 +0000
> @@ -69,7 +69,7 @@
>   * @param flash_node {Node} The node to red flash.
>   * @param msg {String} The message to display.
>  */
> -var display_error = function(flash_node, msg) {
> +Y.lp.display_error = function(flash_node, msg) {
>      create_error_overlay();
>      maybe_red_flash(flash_node, function(){
>          error_overlay.showError(msg);
> @@ -77,5 +77,36 @@
>      });
>  };
>  
> -Y.lp.display_error = display_error;
> -}, "0.1", {"requires":["lazr.formoverlay"]});
> +
> +var info_overlay;
> +/*
> + * Display the form overlay for non-error informational messages.
> + *
> + * @method display_info
> + * @param msg {String} The message to display.
> +*/
> +Y.lp.display_info = function(msg) {
> +    if (info_overlay === undefined) {
> +        info_overlay = new Y.lazr.PrettyOverlay({
> +            //headerContent: '<h2><img src="/@@/info-large"/>Notice</h2>',

Why is this commented out?

> +            centered: true,
> +            visible: false
> +        });
> +        info_overlay.render();
> +    }
> +    var content = Y.Node.create(
> +      '<div style="background: url(/@@/info-large) no-repeat; ' +
> +      'min-height: 32px; padding-left: 40px; padding-top: 16px"/></div>');
> +    content.appendChild(Y.Node.create(msg));
> +    var button_div = Y.Node.create('<div style="text-align: right"></div>');
> +    var ok_button = Y.Node.create('<button>OK</button>');
> +    ok_button.on('click', function(e) {
> +        info_overlay.fire('cancel');
> +    });
> +    button_div.appendChild(ok_button);
> +    content.appendChild(button_div);
> +    info_overlay.set('bodyContent', content);
> +    info_overlay.show();
> +};
> +
> +}, "0.1", {"requires":["lazr.formoverlay", "lazr.overlay"]});
> 
> === modified file 'lib/canonical/launchpad/javascript/registry/team.js'
> --- lib/canonical/launchpad/javascript/registry/team.js	2009-12-18 06:06:30 +0000
> +++ lib/canonical/launchpad/javascript/registry/team.js	2009-12-21 20:35:57 +0000
> @@ -54,13 +54,14 @@
>                  var members_section, members_ul, count_elem;
>                  if (did_status_change === false) {
>                      disable_spinner();
> -                    Y.lp.display_error(
> -                        addmember_link,
> +                    Y.lp.display_info(
>                          selected_person.title + ' is already ' +
> -                        current_status + '.');
> +                        current_status.toLowerCase() +
> +                        ' as a member of the team.');
>                      return;
>                  }
>                  var is_team = false;
> +                current_status = 'foo';

!?

I really hope your windmill test fails with this change...

>                  if (current_status == 'Invited') {
>                      is_team = true;
>                      members_section = box.query('#recently-invited');
> @@ -79,6 +80,7 @@
>                      Y.lp.display_error(
>                          addmember_link,
>                          'Unexpected status: ' + current_status);
> +                    return;
>                  }
>                  var first_node = members_ul.get('firstChild');
>  


-- 
Guilherme Salgado <email address hidden>