If you want me to look at this again I would be happy to, but otherwise land with changes.
Thank you Gary https://codereview.appspot.com/7007047/diff/15001/app/store/env.js File app/store/env.js (right): https://codereview.appspot.com/7007047/diff/15001/app/store/env.js#newcode87 app/store/env.js:87: this.set('defaultSeries', msg.default_series); The above two lines are supposed to be added to handleLoginEvent, and removed from here entirely later. Unfortunately, the current rapi implementation does not support this. https://codereview.appspot.com/7007047/diff/15001/app/store/env.js#newcode113 app/store/env.js:113: } The implementation does not do what was specced, but we should look forward to it anyway, I think. else { if (evt.data.provider_type) { this.set('providerType', msg.data.provider_type); } if (evt.data.default_series) { this.set('defaultSeries', msg.data.default_series); } } In the future we should be able to remove the conditional parts of that, and rely on the two values existing. https://codereview.appspot.com/7007047/diff/15001/app/store/env.js#newcode223 app/store/env.js:223: window.setTimeout(Y.bind(this.login, this), 500); We talked about this and you liked the idea of connecting this to an event instead (event fired by login attempt). https://codereview.appspot.com/7007047/diff/15001/test/index.html File test/index.html (right): https://codereview.appspot.com/7007047/diff/15001/test/index.html#newcode37 test/index.html:37: // *after* the test runner is invoked. In which case the test runner "In this case," https://codereview.appspot.com/7007047/ -- https://code.launchpad.net/~benji/juju-gui/login/+merge/141133 Your team Juju GUI Hackers is requested to review the proposed merge of lp:~benji/juju-gui/login into lp:juju-gui. -- Mailing list: https://launchpad.net/~yellow Post to : [email protected] Unsubscribe : https://launchpad.net/~yellow More help : https://help.launchpad.net/ListHelp
