https://codereview.appspot.com/6849078/diff/1/app/app.js#newcode128
app/app.js:128: // F1...F12 or esc
On 2012/11/19 18:36:51, matthew.scott wrote:
> Minor: This was unclear to me at first, and I had to stare at the next
line for
> a while to understand. What you're testing for is any key that is NOT
a
> function key or esc without a modifier, correct? I think that this
might be
> worth clarifying.
Done.
https://codereview.appspot.com/6849078/diff/1/app/app.js#newcode145
app/app.js:145: this.show_environment();
On 2012/11/19 18:36:51, matthew.scott wrote:
> I'm not sure this is the right way to go about this, as it causes a
regression.
> If I view a service, then hit alt+E, the environment view is shown,
but not
> navigated to, leaving the url, in my case, http://localhost:8888/service/mysql/.
> This breaks back-button behavior.
> * view one service, mysql
> * hit alt+E
> * view another service, wordpress
> * hit back: get taken to viewing mysql, because that was the last
thing in the
> URL stack.
Good catch. Thanks.
I've solved this issue by firing a 'navidateTo' event. So now instead of
this.show_environment(), we call this.fire('navigateTo', { url: '/' });
Hmmm. Not shure. I just want to test if we have the next object (for
unit tests purposes). If the developer passes one thing other than a
function, we should just let the exception be thrown. wdyt?
Thanks for your review, Matt!
https:/ /codereview. appspot. com/6849078/ diff/1/ app/app. js
File app/app.js (right):
https:/ /codereview. appspot. com/6849078/ diff/1/ app/app. js#newcode128
app/app.js:128: // F1...F12 or esc
On 2012/11/19 18:36:51, matthew.scott wrote:
> Minor: This was unclear to me at first, and I had to stare at the next
line for
> a while to understand. What you're testing for is any key that is NOT
a
> function key or esc without a modifier, correct? I think that this
might be
> worth clarifying.
Done.
https:/ /codereview. appspot. com/6849078/ diff/1/ app/app. js#newcode145 environment( ); localhost: 8888/service/ mysql/.
app/app.js:145: this.show_
On 2012/11/19 18:36:51, matthew.scott wrote:
> I'm not sure this is the right way to go about this, as it causes a
regression.
> If I view a service, then hit alt+E, the environment view is shown,
but not
> navigated to, leaving the url, in my case,
http://
> This breaks back-button behavior.
> * view one service, mysql
> * hit alt+E
> * view another service, wordpress
> * hit back: get taken to viewing mysql, because that was the last
thing in the
> URL stack.
Good catch. Thanks. environment( ), we call this.fire( 'navigateTo' , { url: '/' });
I've solved this issue by firing a 'navidateTo' event. So now instead of
this.show_
https:/ /codereview. appspot. com/6849078/ diff/1/ app/app. js#newcode557 isFunction( next), here?
app/app.js:557: if (next) {
On 2012/11/19 18:36:51, matthew.scott wrote:
> Perhaps Y.Lang.
Hmmm. Not shure. I just want to test if we have the next object (for
unit tests purposes). If the developer passes one thing other than a
function, we should just let the exception be thrown. wdyt?
https:/ /codereview. appspot. com/6849078/