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

Removed redundant 'appendHTML()' call on DOM load #688

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

alexeiTruhin
Copy link
Contributor

No description provided.

@aik099
Copy link

aik099 commented Jan 15, 2015

Please keep only on topic changes in this PR and leave out coding standard changes (e.g. trailing whitespaces removal on empty lines and such) to a separate PR.

@jackmoore
Copy link
Owner

@aik099, Thanks, I think that is good advice.

@alexeiTruhin, Thanks for the pull requests. I think this change is fine, but I want to point out it's not completely redundant. The line you removed insures that Colorbox is added to the DOM regardless if it's been assigned to any elements yet. This seems redundant because Colorbox checks to make sure it's present in the DOM as it's called, but having it already in the DOM earlier than that gives the browser the chance to download the background images, avoiding any flash of unstyled content.

@alexeiTruhin
Copy link
Contributor Author

@aik099 yeah, sorry for that. Sublime did it automagically.

@jackmoore I removed the line outside the 'publicMethod'. But it also could be done vice-versa.

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.

3 participants