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

Flexible performance tuning, use the inner html of inline content containers #678

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

Conversation

rjcorwin
Copy link

I ran into a situation where inline mode was required and putting 100+ high quality images inside of hidden div tags would compete with other media loading on the page. In other words, this was not an appropriate situation to cache the full scale images before loading by putting them in a hidden div tag, they needed to be loaded on demand. To accomplish this I made the inline HTML containers script tags to prevent the image tag from being interpreted as HTML until it was needed. For example:

            <script type="text/template" id="modal-node-44">
                <img src="http://38.media.tumblr.com/1168e5eb25c9f871d502945f5dd48f31/tumblr_mxs2sv5RwC1qzy7w4o1_500.jpg">
            </script> 

The problem is that the script tag itself would also end up in the modal content area resulting in what appeared to be an empty modal. Changing one line in jquery.colorbox.js, colorbox now uses the inner HTML of the inline HTML container for the modal content. Not only does this give you cache control, ie. the flexibility of choosing if inline modal content is loaded on page load, it also results in what may be seen as a cleaner approach to when you do want content cached. If anything, it's less verbose.

            <div style="display: none;">
                <div id="modal-node-44">
                    <img src="http://38.media.tumblr.com/1168e5eb25c9f871d502945f5dd48f31/tumblr_mxs2sv5RwC1qzy7w4o1_500.jpg">
                </div> 
            </div>

vs.

            <div style="display: none;" id="modal-node-44">
                <img src="http://38.media.tumblr.com/1168e5eb25c9f871d502945f5dd48f31/tumblr_mxs2sv5RwC1qzy7w4o1_500.jpg">
            </div>

@jackmoore I submit my pull request here for your consideration. I'm wondering what your thoughts are here on my strategy for cache control. If it is deemed a worthy one, I could roll this pull request as an optional useInnerHtmlForInlineContainers setting to make this backwards compatible.

…ntent containers and also support for using the performant script tag
@aik099
Copy link

aik099 commented Dec 30, 2014

So the .html() usage that you've introduced isn't adding temporary div itself, that was added before, but only it's contents.

Any side effects from not having that DIV?

@rjcorwin
Copy link
Author

The only side affect I can think of is if you intended to have that DIV and suddenly found that it wasn't included, this is I think is a good reason to have this functionality as optional so folks who upgrade don't find themselves missing divs in their modals.

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