Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exposed as a function so that multiple instances of it can be created wi… #31

Closed
wants to merge 2 commits into from

Conversation

nitinsurana
Copy link

…th a different parent and do not interfere with each others' calcuation.

It also allows to re-initialize the UI with new dataset without refreshing the page
All qunit tests are working fine.
Fixes #30

…th a different parent and do not interfere with each others' calcuation.

It also allows to re-initialize the UI with new dataset without refreshing the page
All qunit tests are working fine.
@nitinsurana nitinsurana changed the title Exposed a function so that multiple instances of it can be created wi… Exposed as a function so that multiple instances of it can be created wi… May 4, 2019
@zgrossbart
Copy link
Owner

Thank you for this. I'll take a closer look tomorrow.

Do you think this should get merged into the master branch for the jsondiff.com website, or stay a separate project?

@nitinsurana
Copy link
Author

nitinsurana commented May 4, 2019

It's a lot of change :-| I'm not sure, whether we should merge it or not. It's a call you'd have to take.

As far as functionality is concerned, jsondiff.com would remain unaffected with these changes.
It may just become easier for someone to quickly inject this into their app.

@nitinsurana
Copy link
Author

nitinsurana commented May 4, 2019

After giving it another thought - there's still a lot of coupling between UI and diffModel. Let's not merge at this point.
Thanks for this nice project.

@zgrossbart
Copy link
Owner

I agree. I'm going to close this PR. Thank you.

@zgrossbart zgrossbart closed this May 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Corrupts the global namespace, hard to inject multiple instances in page
2 participants