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

Added a json content type. Added the ability to pass arguments to the tr... #22

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chawkinsuf
Copy link

I wanted to be able to pull data as json and display it in the popup, rather than render the html server side. The changes are minimal and I think it fits well with the rest of the system. The update to triggerCall might be useful elsewhere as well.

@dinbror
Copy link
Owner

dinbror commented Dec 2, 2013

Are you just showing raw json data in the popup?

If not I would do the logic in the onOpen callback because you need to process your json data. Are you using templating?

@chawkinsuf
Copy link
Author

I am processing the json of course, and I am using templating. I tried various different ways to not have to update the code, but nothing was to my satisfaction. I will have to go back and look at your suggestion again, as it's been a while since I last worked on this.

Part of the problem, is the ajax content option uses the $.load function. This puts whatever is loaded directly into the html, including json. That function does not provide direct access to the data loaded with ajax, and I am not sure, but I suspect there might be issues trying to parse the json after it has been inserted into html.

Do you see a problem with having a content type that is specific for json data, or other data that you don't want to be directly inserted into the page?

I also came to like the ability to pass the wrapping div directly to the callback for manipulation. That could easily be done for the other content types as well, and I think improves the usability of the system.

@chawkinsuf
Copy link
Author

Do you have an interest in integrating these updates into the project? Are there modifications to my pull request you would suggest before you would pull them?

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.

2 participants