launchpad-reviewers team mailing list archive

[Richard Harding <rick.harding@xxxxxxxxxxxxx>]

Review: Approve

Thanks for the update Ian. This looks and reads much smoother. A few small items below, but the code looks good. Approving per these.

#215
There's no need for the NAME. It's never used. Just put that string into the Base.create call. The two events don't need to be defined before hand. Just do ns.TEAM_CREATED and adjust the calls in the code to use ns.

#237
There's no need for the intermediate widgets reference. It can be condensed to just:
    var perform_load = false;
    if (!Y.Lang.isArray(ns.widgets)) {
        perform_load = true;
        ns.widgets = [];
    }
    ns.widgets.push(this);
    this._load_form(perform_load);

#271
Same as above, just reference ns.team_form.

#356
It's JS convention to use the Y.Lang.isNull() check here. 
https://dev.launchpad.net/JavaScriptReviewNotes?highlight=%28Y.Lang%29

#381
There's an extra indentation level.

#393
The that = this; should be hoisted to the top of the method and then used in the #378 method:

this.error_handler.showError = function (error_msg) {
    that._hideSpinner(submit_link);
    that.error_handler.handleFormValidationError(error_msg, [], []);

}

#659/660
The inputs should be self closing tags with />



-- 
https://code.launchpad.net/~wallyworld/launchpad/new-team-picker-simple-form/+merge/111781
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.