Two way address bar #257
Two way address bar #257
Conversation
This change makes the address bar for the demo app functional, so entering a new URL and pressing enter triggers a transition to that URL in the demo app.
|
|
||
| change(event) { | ||
| this.sendAction("urlChanged", event.target.value); | ||
| } |
There was a problem hiding this comment.
Just to be sure, this will be fired when a new URL is entered and the element is blurred, right? In the previous commit, the action is only fired, when enter is pressed...
There was a problem hiding this comment.
I'm not sure. But it seems that in testing, fillIn never fires the enter event, whatever you do.
There was a problem hiding this comment.
@pangratz Yes, blurring now makes the page switch.
|
Apart from my comment, this looks good! Thanks for fixing my failing tests @Gaurav0 😄 I guess waiting for the iframe to be loaded could eventually be refactored using |
| iframe_window = outputPane(); | ||
| iframe_window.click(iframe_window.find(aboutLink)).then(resolve); | ||
|
|
||
| done(); |
There was a problem hiding this comment.
resolve(); should remove the need for done / async
|
@joostdevries @rwjblue @alexspeller @stefanpenner @pangratz All comments addressed. Please let me know if ok to merge. |
|
lgtm 👍 |
|
wohoo, party! 🎉 |
| }, | ||
|
|
||
| afterEach: function() { | ||
| window.messagesWaiting = 0; | ||
| if (this._waiter) { |
There was a problem hiding this comment.
The idea was for the app code to do the waiting code, as it understands the async more closely.
There was a problem hiding this comment.
The problem with that is that one app sends the communication and a different app receives it. This isn't really "async", the main app just receives a communication and doesn't know when to stop and wait for it. The iframe app can't communicate to the main app except through this channel.
There was a problem hiding this comment.
Maybe an event on willTransition should be send in the demo app as well. I will try to look into this tomorrow...
|
@stefanpenner @Gaurav0 I think this is a huge step forward. A next iteration on this would ideally restart the app with the provided route as initial state (so you can more effectively debug model hook issues). cc @rwjblue |
This updates PR #248 with tests that pass reliably in PhantomJS
@joostdevries @rwjblue @alexspeller @stefanpenner Please review.