search

ghinda / jotted

Environment for showcasing HTML, CSS and JavaScript, with editable source.
https://ghinda.net/jotted/
MIT License
491 stars 36 forks source link

Reload iframe before inject new content inside it. #27

Closed armoucar closed 7 years ago

armoucar commented 7 years ago

Commit message:

This fixes a problem when showing case angular projects. The problem happens when the context has already loaded angular and new content comes into play - for example, when live editing using jotted, angular fails on load its modules again. The solution was then reload the iframe and then inject the new edited content.

Personal message:

Hello @ghinda, here I am again :)

Again with a new PR and don't really know if it makes sense. After the last change that fixed the chrome's iframe loading issues, that broke angular.js showcases that I have. I spent some time trying to figure out the real problem and what I can guess by now is that angular doesn't really let you load things twice in the same context. It seems that it prepares itself and continues executing, but when you try to reload (without reloading in fact) it breaks.

See what you think about this solution and if it's good: good. If you can think of something better, I'd love to see to truly understand what this problem is.

I prepared a small demonstration in here: https://github.com/armoucar/jotted-bug-report

To emulate the error, just edit the code in the HTML tab and observe that the expected js behaviour doesn't happen in the Result tab.

ghinda commented 7 years ago

Thanks for reporting, and the fix! :beers:

Your fix was good, but Safari and WebKit on iOS just don't want to play along. Because of issues with the load event, this didn't work there.

The only fix that seems to work everywhere is to re-create the iframe element on each change. I released a new version (1.4.4) with this fix.

This will partially fix your script, but you still have an issue here: https://github.com/armoucar/jotted-bug-report/blob/master/angular-jotted.js#L55-L58

that causes the script tags to be inserted twice.

So params.content ends up containing 4 script tags.

Once that's fixed and you upgrade Jotted, everything should work.

I added you as a collaborator, so you can commit directly to the repo in case you find other issues.

If you open a pull request from a different branch in the repo, the tests will run in SauceLabs and you can see if any browsers are failing.

Thanks for all the help! :tada:

armoucar commented 7 years ago

Hi @ghinda, thanks for adding me as a collaborator. Anyway, if I have any contribution, I'll push them in a different branch so you can revise before merging it.

Thanks for pointing out the problem in my code. It is really loading the script/link tag twice in the first rendering. It is fixed now, thanks again 😊

I saw your last commit in the branch master and it doesn't seem to have the fix you meant. Only two files were changed, CHANGELOG.md and package.json. I tested the 1.4.4 version and the problem is still happening.

Thanks!

ghinda commented 7 years ago

Oh man! I forgot to merge the branch. :stuck_out_tongue_winking_eye: Not gonna look good on my resume.

Thanks! Had to push a new release with the fix! :shipit:

Please try out 1.4.5 and let me know if the bug is fixed.

armoucar commented 7 years ago

Yep, it is fixed. Thank you very much 😸

I'll take my time to go deeper in understand jotted. I still don't have any experience with ES6 projects and this is a nice opportunity learn it better.

Thanks again :)